Changeset 201359 in webkit

Timestamp:

May 24, 2016, 3:28:20 PM (10 years ago)

Author:

sbarati@apple.com

Message:

We can cache lookups to JSScope::abstractResolve inside CodeBlock::finishCreation
​https://bugs.webkit.org/show_bug.cgi?id=158036

Reviewed by Geoffrey Garen.

This patch implements a 1 item cache for JSScope::abstractResolve. I also tried
implementing the cache as a HashMap, but it seemed either less profitable on some
benchmarks or just as profitable on others. Therefore, it's cleaner to just
use a 1 item cache.

• bytecode/CodeBlock.cpp:

(JSC::CodeBlock::CodeBlock):
(JSC::AbstractResolveKey::AbstractResolveKey):
(JSC::AbstractResolveKey::operator==):
(JSC::AbstractResolveKey::isEmptyValue):
(JSC::CodeBlock::finishCreation):

• runtime/GetPutInfo.h:

(JSC::needsVarInjectionChecks):
(JSC::ResolveOp::ResolveOp):

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• bytecode/CodeBlock.cpp (modified) (6 diffs)
• runtime/GetPutInfo.h (modified) (1 diff)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2016-05-24  Saam barati  <sbarati@apple.com>
+
+        We can cache lookups to JSScope::abstractResolve inside CodeBlock::finishCreation
+        https://bugs.webkit.org/show_bug.cgi?id=158036
+
+        Reviewed by Geoffrey Garen.
+
+        This patch implements a 1 item cache for JSScope::abstractResolve. I also tried
+        implementing the cache as a HashMap, but it seemed either less profitable on some
+        benchmarks or just as profitable on others. Therefore, it's cleaner to just
+        use a 1 item cache.
+
+        * bytecode/CodeBlock.cpp:
+        (JSC::CodeBlock::CodeBlock):
+        (JSC::AbstractResolveKey::AbstractResolveKey):
+        (JSC::AbstractResolveKey::operator==):
+        (JSC::AbstractResolveKey::isEmptyValue):
+        (JSC::CodeBlock::finishCreation):
+        * runtime/GetPutInfo.h:
+        (JSC::needsVarInjectionChecks):
+        (JSC::ResolveOp::ResolveOp):
+
 2016-05-24  Filip Pizlo  <fpizlo@apple.com>
 

trunk/Source/JavaScriptCore/bytecode/CodeBlock.cpp

@@ -1847 +1847 @@
 }
 
+struct AbstractResolveKey {
+    AbstractResolveKey()
+        : m_impl(nullptr)
+    { }
+    AbstractResolveKey(size_t depth, const Identifier& ident, GetOrPut getOrPut, ResolveType resolveType, InitializationMode initializationMode)
+        : m_depth(depth)
+        , m_impl(ident.impl())
+        , m_getOrPut(getOrPut)
+        , m_resolveType(resolveType)
+        , m_initializationMode(initializationMode)
+    { }
+
+
+    bool operator==(const AbstractResolveKey& other) const
+    { 
+        return m_impl == other.m_impl
+            && m_depth == other.m_depth
+            && m_getOrPut == other.m_getOrPut
+            && m_resolveType == other.m_resolveType
+            && m_initializationMode == other.m_initializationMode;
+    }
+
+    bool isNull() const { return !m_impl; }
+
+    size_t m_depth;
+    UniquedStringImpl* m_impl;
+    GetOrPut m_getOrPut;
+    ResolveType m_resolveType;
+    InitializationMode m_initializationMode;
+};
+
 void CodeBlock::finishCreation(VM& vm, CopyParsedBlockTag, CodeBlock& other)
 {
@@ -2015 +2046 @@
 #endif
 
+    AbstractResolveKey lastResolveKey;
+    ResolveOp lastCachedOp;
+    auto cachedAbstractResolve = [&] (size_t localScopeDepth, const Identifier& ident, GetOrPut getOrPut, ResolveType resolveType, InitializationMode initializationMode) -> const ResolveOp& {
+        AbstractResolveKey key(localScopeDepth, ident, getOrPut, resolveType, initializationMode);
+        if (key == lastResolveKey) {
+            ASSERT(!lastResolveKey.isNull());
+            return lastCachedOp;
+        }
+        lastCachedOp = JSScope::abstractResolve(m_globalObject->globalExec(), localScopeDepth, scope, ident, getOrPut, resolveType, initializationMode);
+        lastResolveKey = key;
+        return lastCachedOp;
+    };
+
     // Copy and translate the UnlinkedInstructions
     unsigned instructionCount = unlinkedCodeBlock->instructions().count();
@@ -2126 +2170 @@
             int localScopeDepth = pc[5].u.operand;
 
-            ResolveOp op = JSScope::abstractResolve(m_globalObject->globalExec(), localScopeDepth, scope, ident, Get, type, InitializationMode::NotInitialization);
+            const ResolveOp& op = cachedAbstractResolve(localScopeDepth, ident, Get, type, InitializationMode::NotInitialization);
             instructions[i + 4].u.operand = op.type;
             instructions[i + 5].u.operand = op.depth;
@@ -2163 +2207 @@
 
             const Identifier& ident = identifier(pc[3].u.operand);
-            ResolveOp op = JSScope::abstractResolve(m_globalObject->globalExec(), localScopeDepth, scope, ident, Get, getPutInfo.resolveType(), InitializationMode::NotInitialization);
+            const ResolveOp& op = cachedAbstractResolve(localScopeDepth, ident, Get, getPutInfo.resolveType(), InitializationMode::NotInitialization);
 
             instructions[i + 4].u.operand = GetPutInfo(getPutInfo.resolveMode(), op.type, getPutInfo.initializationMode()).operand();
@@ -2198 +2242 @@
             int localScopeDepth = pc[5].u.operand;
             instructions[i + 5].u.pointer = nullptr;
-            ResolveOp op = JSScope::abstractResolve(m_globalObject->globalExec(), localScopeDepth, scope, ident, Put, getPutInfo.resolveType(), getPutInfo.initializationMode());
+            const ResolveOp& op = cachedAbstractResolve(localScopeDepth, ident, Put, getPutInfo.resolveType(), getPutInfo.initializationMode());
 
             instructions[i + 4].u.operand = GetPutInfo(getPutInfo.resolveMode(), op.type, getPutInfo.initializationMode()).operand();
@@ -2232 +2276 @@
                 // Even though type profiling may be profiling either a Get or a Put, we can always claim a Get because
                 // we're abstractly "read"ing from a JSScope.
-                ResolveOp op = JSScope::abstractResolve(m_globalObject->globalExec(), localScopeDepth, scope, ident, Get, type, InitializationMode::NotInitialization);
+                const ResolveOp& op = cachedAbstractResolve(localScopeDepth, ident, Get, type, InitializationMode::NotInitialization);
 
                 if (op.type == ClosureVar || op.type == ModuleVar)

trunk/Source/JavaScriptCore/runtime/GetPutInfo.h

@@ -180 +180 @@
 
 struct ResolveOp {
+    ResolveOp()
+        : depth(0)
+        , structure(nullptr)
+        , lexicalEnvironment(nullptr)
+        , watchpointSet(nullptr)
+        , importedName(nullptr)
+    { }
+
     ResolveOp(ResolveType type, size_t depth, Structure* structure, JSLexicalEnvironment* lexicalEnvironment, WatchpointSet* watchpointSet, uintptr_t operand, UniquedStringImpl* importedName = nullptr)
         : type(type)