Reviewed by Oliver Hunt.
authorap@webkit.org <ap@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 29 Jul 2008 08:16:17 +0000 (08:16 +0000)
committerap@webkit.org <ap@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 29 Jul 2008 08:16:17 +0000 (08:16 +0000)
        Store UString::Rep::isStatic bit in identifierTable pointer instead of reportedCost for
        slightly nicer code and a 0.5% SunSpider improvement.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@35418 268f45cc-cd09-0410-ab3c-d52691b4dbfc

JavaScriptCore/API/JSClassRef.cpp
JavaScriptCore/API/JSStringRef.cpp
JavaScriptCore/ChangeLog
JavaScriptCore/kjs/PropertyNameArray.cpp
JavaScriptCore/kjs/identifier.cpp
JavaScriptCore/kjs/identifier.h
JavaScriptCore/kjs/ustring.cpp
JavaScriptCore/kjs/ustring.h

index fe0c925..515272b 100644 (file)
@@ -83,12 +83,12 @@ OpaqueJSClass::OpaqueJSClass(const JSClassDefinition* definition, OpaqueJSClass*
 
 OpaqueJSClass::~OpaqueJSClass()
 {
-    ASSERT(!m_className.rep()->identifierTable);
+    ASSERT(!m_className.rep()->identifierTable());
 
     if (m_staticValues) {
         OpaqueJSClassStaticValuesTable::const_iterator end = m_staticValues->end();
         for (OpaqueJSClassStaticValuesTable::const_iterator it = m_staticValues->begin(); it != end; ++it) {
-            ASSERT(!it->first->identifierTable);
+            ASSERT(!it->first->identifierTable());
             delete it->second;
         }
         delete m_staticValues;
@@ -97,7 +97,7 @@ OpaqueJSClass::~OpaqueJSClass()
     if (m_staticFunctions) {
         OpaqueJSClassStaticFunctionsTable::const_iterator end = m_staticFunctions->end();
         for (OpaqueJSClassStaticFunctionsTable::const_iterator it = m_staticFunctions->begin(); it != end; ++it) {
-            ASSERT(!it->first->identifierTable);
+            ASSERT(!it->first->identifierTable());
             delete it->second;
         }
         delete m_staticFunctions;
@@ -149,7 +149,7 @@ OpaqueJSClassContextData::OpaqueJSClassContextData(OpaqueJSClass* jsClass)
         staticValues = new OpaqueJSClassStaticValuesTable;
         OpaqueJSClassStaticValuesTable::const_iterator end = jsClass->m_staticValues->end();
         for (OpaqueJSClassStaticValuesTable::const_iterator it = jsClass->m_staticValues->begin(); it != end; ++it) {
-            ASSERT(!it->first->identifierTable);
+            ASSERT(!it->first->identifierTable());
             staticValues->add(UString::Rep::createCopying(it->first->data(), it->first->size()),
                               new StaticValueEntry(it->second->getProperty, it->second->setProperty, it->second->attributes));
         }
@@ -162,7 +162,7 @@ OpaqueJSClassContextData::OpaqueJSClassContextData(OpaqueJSClass* jsClass)
         staticFunctions = new OpaqueJSClassStaticFunctionsTable;
         OpaqueJSClassStaticFunctionsTable::const_iterator end = jsClass->m_staticFunctions->end();
         for (OpaqueJSClassStaticFunctionsTable::const_iterator it = jsClass->m_staticFunctions->begin(); it != end; ++it) {
-            ASSERT(!it->first->identifierTable);
+            ASSERT(!it->first->identifierTable());
             staticFunctions->add(UString::Rep::createCopying(it->first->data(), it->first->size()),
                               new StaticFunctionEntry(it->second->callAsFunction, it->second->attributes));
         }
index 4356543..609fe96 100644 (file)
@@ -64,7 +64,7 @@ JSStringRef JSStringRetain(JSStringRef string)
 void JSStringRelease(JSStringRef string)
 {
     UString::Rep* rep = toJS(string);
-    bool needsLocking = rep->identifierTable;
+    bool needsLocking = rep->identifierTable();
     if (needsLocking) {
         // It is wasteful to take the lock for non-shared contexts, but we don't have a good way
         // to determine what the context is.
index 7264c5e..69638bd 100644 (file)
@@ -1,3 +1,35 @@
+2008-07-29  Alexey Proskuryakov  <ap@webkit.org>
+
+        Reviewed by Oliver Hunt.
+
+        Store UString::Rep::isStatic bit in identifierTable pointer instead of reportedCost for
+        slightly nicer code and a 0.5% SunSpider improvement.
+
+        * API/JSClassRef.cpp:
+        (OpaqueJSClass::~OpaqueJSClass):
+        (OpaqueJSClassContextData::OpaqueJSClassContextData):
+        * API/JSStringRef.cpp:
+        (JSStringRelease):
+        * kjs/PropertyNameArray.cpp:
+        (KJS::PropertyNameArray::add):
+        * kjs/identifier.cpp:
+        (KJS::IdentifierTable::~IdentifierTable):
+        (KJS::IdentifierTable::add):
+        (KJS::Identifier::addSlowCase):
+        (KJS::Identifier::remove):
+        * kjs/identifier.h:
+        (KJS::Identifier::add):
+        * kjs/ustring.cpp:
+        (KJS::):
+        (KJS::UString::Rep::create):
+        (KJS::UString::Rep::destroy):
+        * kjs/ustring.h:
+        (KJS::UString::Rep::identifierTable):
+        (KJS::UString::Rep::setIdentifierTable):
+        (KJS::UString::Rep::isStatic):
+        (KJS::UString::Rep::setStatic):
+        (KJS::UString::cost):
+
 2008-07-28  Geoffrey Garen  <ggaren@apple.com>
 
         Reviewed by Sam Weinig.
index 7fcb006..eb36533 100644 (file)
@@ -27,7 +27,7 @@ static const size_t setThreshold = 20;
 
 void PropertyNameArray::add(UString::Rep* identifier)
 {
-    ASSERT(identifier == &UString::Rep::null || identifier == &UString::Rep::empty || identifier->identifierTable);
+    ASSERT(identifier == &UString::Rep::null || identifier == &UString::Rep::empty || identifier->identifierTable());
 
     size_t size = m_vector.size();
     if (size < setThreshold) {
index ea3e336..82faf9e 100644 (file)
@@ -40,13 +40,13 @@ public:
     {
         HashSet<UString::Rep*>::iterator end = m_table.end();
         for (HashSet<UString::Rep*>::iterator iter = m_table.begin(); iter != end; ++iter)
-            (*iter)->identifierTable = 0;
+            (*iter)->setIdentifierTable(0);
     }
     
     std::pair<HashSet<UString::Rep*>::iterator, bool> add(UString::Rep* value)
     {
         std::pair<HashSet<UString::Rep*>::iterator, bool> result = m_table.add(value);
-        (*result.first)->identifierTable = this;
+        (*result.first)->setIdentifierTable(this);
         return result;
     }
 
@@ -54,7 +54,7 @@ public:
     std::pair<HashSet<UString::Rep*>::iterator, bool> add(U value)
     {
         std::pair<HashSet<UString::Rep*>::iterator, bool> result = m_table.add<U, V>(value);
-        (*result.first)->identifierTable = this;
+        (*result.first)->setIdentifierTable(this);
         return result;
     }
 
@@ -204,7 +204,7 @@ PassRefPtr<UString::Rep> Identifier::add(ExecState* exec, const UChar* s, int le
 
 PassRefPtr<UString::Rep> Identifier::addSlowCase(JSGlobalData* globalData, UString::Rep* r)
 {
-    ASSERT(!r->identifierTable);
+    ASSERT(!r->identifierTable());
 
     if (r->len == 0) {
         UString::Rep::empty.hash();
@@ -221,7 +221,7 @@ PassRefPtr<UString::Rep> Identifier::addSlowCase(ExecState* exec, UString::Rep*
 
 void Identifier::remove(UString::Rep *r)
 {
-    r->identifierTable->remove(r);
+    r->identifierTable()->remove(r);
 }
 
 } // namespace KJS
index 8c69c5c..fecb80f 100644 (file)
@@ -91,13 +91,13 @@ namespace KJS {
 
         static PassRefPtr<UString::Rep> add(ExecState* exec, UString::Rep* r)
         {
-            if (r->identifierTable)
+            if (r->identifierTable())
                 return r;
             return addSlowCase(exec, r);
         }
         static PassRefPtr<UString::Rep> add(JSGlobalData* globalData, UString::Rep* r)
         {
-            if (r->identifierTable)
+            if (r->identifierTable())
                 return r;
             return addSlowCase(globalData, r);
         }
index 5380b7c..7501088 100644 (file)
@@ -169,8 +169,8 @@ bool operator==(const CString& c1, const CString& c2)
 // 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;
-UString::Rep UString::Rep::null = { 0, 0, INT_MAX / 2, 0, 0, &UString::Rep::null, true, 0, 0, 0, 0, 0, 0 };
-UString::Rep UString::Rep::empty = { 0, 0, INT_MAX / 2, 0, 0, &UString::Rep::empty, true, 0, &sharedEmptyChar, 0, 0, 0, 0 };
+UString::Rep UString::Rep::null = { 0, 0, INT_MAX / 2, 0, 1, &UString::Rep::null, 0, 0, 0, 0, 0, 0 };
+UString::Rep UString::Rep::empty = { 0, 0, INT_MAX / 2, 0, 1, &UString::Rep::empty, 0, &sharedEmptyChar, 0, 0, 0, 0 };
 
 static char* statBuffer = 0; // Only used for debugging via UString::ascii().
 
@@ -190,9 +190,8 @@ PassRefPtr<UString::Rep> UString::Rep::create(UChar* d, int l)
     r->len = l;
     r->rc = 1;
     r->_hash = 0;
-    r->identifierTable = 0;
+    r->m_identifierTable = 0;
     r->baseString = r;
-    r->isStatic = false;
     r->reportedCost = 0;
     r->buf = d;
     r->usedCapacity = l;
@@ -220,9 +219,8 @@ PassRefPtr<UString::Rep> UString::Rep::create(PassRefPtr<Rep> base, int offset,
     r->len = length;
     r->rc = 1;
     r->_hash = 0;
-    r->identifierTable = 0;
+    r->m_identifierTable = 0;
     r->baseString = base.releaseRef();
-    r->isStatic = false;
     r->reportedCost = 0;
     r->buf = 0;
     r->usedCapacity = 0;
@@ -252,8 +250,8 @@ void UString::Rep::destroy()
 {
     // Static null and empty strings can never be destroyed, but we cannot rely on 
     // reference counting, because ref/deref are not thread-safe.
-    if (!isStatic) {
-        if (identifierTable)
+    if (!isStatic()) {
+        if (identifierTable())
             Identifier::remove(this);
         if (baseString == this)
             fastFree(buf);
index 3b4072c..497e4ec 100644 (file)
@@ -96,6 +96,12 @@ namespace KJS {
             static unsigned computeHash(const char*, int length);
             static unsigned computeHash(const char* s) { return computeHash(s, strlen(s)); }
 
+            IdentifierTable* identifierTable() const { return reinterpret_cast<IdentifierTable*>(m_identifierTable & ~static_cast<uintptr_t>(1)); }
+            void setIdentifierTable(IdentifierTable* table) { ASSERT(!isStatic()); m_identifierTable = reinterpret_cast<intptr_t>(table); }
+
+            bool isStatic() const { return m_identifierTable & 1; }
+            void setStatic(bool v) { ASSERT(!identifierTable()); m_identifierTable = v; }
+
             Rep* ref() { ++rc; return this; }
             ALWAYS_INLINE void deref() { if (--rc == 0) destroy(); }
 
@@ -104,10 +110,9 @@ namespace KJS {
             int len;
             int rc; // For null and empty static strings, this field does not reflect a correct count, because ref/deref are not thread-safe. A special case in destroy() guarantees that these do not get deleted.
             mutable unsigned _hash;
-            IdentifierTable* identifierTable; // 0 if not an identifier. Since garbage collection can happen on a different thread, there is no other way to get to the table during destruction.
+            intptr_t m_identifierTable; // A pointer to identifier table. The lowest bit is used to indicate whether the string is static (null or empty).
             UString::Rep* baseString;
-            bool isStatic : 1;
-            size_t reportedCost : 31;
+            size_t reportedCost;
 
             // potentially shared data
             UChar* buf;
@@ -308,28 +313,18 @@ namespace KJS {
 
     inline size_t UString::cost() const
     {
-       size_t capacity = (m_rep->baseString->capacity + m_rep->baseString->preCapacity) * sizeof(UChar);
-       size_t reportedCost = m_rep->baseString->reportedCost;
-       ASSERT(capacity >= reportedCost);
-
-       size_t capacityDelta = capacity - reportedCost;
+        size_t capacity = (m_rep->baseString->capacity + m_rep->baseString->preCapacity) * sizeof(UChar);
+        size_t reportedCost = m_rep->baseString->reportedCost;
+        ASSERT(capacity >= reportedCost);
 
-       if (capacityDelta < static_cast<size_t>(minShareSize))
-           return 0;
+        size_t capacityDelta = capacity - reportedCost;
 
-#if COMPILER(MSVC)
-// MSVC complains about this assignment, since reportedCost is a 31-bit size_t.
-#pragma warning(push)
-#pragma warning(disable: 4267)
-#endif
+        if (capacityDelta < static_cast<size_t>(minShareSize))
+            return 0;
 
         m_rep->baseString->reportedCost = capacity;
 
-#if COMPILER(MSVC)
-#pragma warning(pop)
-#endif
-
-       return capacityDelta;
+        return capacityDelta;
     }
 
 } // namespace KJS