[SCM] WebKit Debian packaging branch, webkit-1.2, updated. upstream/1.1.90-6072-g9a69373
barraclough at apple.com
barraclough at apple.com
Thu Apr 8 02:19:16 UTC 2010
The following commit has been merged in the webkit-1.2 branch:
commit 77b7fcaf09fe802be73b3c35d33927c126752567
Author: barraclough at apple.com <barraclough at apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Thu Mar 11 02:36:08 2010 +0000
https://bugs.webkit.org/show_bug.cgi?id=35991
Would be faster to not use a thread specific to implement StringImpl::empty()
Reviewed by Darin Adler, Geoffrey Garen, Maciej Stachowiak.
JavaScriptCore:
Change JSC::UStringImpl's implementation of empty() match to match StringImpl's new implementation
(use a static defined within the empty() method), and change the interface to match too (return
a pointer not a reference).
~0% performance impact (possible minor progression from moving empty() from .h to .cpp).
* JavaScriptCore.exp:
* runtime/Identifier.cpp:
(JSC::Identifier::add):
(JSC::Identifier::addSlowCase):
* runtime/PropertyNameArray.cpp:
(JSC::PropertyNameArray::add):
* runtime/UString.cpp:
(JSC::initializeUString):
(JSC::UString::UString):
* runtime/UStringImpl.cpp:
(JSC::UStringImpl::empty):
(JSC::UStringImpl::create):
* runtime/UStringImpl.h:
(JSC::UStringImpl::adopt):
(JSC::UStringImpl::createUninitialized):
(JSC::UStringImpl::tryCreateUninitialized):
WebCore:
Copy JavaScriptCore in making 'static' strings threadsafe, make the empty string a static,
shared by all threads.
~2% progression on Dromaeo DOM core & JS lib tests.
* platform/ThreadGlobalData.cpp:
(WebCore::ThreadGlobalData::ThreadGlobalData):
(WebCore::ThreadGlobalData::~ThreadGlobalData):
* platform/ThreadGlobalData.h:
(WebCore::ThreadGlobalData::eventNames):
* platform/text/StringImpl.cpp:
(WebCore::StringImpl::StringImpl):
(WebCore::StringImpl::empty):
* platform/text/StringImpl.h:
(WebCore::StringImpl::deref):
(WebCore::StringImpl::hasOneRef):
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@55825 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/JavaScriptCore/ChangeLog b/JavaScriptCore/ChangeLog
index 9a2ee93..9a7b52e 100644
--- a/JavaScriptCore/ChangeLog
+++ b/JavaScriptCore/ChangeLog
@@ -1,3 +1,33 @@
+2010-03-10 Gavin Barraclough <barraclough at apple.com>
+
+ Reviewed by Darin Adler, Geoffrey Garen, Maciej Stachowiak.
+
+ https://bugs.webkit.org/show_bug.cgi?id=35991
+ Would be faster to not use a thread specific to implement StringImpl::empty()
+
+ Change JSC::UStringImpl's implementation of empty() match to match StringImpl's new implementation
+ (use a static defined within the empty() method), and change the interface to match too (return
+ a pointer not a reference).
+
+ ~0% performance impact (possible minor progression from moving empty() from .h to .cpp).
+
+ * JavaScriptCore.exp:
+ * runtime/Identifier.cpp:
+ (JSC::Identifier::add):
+ (JSC::Identifier::addSlowCase):
+ * runtime/PropertyNameArray.cpp:
+ (JSC::PropertyNameArray::add):
+ * runtime/UString.cpp:
+ (JSC::initializeUString):
+ (JSC::UString::UString):
+ * runtime/UStringImpl.cpp:
+ (JSC::UStringImpl::empty):
+ (JSC::UStringImpl::create):
+ * runtime/UStringImpl.h:
+ (JSC::UStringImpl::adopt):
+ (JSC::UStringImpl::createUninitialized):
+ (JSC::UStringImpl::tryCreateUninitialized):
+
2010-03-10 Dmitry Titov <dimich at chromium.org>
Not reviewed, fixing Snow Leopard build.
diff --git a/JavaScriptCore/JavaScriptCore.exp b/JavaScriptCore/JavaScriptCore.exp
index d171746..9e2493f 100644
--- a/JavaScriptCore/JavaScriptCore.exp
+++ b/JavaScriptCore/JavaScriptCore.exp
@@ -106,7 +106,7 @@ __ZN3JSC11JSByteArray15createStructureENS_7JSValueE
__ZN3JSC11JSByteArrayC1EPNS_9ExecStateEN3WTF17NonNullPassRefPtrINS_9StructureEEEPNS3_9ByteArrayEPKNS_9ClassInfoE
__ZN3JSC11ParserArena5resetEv
__ZN3JSC11UStringImpl12sharedBufferEv
-__ZN3JSC11UStringImpl7s_emptyE
+__ZN3JSC11UStringImpl5emptyEv
__ZN3JSC11UStringImplD1Ev
__ZN3JSC11checkSyntaxEPNS_9ExecStateERKNS_10SourceCodeE
__ZN3JSC12DateInstance4infoE
diff --git a/JavaScriptCore/runtime/Identifier.cpp b/JavaScriptCore/runtime/Identifier.cpp
index 24deee0..4741261 100644
--- a/JavaScriptCore/runtime/Identifier.cpp
+++ b/JavaScriptCore/runtime/Identifier.cpp
@@ -130,8 +130,8 @@ PassRefPtr<UString::Rep> Identifier::add(JSGlobalData* globalData, const char* c
return rep;
}
if (!c[0]) {
- UString::Rep::empty().hash();
- return &UString::Rep::empty();
+ UString::Rep::empty()->hash();
+ return UString::Rep::empty();
}
if (!c[1])
return add(globalData, globalData->smallStrings.singleCharacterStringRep(static_cast<unsigned char>(c[0])));
@@ -194,8 +194,8 @@ PassRefPtr<UString::Rep> Identifier::add(JSGlobalData* globalData, const UChar*
return add(globalData, globalData->smallStrings.singleCharacterStringRep(c));
}
if (!length) {
- UString::Rep::empty().hash();
- return &UString::Rep::empty();
+ UString::Rep::empty()->hash();
+ return UString::Rep::empty();
}
UCharBuffer buf = {s, length};
pair<HashSet<UString::Rep*>::iterator, bool> addResult = globalData->identifierTable->add<UCharBuffer, UCharBufferTranslator>(buf);
@@ -225,8 +225,8 @@ PassRefPtr<UString::Rep> Identifier::addSlowCase(JSGlobalData* globalData, UStri
}
}
if (!r->length()) {
- UString::Rep::empty().hash();
- return &UString::Rep::empty();
+ UString::Rep::empty()->hash();
+ return UString::Rep::empty();
}
return *globalData->identifierTable->add(r).first;
}
diff --git a/JavaScriptCore/runtime/PropertyNameArray.cpp b/JavaScriptCore/runtime/PropertyNameArray.cpp
index 9f84196..6b24669 100644
--- a/JavaScriptCore/runtime/PropertyNameArray.cpp
+++ b/JavaScriptCore/runtime/PropertyNameArray.cpp
@@ -30,7 +30,7 @@ static const size_t setThreshold = 20;
void PropertyNameArray::add(UString::Rep* identifier)
{
- ASSERT(identifier == UString::null().rep() || identifier == &UString::Rep::empty() || identifier->isIdentifier());
+ ASSERT(identifier == UString::null().rep() || identifier == UString::Rep::empty() || identifier->isIdentifier());
size_t size = m_data->propertyNameVector().size();
if (size < setThreshold) {
diff --git a/JavaScriptCore/runtime/UString.cpp b/JavaScriptCore/runtime/UString.cpp
index 1684ec2..5215bcd 100644
--- a/JavaScriptCore/runtime/UString.cpp
+++ b/JavaScriptCore/runtime/UString.cpp
@@ -146,17 +146,15 @@ bool operator==(const CString& c1, const CString& c2)
return len == c2.size() && (len == 0 || memcmp(c1.c_str(), c2.c_str(), len) == 0);
}
-// These static strings are immutable, except for rc, whose initial value is chosen to
-// reduce the possibility of it becoming zero due to ref/deref not being thread-safe.
-static UChar sharedEmptyChar;
-UStringImpl* UStringImpl::s_empty;
-
+// The null string is immutable, except for refCount.
UString::Rep* UString::s_nullRep;
UString* UString::s_nullUString;
void initializeUString()
{
- UStringImpl::s_empty = new UStringImpl(&sharedEmptyChar, 0, UStringImpl::ConstructStaticString);
+ // UStringImpl::empty() does not construct its static string in a threadsafe fashion,
+ // so ensure it has been initialized from here.
+ UStringImpl::empty();
UString::s_nullRep = new UStringImpl(0, 0, UStringImpl::ConstructStaticString);
UString::s_nullUString = new UString;
@@ -173,11 +171,8 @@ UString::UString(const char* c, unsigned length)
}
UString::UString(const UChar* c, unsigned length)
+ : m_rep(Rep::create(c, length))
{
- if (length == 0)
- m_rep = &Rep::empty();
- else
- m_rep = Rep::create(c, length);
}
UString UString::from(int i)
diff --git a/JavaScriptCore/runtime/UStringImpl.cpp b/JavaScriptCore/runtime/UStringImpl.cpp
index afd8244..3ba32de 100644
--- a/JavaScriptCore/runtime/UStringImpl.cpp
+++ b/JavaScriptCore/runtime/UStringImpl.cpp
@@ -27,6 +27,7 @@
#include "UStringImpl.h"
#include "Identifier.h"
+#include "StdLibExtras.h"
#include "UString.h"
#include <wtf/unicode/UTF8.h>
@@ -35,41 +36,45 @@ using namespace std;
namespace JSC {
-PassRefPtr<UStringImpl> UStringImpl::create(const char* c)
+UStringImpl* UStringImpl::empty()
{
- ASSERT(c);
-
- if (!c[0])
- return &UStringImpl::empty();
-
- size_t length = strlen(c);
- UChar* d;
- PassRefPtr<UStringImpl> result = UStringImpl::createUninitialized(length, d);
- for (size_t i = 0; i < length; i++)
- d[i] = static_cast<unsigned char>(c[i]); // use unsigned char to zero-extend instead of sign-extend
- return result;
+ // A non-null pointer at an invalid address (in page zero) so that if it were to be accessed we
+ // should catch the error with fault (however it should be impossible to access, since length is zero).
+ static const UChar* invalidNonNullUCharPtr = reinterpret_cast<UChar*>(static_cast<intptr_t>(1));
+ DEFINE_STATIC_LOCAL(UStringImpl, emptyString, (invalidNonNullUCharPtr, 0, ConstructStaticString));
+ return &emptyString;
}
-PassRefPtr<UStringImpl> UStringImpl::create(const char* c, unsigned length)
+PassRefPtr<UStringImpl> UStringImpl::create(const UChar* characters, unsigned length)
{
- ASSERT(c);
+ if (!characters || !length)
+ return empty();
- if (!length)
- return &UStringImpl::empty();
+ UChar* data;
+ PassRefPtr<UStringImpl> string = createUninitialized(length, data);
+ memcpy(data, characters, length * sizeof(UChar));
+ return string;
+}
- UChar* d;
- PassRefPtr<UStringImpl> result = UStringImpl::createUninitialized(length, d);
- for (unsigned i = 0; i < length; i++)
- d[i] = static_cast<unsigned char>(c[i]); // use unsigned char to zero-extend instead of sign-extend
- return result;
+PassRefPtr<UStringImpl> UStringImpl::create(const char* characters, unsigned length)
+{
+ if (!characters || !length)
+ return empty();
+
+ UChar* data;
+ PassRefPtr<UStringImpl> string = createUninitialized(length, data);
+ for (unsigned i = 0; i != length; ++i) {
+ unsigned char c = characters[i];
+ data[i] = c;
+ }
+ return string;
}
-PassRefPtr<UStringImpl> UStringImpl::create(const UChar* buffer, unsigned length)
+PassRefPtr<UStringImpl> UStringImpl::create(const char* string)
{
- UChar* newBuffer;
- PassRefPtr<UStringImpl> impl = createUninitialized(length, newBuffer);
- copyChars(newBuffer, buffer, length);
- return impl;
+ if (!string)
+ return empty();
+ return create(string, strlen(string));
}
SharedUChar* UStringImpl::baseSharedBuffer()
diff --git a/JavaScriptCore/runtime/UStringImpl.h b/JavaScriptCore/runtime/UStringImpl.h
index a1d7bb8..06903c7 100644
--- a/JavaScriptCore/runtime/UStringImpl.h
+++ b/JavaScriptCore/runtime/UStringImpl.h
@@ -113,12 +113,12 @@ public:
ASSERT(vector.data());
return adoptRef(new UStringImpl(vector.releaseBuffer(), length));
}
- return &empty();
+ return empty();
}
- static PassRefPtr<UStringImpl> create(const char* c);
- static PassRefPtr<UStringImpl> create(const char* c, unsigned length);
static PassRefPtr<UStringImpl> create(const UChar* buffer, unsigned length);
+ static PassRefPtr<UStringImpl> create(const char* c, unsigned length);
+ static PassRefPtr<UStringImpl> create(const char* c);
static PassRefPtr<UStringImpl> create(PassRefPtr<UStringImpl> rep, unsigned offset, unsigned length)
{
@@ -136,7 +136,7 @@ public:
{
if (!length) {
output = 0;
- return &empty();
+ return empty();
}
if (length > ((std::numeric_limits<size_t>::max() - sizeof(UStringImpl)) / sizeof(UChar)))
@@ -150,7 +150,7 @@ public:
{
if (!length) {
output = 0;
- return &empty();
+ return empty();
}
if (length > ((std::numeric_limits<size_t>::max() - sizeof(UStringImpl)) / sizeof(UChar)))
@@ -203,7 +203,7 @@ public:
static unsigned computeHash(const char* s, unsigned length) { return WTF::stringHash(s, length); }
static unsigned computeHash(const char* s) { return WTF::stringHash(s); }
- static UStringImpl& empty() { return *s_empty; }
+ static UStringImpl* empty();
ALWAYS_INLINE void checkConsistency() const
{
@@ -295,8 +295,6 @@ private:
};
mutable unsigned m_hash;
- JS_EXPORTDATA static UStringImpl* s_empty;
-
friend class JIT;
friend class SmallStringsStorage;
friend class UStringOrRopeImpl;
diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index 8c6f30b..bd07020 100644
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,27 @@
+2010-03-10 Gavin Barraclough <barraclough at apple.com>
+
+ Reviewed by Darin Adler, Geoffrey Garen, Maciej Stachowiak.
+
+ https://bugs.webkit.org/show_bug.cgi?id=35991
+ Would be faster to not use a thread specific to implement StringImpl::empty()
+
+ Copy JavaScriptCore in making 'static' strings threadsafe, make the empty string a static,
+ shared by all threads.
+
+ ~2% progression on Dromaeo DOM core & JS lib tests.
+
+ * platform/ThreadGlobalData.cpp:
+ (WebCore::ThreadGlobalData::ThreadGlobalData):
+ (WebCore::ThreadGlobalData::~ThreadGlobalData):
+ * platform/ThreadGlobalData.h:
+ (WebCore::ThreadGlobalData::eventNames):
+ * platform/text/StringImpl.cpp:
+ (WebCore::StringImpl::StringImpl):
+ (WebCore::StringImpl::empty):
+ * platform/text/StringImpl.h:
+ (WebCore::StringImpl::deref):
+ (WebCore::StringImpl::hasOneRef):
+
2010-03-08 Dumitru Daniliuc <dumi at chromium.org>
Reviewed by Adam Barth.
diff --git a/WebCore/WebCore.xcodeproj/project.pbxproj b/WebCore/WebCore.xcodeproj/project.pbxproj
index e7dcb46..4ef1f9c 100644
--- a/WebCore/WebCore.xcodeproj/project.pbxproj
+++ b/WebCore/WebCore.xcodeproj/project.pbxproj
@@ -14712,8 +14712,8 @@
BC53DA2D1143121E000D817E /* DOMWrapperWorld.h */,
1432E8480C51493F00B1500F /* GCController.cpp */,
1432E8460C51493800B1500F /* GCController.h */,
- B5D3601E112F8BA80048DEA8 /* JSDatabaseCallback.cpp */,
- B5D3601C112F8BA00048DEA8 /* JSDatabaseCallback.h */,
+ B5D3601E112F8BA80048DEA8 /* JSDatabaseCallback.cpp */,
+ B5D3601C112F8BA00048DEA8 /* JSDatabaseCallback.h */,
BC53DAC411432FD9000D817E /* JSDebugWrapperSet.cpp */,
BC53DAC111432EEE000D817E /* JSDebugWrapperSet.h */,
93B70D4709EB0C7C009D8468 /* JSDOMBinding.cpp */,
diff --git a/WebCore/platform/ThreadGlobalData.cpp b/WebCore/platform/ThreadGlobalData.cpp
index 5c3c294..ea8ee20 100644
--- a/WebCore/platform/ThreadGlobalData.cpp
+++ b/WebCore/platform/ThreadGlobalData.cpp
@@ -55,8 +55,7 @@ ThreadGlobalData* ThreadGlobalData::staticData;
#endif
ThreadGlobalData::ThreadGlobalData()
- : m_emptyString(new StringImpl)
- , m_atomicStringTable(new HashSet<StringImpl*>)
+ : m_atomicStringTable(new HashSet<StringImpl*>)
, m_eventNames(new EventNames)
, m_threadTimers(new ThreadTimers)
#ifndef NDEBUG
@@ -69,6 +68,12 @@ ThreadGlobalData::ThreadGlobalData()
, m_cachedConverterTEC(new TECConverterWrapper)
#endif
{
+ // StringImpl::empty() does not construct its static string in a threadsafe fashion,
+ // so ensure it has been initialized from here.
+ //
+ // This constructor will have been called on the main thread before being called on
+ // any other thread, and is only called once per thread.
+ StringImpl::empty();
}
ThreadGlobalData::~ThreadGlobalData()
@@ -82,14 +87,6 @@ ThreadGlobalData::~ThreadGlobalData()
delete m_eventNames;
delete m_atomicStringTable;
delete m_threadTimers;
-
- // Using member variable m_isMainThread instead of calling WTF::isMainThread() directly
- // to avoid issues described in https://bugs.webkit.org/show_bug.cgi?id=25973.
- // In short, some pthread-based platforms and ports can not use WTF::CurrentThread() and WTF::isMainThread()
- // in destructors of thread-specific data.
- ASSERT(m_isMainThread || m_emptyString->hasOneRef()); // We intentionally don't clean up static data on application quit, so there will be many strings remaining on the main thread.
-
- delete m_emptyString;
}
} // namespace WebCore
diff --git a/WebCore/platform/ThreadGlobalData.h b/WebCore/platform/ThreadGlobalData.h
index 2f80d7b..a984645 100644
--- a/WebCore/platform/ThreadGlobalData.h
+++ b/WebCore/platform/ThreadGlobalData.h
@@ -51,7 +51,6 @@ namespace WebCore {
~ThreadGlobalData();
EventNames& eventNames() { return *m_eventNames; }
- StringImpl* emptyString() { return m_emptyString; }
HashSet<StringImpl*>& atomicStringTable() { return *m_atomicStringTable; }
ThreadTimers& threadTimers() { return *m_threadTimers; }
@@ -64,7 +63,6 @@ namespace WebCore {
#endif
private:
- StringImpl* m_emptyString;
HashSet<StringImpl*>* m_atomicStringTable;
EventNames* m_eventNames;
ThreadTimers* m_threadTimers;
diff --git a/WebCore/platform/text/StringImpl.cpp b/WebCore/platform/text/StringImpl.cpp
index abb33b0..96c44ec 100644
--- a/WebCore/platform/text/StringImpl.cpp
+++ b/WebCore/platform/text/StringImpl.cpp
@@ -57,7 +57,7 @@ StringImpl::StringImpl()
: m_data(0)
, m_sharedBuffer(0)
, m_length(0)
- , m_refCountAndFlags(s_refCountIncrement | BufferInternal)
+ , m_refCountAndFlags(s_refCountFlagStatic | BufferInternal)
, m_hash(0)
{
// Ensure that the hash is computed so that AtomicStringHash can call existingHash()
@@ -120,7 +120,8 @@ StringImpl::~StringImpl()
StringImpl* StringImpl::empty()
{
- return threadGlobalData().emptyString();
+ DEFINE_STATIC_LOCAL(StringImpl, emptyString, ());
+ return &emptyString;
}
bool StringImpl::containsOnlyWhitespace()
diff --git a/WebCore/platform/text/StringImpl.h b/WebCore/platform/text/StringImpl.h
index 2c66019..5c86ffb 100644
--- a/WebCore/platform/text/StringImpl.h
+++ b/WebCore/platform/text/StringImpl.h
@@ -119,8 +119,8 @@ public:
inline static unsigned computeHash(const char* data) { return WTF::stringHash(data); }
StringImpl* ref() { m_refCountAndFlags += s_refCountIncrement; return this; }
- ALWAYS_INLINE void deref() { m_refCountAndFlags -= s_refCountIncrement; if (!(m_refCountAndFlags & s_refCountMask)) delete this; }
- ALWAYS_INLINE bool hasOneRef() const { return (m_refCountAndFlags & s_refCountMask) == s_refCountIncrement; }
+ ALWAYS_INLINE void deref() { m_refCountAndFlags -= s_refCountIncrement; if (!(m_refCountAndFlags & (s_refCountMask | s_refCountFlagStatic))) delete this; }
+ ALWAYS_INLINE bool hasOneRef() const { return (m_refCountAndFlags & (s_refCountMask | s_refCountFlagStatic)) == s_refCountIncrement; }
// Returns a StringImpl suitable for use on another thread.
PassRefPtr<StringImpl> crossThreadString();
@@ -201,8 +201,9 @@ private:
// In this case, the m_data pointer is an "internal buffer", and does not need to be deallocated.
BufferOwnership bufferOwnership() const { return static_cast<BufferOwnership>(m_refCountAndFlags & s_refCountMaskBufferOwnership); }
- static const unsigned s_refCountMask = 0xFFFFFFF0;
- static const unsigned s_refCountIncrement = 0x10;
+ static const unsigned s_refCountMask = 0xFFFFFFE0;
+ static const unsigned s_refCountIncrement = 0x20;
+ static const unsigned s_refCountFlagStatic = 0x10;
static const unsigned s_refCountFlagHasTerminatingNullCharacter = 0x8;
static const unsigned s_refCountFlagInTable = 0x4;
static const unsigned s_refCountMaskBufferOwnership = 0x3;
--
WebKit Debian packaging
More information about the Pkg-webkit-commits
mailing list