Changeset 201531 in webkit

Timestamp:

May 31, 2016, 3:10:10 PM (10 years ago)

Author:

commit-queue@webkit.org

Message:

Unreviewed, rolling out r201359.
​https://bugs.webkit.org/show_bug.cgi?id=158238

"It was not a speedup on anything" (Requested by saamyjoon on
#webkit).

Reverted changeset:

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

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-31  Commit Queue  <commit-queue@webkit.org>
+
+        Unreviewed, rolling out r201359.
+        https://bugs.webkit.org/show_bug.cgi?id=158238
+
+        "It was not a speedup on anything" (Requested by saamyjoon on
+        #webkit).
+
+        Reverted changeset:
+
+        "We can cache lookups to JSScope::abstractResolve inside
+        CodeBlock::finishCreation"
+        https://bugs.webkit.org/show_bug.cgi?id=158036
+        http://trac.webkit.org/changeset/201359
+
 2016-05-31  Yusuke Suzuki  <utatane.tea@gmail.com>
 

trunk/Source/JavaScriptCore/bytecode/CodeBlock.cpp

@@ -1866 +1866 @@
 }
 
-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)
 {
@@ -2065 +2034 @@
 #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();
@@ -2189 +2145 @@
             int localScopeDepth = pc[5].u.operand;
 
-            const ResolveOp& op = cachedAbstractResolve(localScopeDepth, ident, Get, type, InitializationMode::NotInitialization);
+            ResolveOp op = JSScope::abstractResolve(m_globalObject->globalExec(), localScopeDepth, scope, ident, Get, type, InitializationMode::NotInitialization);
             instructions[i + 4].u.operand = op.type;
             instructions[i + 5].u.operand = op.depth;
@@ -2226 +2182 @@
 
             const Identifier& ident = identifier(pc[3].u.operand);
-            const ResolveOp& op = cachedAbstractResolve(localScopeDepth, ident, Get, getPutInfo.resolveType(), InitializationMode::NotInitialization);
+            ResolveOp op = JSScope::abstractResolve(m_globalObject->globalExec(), localScopeDepth, scope, ident, Get, getPutInfo.resolveType(), InitializationMode::NotInitialization);
 
             instructions[i + 4].u.operand = GetPutInfo(getPutInfo.resolveMode(), op.type, getPutInfo.initializationMode()).operand();
@@ -2261 +2217 @@
             int localScopeDepth = pc[5].u.operand;
             instructions[i + 5].u.pointer = nullptr;
-            const ResolveOp& op = cachedAbstractResolve(localScopeDepth, ident, Put, getPutInfo.resolveType(), getPutInfo.initializationMode());
+            ResolveOp op = JSScope::abstractResolve(m_globalObject->globalExec(), localScopeDepth, scope, ident, Put, getPutInfo.resolveType(), getPutInfo.initializationMode());
 
             instructions[i + 4].u.operand = GetPutInfo(getPutInfo.resolveMode(), op.type, getPutInfo.initializationMode()).operand();
@@ -2295 +2251 @@
                 // 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.
-                const ResolveOp& op = cachedAbstractResolve(localScopeDepth, ident, Get, type, InitializationMode::NotInitialization);
+                ResolveOp op = JSScope::abstractResolve(m_globalObject->globalExec(), localScopeDepth, scope, 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)