We can cache lookups to JSScope::abstractResolve inside CodeBlock::fi…

…nishCreation

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):


Canonical link: https://commits.webkit.org/176167@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@201359 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/JavaScriptCore/ChangeLog

@@ -1,3 +1,25 @@
+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>
 
         Unreviwed, add a comment to describe the test's failure mode. Suggested by mlam.

Source/JavaScriptCore/bytecode/CodeBlock.cpp

@@ -1846,6 +1846,37 @@ CodeBlock::CodeBlock(VM* vm, Structure* structure, CopyParsedBlockTag, CodeBlock
     setNumParameters(other.numParameters());
 }
 
+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)
 {
     Base::finishCreation(vm);
@@ -2014,6 +2045,19 @@ void CodeBlock::finishCreation(VM& vm, ScriptExecutable* ownerExecutable, Unlink
     setCalleeSaveRegisters(RegisterSet::llintBaselineCalleeSaveRegisters());
 #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();
     UnlinkedInstructionStream::Reader instructionReader(unlinkedCodeBlock->instructions());
@@ -2125,7 +2169,7 @@ void CodeBlock::finishCreation(VM& vm, ScriptExecutable* ownerExecutable, Unlink
             RELEASE_ASSERT(type != LocalClosureVar);
             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;
             if (op.lexicalEnvironment) {
@@ -2162,7 +2206,7 @@ void CodeBlock::finishCreation(VM& vm, ScriptExecutable* ownerExecutable, Unlink
             }
 
             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();
             if (op.type == ModuleVar)
@@ -2197,7 +2241,7 @@ void CodeBlock::finishCreation(VM& vm, ScriptExecutable* ownerExecutable, Unlink
             const Identifier& ident = identifier(pc[2].u.operand);
             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();
             if (op.type == GlobalVar || op.type == GlobalVarWithVarInjectionChecks || op.type == GlobalLexicalVar || op.type == GlobalLexicalVarWithVarInjectionChecks)
@@ -2231,7 +2275,7 @@ void CodeBlock::finishCreation(VM& vm, ScriptExecutable* ownerExecutable, Unlink
                 ResolveType type = static_cast<ResolveType>(pc[5].u.operand);
                 // 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)
                     symbolTable = op.lexicalEnvironment->symbolTable();

Source/JavaScriptCore/runtime/GetPutInfo.h

@@ -179,6 +179,14 @@ ALWAYS_INLINE bool needsVarInjectionChecks(ResolveType type)
 }
 
 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)
         , depth(depth)