Changeset 242054 in webkit

Timestamp:

Feb 25, 2019 12:15:53 PM (5 years ago)

Author:

Tadeu Zagallo

Message:

Avoid hashing CompactVariableEnvironment when decoding CompactVariableMap::Handle
​https://bugs.webkit.org/show_bug.cgi?id=194937

Reviewed by Saam Barati.

Hashing the CompactVariableEnvironment is expensive and we could avoid it
when decoding multiple handles to the same environment. This is sound because
a pointer to the same CompactVariableEnvironment will hash the same.

• runtime/CachedTypes.cpp:

(JSC::Decoder::handleForEnvironment const):
(JSC::Decoder::setHandleForEnvironment):
(JSC::CachedCompactVariableMapHandle::decode const):

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• runtime/CachedTypes.cpp (modified) (3 diffs)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2019-02-25  Tadeu Zagallo  <tzagallo@apple.com>
+
+        Avoid hashing CompactVariableEnvironment when decoding CompactVariableMap::Handle
+        https://bugs.webkit.org/show_bug.cgi?id=194937
+
+        Reviewed by Saam Barati.
+
+        Hashing the CompactVariableEnvironment is expensive and we could avoid it
+        when decoding multiple handles to the same environment. This is sound because
+        a pointer to the same CompactVariableEnvironment will hash the same.
+
+        * runtime/CachedTypes.cpp:
+        (JSC::Decoder::handleForEnvironment const):
+        (JSC::Decoder::setHandleForEnvironment):
+        (JSC::CachedCompactVariableMapHandle::decode const):
+
 2019-02-25  Tadeu Zagallo  <tzagallo@apple.com>
 

trunk/Source/JavaScriptCore/runtime/CachedTypes.cpp

@@ -260 +260 @@
     }
 
+    CompactVariableMap::Handle handleForEnvironment(CompactVariableEnvironment* environment) const
+    {
+        auto it = m_environmentToHandleMap.find(environment);
+        ASSERT(it != m_environmentToHandleMap.end());
+        return it->value;
+    }
+
+    void setHandleForEnvironment(CompactVariableEnvironment* environment, const CompactVariableMap::Handle& handle)
+    {
+        auto addResult = m_environmentToHandleMap.add(environment, handle);
+        ASSERT_UNUSED(addResult, addResult.isNewEntry);
+    }
+
 private:
     VM& m_vm;
@@ -268 +281 @@
     HashMap<ptrdiff_t, void*> m_offsetToPtrMap;
     Vector<std::function<void()>> m_finalizers;
+    HashMap<CompactVariableEnvironment*, CompactVariableMap::Handle> m_environmentToHandleMap;
 };
 
@@ -939 +953 @@
         bool isNewAllocation;
         CompactVariableEnvironment* environment = m_environment.decode(decoder, isNewAllocation);
+        if (!isNewAllocation)
+            return decoder.handleForEnvironment(environment);
         bool isNewEntry;
         CompactVariableMap::Handle handle = decoder.vm().m_compactVariableMap->get(environment, isNewEntry);
-        if (!isNewAllocation)
-            ASSERT(!isNewEntry);
-        else if (!isNewEntry) {
+        if (!isNewEntry) {
             decoder.addFinalizer([=] {
                 delete environment;
             });
         }
+        decoder.setHandleForEnvironment(environment, handle);
         return handle;
     }