Title: [201359] trunk/Source/_javascript_Core
Revision
201359
Author
sbar...@apple.com
Date
2016-05-24 15:28:20 -0700 (Tue, 24 May 2016)

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

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (201358 => 201359)


--- trunk/Source/_javascript_Core/ChangeLog	2016-05-24 22:04:07 UTC (rev 201358)
+++ trunk/Source/_javascript_Core/ChangeLog	2016-05-24 22:28:20 UTC (rev 201359)
@@ -1,3 +1,25 @@
+2016-05-24  Saam barati  <sbar...@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  <fpi...@apple.com>
 
         Unreviwed, add a comment to describe the test's failure mode. Suggested by mlam.

Modified: trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp (201358 => 201359)


--- trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp	2016-05-24 22:04:07 UTC (rev 201358)
+++ trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp	2016-05-24 22:28:20 UTC (rev 201359)
@@ -1846,6 +1846,37 @@
     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 @@
     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 @@
             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 @@
             }
 
             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 @@
             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 @@
                 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();

Modified: trunk/Source/_javascript_Core/runtime/GetPutInfo.h (201358 => 201359)


--- trunk/Source/_javascript_Core/runtime/GetPutInfo.h	2016-05-24 22:04:07 UTC (rev 201358)
+++ trunk/Source/_javascript_Core/runtime/GetPutInfo.h	2016-05-24 22:28:20 UTC (rev 201359)
@@ -179,6 +179,14 @@
 }
 
 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)
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to