Changeset 184009 in webkit

Timestamp:

May 8, 2015, 1:07:29 PM (11 years ago)

Author:

oliver@apple.com

Message:

MapDataImpl::add() shouldn't do the same hash lookup twice.
​https://bugs.webkit.org/show_bug.cgi?id=144759

Reviewed by Gavin Barraclough.

We don't actually need to do a double lookup here, all we need to
do is update the index to point to the correct m_size.

• runtime/MapDataInlines.h:

(JSC::JSIterator>::add):

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• runtime/MapDataInlines.h (modified) (3 diffs)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2015-05-08  Oliver Hunt  <oliver@apple.com>
+
+        MapDataImpl::add() shouldn't do the same hash lookup twice.
+        https://bugs.webkit.org/show_bug.cgi?id=144759
+
+        Reviewed by Gavin Barraclough.
+
+        We don't actually need to do a double lookup here, all we need to
+        do is update the index to point to the correct m_size.
+
+        * runtime/MapDataInlines.h:
+        (JSC::JSIterator>::add):
+
 2015-05-08  Andreas Kling  <akling@apple.com>
 

trunk/Source/JavaScriptCore/runtime/MapDataInlines.h

@@ -92 +92 @@
 inline Entry* MapDataImpl<Entry, JSIterator>::add(ExecState* exec, JSCell* owner, Map& map, Key key, KeyType keyValue)
 {
-    typename Map::iterator location = map.find(key);
-    if (location != map.end())
-        return &m_entries[location->value];
-
-    if (!ensureSpaceForAppend(exec, owner))
+    auto result = map.add(key, m_size);
+    if (!result.isNewEntry)
+        return &m_entries[result.iterator->value];
+
+    if (!ensureSpaceForAppend(exec, owner)) {
+        map.remove(result.iterator);
         return 0;
-
-    auto result = map.add(key, m_size);
-    RELEASE_ASSERT(result.isNewEntry);
+    }
+
+    result.iterator->value = m_size;
     Entry* entry = &m_entries[m_size++];
     new (entry) Entry();
@@ -175 +176 @@
     ASSERT(shouldPack());
     int32_t newEnd = 0;
-    RELEASE_ASSERT(newCapacity > 0);
+    ASSERT(newCapacity > 0);
+    RELEASE_ASSERT(newCapacity > m_size);
+
     for (int32_t i = 0; i < m_size; i++) {
         Entry& entry = m_entries[i];
@@ -195 +198 @@
 
     // Fixup for the hashmaps
-    for (auto ptr = m_valueKeyedTable.begin(); ptr != m_valueKeyedTable.end(); ++ptr)
-        ptr->value = m_entries[ptr->value].key().get().asInt32();
-    for (auto ptr = m_cellKeyedTable.begin(); ptr != m_cellKeyedTable.end(); ++ptr)
-        ptr->value = m_entries[ptr->value].key().get().asInt32();
-    for (auto ptr = m_stringKeyedTable.begin(); ptr != m_stringKeyedTable.end(); ++ptr)
-        ptr->value = m_entries[ptr->value].key().get().asInt32();
-    for (auto ptr = m_symbolKeyedTable.begin(); ptr != m_symbolKeyedTable.end(); ++ptr)
-        ptr->value = m_entries[ptr->value].key().get().asInt32();
+    for (auto ptr = m_valueKeyedTable.begin(); ptr != m_valueKeyedTable.end(); ++ptr) {
+        if (ptr->value < m_size)
+            ptr->value = m_entries[ptr->value].key().get().asInt32();
+    }
+    for (auto ptr = m_cellKeyedTable.begin(); ptr != m_cellKeyedTable.end(); ++ptr) {
+        if (ptr->value < m_size)
+            ptr->value = m_entries[ptr->value].key().get().asInt32();
+    }
+    for (auto ptr = m_stringKeyedTable.begin(); ptr != m_stringKeyedTable.end(); ++ptr) {
+        if (ptr->value < m_size)
+            ptr->value = m_entries[ptr->value].key().get().asInt32();
+    }
+    for (auto ptr = m_symbolKeyedTable.begin(); ptr != m_symbolKeyedTable.end(); ++ptr) {
+        if (ptr->value < m_size)
+            ptr->value = m_entries[ptr->value].key().get().asInt32();
+    }
 
     ASSERT((m_size - newEnd) == m_deletedCount);