Changeset 13066 in webkit for trunk/JavaScriptCore/kjs/property_map.cpp

Timestamp:

Feb 28, 2006, 11:19:54 PM (19 years ago)

Author:

ggaren

Message:

Reviewed by Darin.

• Fixed <rdar://problem/4448098> Switch PropertyMap deleted entry placeholder to -1 from UString::Rep::null

This turned out to be only a small speedup (.12%). That's within the
margin of error for super accurate JS iBench, but Shark confirms the
same, so I think it's worth landing.

FYI, I also confirmed that the single entry optimization in
PropertyMap is a 3.2% speedup.

• kjs/property_map.cpp: (KJS::PropertyMap::~PropertyMap): (KJS::PropertyMap::clear): (KJS::PropertyMap::put): (KJS::PropertyMap::insert): (KJS::PropertyMap::rehash): (KJS::PropertyMap::remove): (KJS::PropertyMap::addSparseArrayPropertiesToReferenceList): (KJS::PropertyMap::checkConsistency):
• kjs/property_map.h: (KJS::PropertyMap::deletedSentinel):

File:

• trunk/JavaScriptCore/kjs/property_map.cpp (modified) (11 diffs)

trunk/JavaScriptCore/kjs/property_map.cpp

@@ -37 +37 @@
 #define USE_SINGLE_ENTRY 1
 
-// At the time I added USE_SINGLE_ENTRY, the optimization still gave a 1.5%
-// performance boost to the iBench JavaScript benchmark so I didn't remove it.
+// 2/28/2006 ggaren: super accurate JS iBench says that USE_SINGLE_ENTRY is a
+// 3.2% performance boost.
 
 // FIXME: _singleEntry.index is unused.
@@ -100 +100 @@
 // Algorithm concepts from Algorithms in C++, Sedgewick.
 
+// This is a method rather than a variable to work around <rdar://problem/4462053>
+static inline UString::Rep* deletedSentinel() { return reinterpret_cast<UString::Rep*>(-1); }
+
 PropertyMap::~PropertyMap()
 {
@@ -115 +118 @@
     for (int i = 0; i < minimumKeysToProcess; i++) {
         UString::Rep *key = entries[i].key;
-        if (key)
+        if (key && key != deletedSentinel())
             key->deref();
-        else
+        else if (key != deletedSentinel())
             ++minimumKeysToProcess;
     }
@@ -140 +143 @@
     for (int i = 0; i < size; i++) {
         UString::Rep *key = entries[i].key;
-        if (key) {
+        if (key && key != deletedSentinel()) {
             key->deref();
             entries[i].key = 0;
@@ -343 +346 @@
         }
         // If we find the deleted-element sentinel, remember it for use later.
-        if (key == &UString::Rep::null && !foundDeletedElement) {
+        if (key == deletedSentinel() && !foundDeletedElement) {
             foundDeletedElement = true;
             deletedElementIndex = i;
@@ -358 +361 @@
     if (foundDeletedElement) {
         i = deletedElementIndex;
-        entries[i].key->deref();
         --_table->sentinelCount;
     }
@@ -387 +389 @@
 #endif
     while (entries[i].key) {
-        assert(entries[i].key != &UString::Rep::null);
+        assert(entries[i].key != deletedSentinel());
         if (k == 0)
             k = 1 | (h % sizeMask);
@@ -447 +449 @@
         if (key) {
             // Don't copy deleted-element sentinels.
-            if (key == &UString::Rep::null)
-                key->deref();
-            else {
+            if (key != deletedSentinel()) {
                 int index = entry.index;
                 lastIndexUsed = max(index, lastIndexUsed);
@@ -509 +509 @@
         return;
     
-    // Replace this one element with the deleted sentinel,
-    // &UString::Rep::null; also set value to 0 and attributes to DontEnum
+    // Replace this one element with the deleted sentinel. Also set value to 0 and attributes to DontEnum
     // to help callers that iterate all keys not have to check for the sentinel.
     key->deref();
-    key = &UString::Rep::null;
-    key->ref();
+    key = deletedSentinel();
     entries[i].key = key;
     entries[i].value = 0;
@@ -634 +632 @@
     for (int i = 0; i != size; ++i) {
         UString::Rep *key = entries[i].key;
-        if (key && key != &UString::Rep::null) {
+        if (key && key != deletedSentinel()) {
             UString k(key);
             bool fitsInUInt32;
@@ -730 +728 @@
         if (!rep)
             continue;
-        if (rep == &UString::Rep::null) {
+        if (rep == deletedSentinel()) {
             ++sentinelCount;
             continue;