Title: [172808] trunk/Source/_javascript_Core
Revision
172808
Author
[email protected]
Date
2014-08-20 13:47:45 -0700 (Wed, 20 Aug 2014)

Log Message

Stop implicitly skipping a function's own activation when walking the scope chain
https://bugs.webkit.org/show_bug.cgi?id=136118

Reviewed by Geoffrey Garen.

Remove the current logic that implicitly skips a function's
own activation when walking the scope chain. This is ground
work for ensuring that all closed variable access is made
through the function's activation. This leads to a further
10% regression on earley, but we're already tracking the
overall performance regression.

* bytecode/CodeBlock.cpp:
(JSC::CodeBlock::CodeBlock):
* dfg/DFGAbstractInterpreterInlines.h:
(JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
* dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::getScope):
(JSC::DFG::ByteCodeParser::parseBlock):
* dfg/DFGClobberize.h:
(JSC::DFG::clobberize):
* dfg/DFGDoesGC.cpp:
(JSC::DFG::doesGC):
* dfg/DFGFixupPhase.cpp:
(JSC::DFG::FixupPhase::fixupNode):
* dfg/DFGHeapLocation.cpp:
(WTF::printInternal):
* dfg/DFGHeapLocation.h:
* dfg/DFGNodeType.h:
* dfg/DFGPredictionPropagationPhase.cpp:
(JSC::DFG::PredictionPropagationPhase::propagate):
* dfg/DFGSafeToExecute.h:
(JSC::DFG::safeToExecute):
* dfg/DFGSpeculativeJIT32_64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* dfg/DFGSpeculativeJIT64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* jit/JITPropertyAccess.cpp:
(JSC::JIT::emitResolveClosure):
* llint/LowLevelInterpreter32_64.asm:
* llint/LowLevelInterpreter64.asm:
* runtime/JSScope.cpp:
(JSC::JSScope::abstractResolve):
* runtime/JSScope.h:

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (172807 => 172808)


--- trunk/Source/_javascript_Core/ChangeLog	2014-08-20 20:28:24 UTC (rev 172807)
+++ trunk/Source/_javascript_Core/ChangeLog	2014-08-20 20:47:45 UTC (rev 172808)
@@ -1,3 +1,50 @@
+2014-08-20  Oliver Hunt  <[email protected]>
+
+        Stop implicitly skipping a function's own activation when walking the scope chain
+        https://bugs.webkit.org/show_bug.cgi?id=136118
+
+        Reviewed by Geoffrey Garen.
+
+        Remove the current logic that implicitly skips a function's
+        own activation when walking the scope chain. This is ground
+        work for ensuring that all closed variable access is made
+        through the function's activation. This leads to a further
+        10% regression on earley, but we're already tracking the
+        overall performance regression.
+
+        * bytecode/CodeBlock.cpp:
+        (JSC::CodeBlock::CodeBlock):
+        * dfg/DFGAbstractInterpreterInlines.h:
+        (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
+        * dfg/DFGByteCodeParser.cpp:
+        (JSC::DFG::ByteCodeParser::getScope):
+        (JSC::DFG::ByteCodeParser::parseBlock):
+        * dfg/DFGClobberize.h:
+        (JSC::DFG::clobberize):
+        * dfg/DFGDoesGC.cpp:
+        (JSC::DFG::doesGC):
+        * dfg/DFGFixupPhase.cpp:
+        (JSC::DFG::FixupPhase::fixupNode):
+        * dfg/DFGHeapLocation.cpp:
+        (WTF::printInternal):
+        * dfg/DFGHeapLocation.h:
+        * dfg/DFGNodeType.h:
+        * dfg/DFGPredictionPropagationPhase.cpp:
+        (JSC::DFG::PredictionPropagationPhase::propagate):
+        * dfg/DFGSafeToExecute.h:
+        (JSC::DFG::safeToExecute):
+        * dfg/DFGSpeculativeJIT32_64.cpp:
+        (JSC::DFG::SpeculativeJIT::compile):
+        * dfg/DFGSpeculativeJIT64.cpp:
+        (JSC::DFG::SpeculativeJIT::compile):
+        * jit/JITPropertyAccess.cpp:
+        (JSC::JIT::emitResolveClosure):
+        * llint/LowLevelInterpreter32_64.asm:
+        * llint/LowLevelInterpreter64.asm:
+        * runtime/JSScope.cpp:
+        (JSC::JSScope::abstractResolve):
+        * runtime/JSScope.h:
+
 2014-08-20  Michael Saboff  <[email protected]>
 
         REGRESSION: Web Inspector crashes when reloading apple.com with Timeline recording active

Modified: trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp (172807 => 172808)


--- trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp	2014-08-20 20:28:24 UTC (rev 172807)
+++ trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp	2014-08-20 20:47:45 UTC (rev 172808)
@@ -1944,7 +1944,7 @@
             const Identifier& ident = identifier(pc[2].u.operand);
             ResolveType type = static_cast<ResolveType>(pc[3].u.operand);
 
-            ResolveOp op = JSScope::abstractResolve(m_globalObject->globalExec(), scope, ident, Get, type);
+            ResolveOp op = JSScope::abstractResolve(m_globalObject->globalExec(), needsActivation(), scope, ident, Get, type);
             instructions[i + 3].u.operand = op.type;
             instructions[i + 4].u.operand = op.depth;
             if (op.activation)
@@ -1961,7 +1961,7 @@
             // get_from_scope dst, scope, id, ResolveModeAndType, Structure, Operand
             const Identifier& ident = identifier(pc[3].u.operand);
             ResolveModeAndType modeAndType = ResolveModeAndType(pc[4].u.operand);
-            ResolveOp op = JSScope::abstractResolve(m_globalObject->globalExec(), scope, ident, Get, modeAndType.type());
+            ResolveOp op = JSScope::abstractResolve(m_globalObject->globalExec(), needsActivation(), scope, ident, Get, modeAndType.type());
 
             instructions[i + 4].u.operand = ResolveModeAndType(modeAndType.mode(), op.type).operand();
             if (op.type == GlobalVar || op.type == GlobalVarWithVarInjectionChecks)
@@ -1977,7 +1977,7 @@
             // put_to_scope scope, id, value, ResolveModeAndType, Structure, Operand
             const Identifier& ident = identifier(pc[2].u.operand);
             ResolveModeAndType modeAndType = ResolveModeAndType(pc[4].u.operand);
-            ResolveOp op = JSScope::abstractResolve(m_globalObject->globalExec(), scope, ident, Put, modeAndType.type());
+            ResolveOp op = JSScope::abstractResolve(m_globalObject->globalExec(), needsActivation(), scope, ident, Put, modeAndType.type());
 
             instructions[i + 4].u.operand = ResolveModeAndType(modeAndType.mode(), op.type).operand();
             if (op.type == GlobalVar || op.type == GlobalVarWithVarInjectionChecks)
@@ -2008,7 +2008,7 @@
             case ProfileTypesBytecodeGetFromScope: {
                 const Identifier& ident = identifier(pc[4].u.operand);
                 ResolveType type = static_cast<ResolveType>(pc[5].u.operand);
-                ResolveOp op = JSScope::abstractResolve(m_globalObject->globalExec(), scope, ident, (flag == ProfileTypesBytecodeGetFromScope ? Get : Put), type);
+                ResolveOp op = JSScope::abstractResolve(m_globalObject->globalExec(), needsActivation(), scope, ident, (flag == ProfileTypesBytecodeGetFromScope ? Get : Put), type);
 
                 // FIXME: handle other values for op.type here, and also consider what to do when we can't statically determine the globalID
                 // https://bugs.webkit.org/show_bug.cgi?id=135184

Modified: trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h (172807 => 172808)


--- trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h	2014-08-20 20:28:24 UTC (rev 172807)
+++ trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h	2014-08-20 20:47:45 UTC (rev 172808)
@@ -1386,7 +1386,6 @@
         
     case GetScope: // FIXME: We could get rid of these if we know that the JSFunction is a constant. https://bugs.webkit.org/show_bug.cgi?id=106202
     case GetMyScope:
-    case SkipTopScope:
         forNode(node).setType(SpecObjectOther);
         break;
 

Modified: trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp (172807 => 172808)


--- trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp	2014-08-20 20:28:24 UTC (rev 172807)
+++ trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp	2014-08-20 20:47:45 UTC (rev 172808)
@@ -198,7 +198,7 @@
         bool isDirect);
     void emitChecks(const ConstantStructureCheckVector&);
 
-    Node* getScope(bool skipTop, unsigned skipCount);
+    Node* getScope(unsigned skipCount);
     
     // Prepare to parse a block.
     void prepareToParseBlock();
@@ -2023,13 +2023,9 @@
     m_constants.resize(0);
 }
 
-Node* ByteCodeParser::getScope(bool skipTop, unsigned skipCount)
+Node* ByteCodeParser::getScope(unsigned skipCount)
 {
     Node* localBase = get(VirtualRegister(JSStack::ScopeChain));
-    if (skipTop) {
-        ASSERT(!inlineCallFrame());
-        localBase = addToGraph(SkipTopScope, localBase);
-    }
     for (unsigned n = skipCount; n--;)
         localBase = addToGraph(SkipScope, localBase);
     return localBase;
@@ -2929,8 +2925,7 @@
                     set(VirtualRegister(dst), weakJSConstant(activation));
                     break;
                 }
-                set(VirtualRegister(dst),
-                    getScope(m_inlineStackTop->m_codeBlock->needsActivation(), depth));
+                set(VirtualRegister(dst), getScope(depth));
                 break;
             }
             case Dynamic:

Modified: trunk/Source/_javascript_Core/dfg/DFGClobberize.h (172807 => 172808)


--- trunk/Source/_javascript_Core/dfg/DFGClobberize.h	2014-08-20 20:28:24 UTC (rev 172807)
+++ trunk/Source/_javascript_Core/dfg/DFGClobberize.h	2014-08-20 20:47:45 UTC (rev 172808)
@@ -748,11 +748,6 @@
             def(PureValue(node));
         return;
         
-    case SkipTopScope:
-        read(AbstractHeap(Variables, graph.activationRegister()));
-        def(HeapLocation(SkipTopScopeLoc, AbstractHeap(Variables, graph.activationRegister()), node->child1()), node);
-        return;
-        
     case GetClosureRegisters:
         read(JSVariableObject_registers);
         def(HeapLocation(ClosureRegistersLoc, JSVariableObject_registers, node->child1()), node);

Modified: trunk/Source/_javascript_Core/dfg/DFGDoesGC.cpp (172807 => 172808)


--- trunk/Source/_javascript_Core/dfg/DFGDoesGC.cpp	2014-08-20 20:28:24 UTC (rev 172807)
+++ trunk/Source/_javascript_Core/dfg/DFGDoesGC.cpp	2014-08-20 20:47:45 UTC (rev 172808)
@@ -96,7 +96,6 @@
     case CheckArray:
     case GetScope:
     case GetMyScope:
-    case SkipTopScope:
     case SkipScope:
     case GetClosureRegisters:
     case GetClosureVar:

Modified: trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp (172807 => 172808)


--- trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp	2014-08-20 20:28:24 UTC (rev 172807)
+++ trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp	2014-08-20 20:47:45 UTC (rev 172808)
@@ -853,7 +853,6 @@
         }
 
         case GetClosureRegisters:
-        case SkipTopScope:
         case SkipScope:
         case GetScope:
         case GetGetter:

Modified: trunk/Source/_javascript_Core/dfg/DFGHeapLocation.cpp (172807 => 172808)


--- trunk/Source/_javascript_Core/dfg/DFGHeapLocation.cpp	2014-08-20 20:28:24 UTC (rev 172807)
+++ trunk/Source/_javascript_Core/dfg/DFGHeapLocation.cpp	2014-08-20 20:47:45 UTC (rev 172808)
@@ -136,10 +136,6 @@
         out.print("NamedPropertyLoc");
         return;
         
-    case SkipTopScopeLoc:
-        out.print("SkipTopScopeLoc");
-        return;
-        
     case TypedArrayByteOffsetLoc:
         out.print("TypedArrayByteOffsetLoc");
         return;

Modified: trunk/Source/_javascript_Core/dfg/DFGHeapLocation.h (172807 => 172808)


--- trunk/Source/_javascript_Core/dfg/DFGHeapLocation.h	2014-08-20 20:28:24 UTC (rev 172807)
+++ trunk/Source/_javascript_Core/dfg/DFGHeapLocation.h	2014-08-20 20:47:45 UTC (rev 172808)
@@ -55,7 +55,6 @@
     MyArgumentsLengthLoc,
     NamedPropertyLoc,
     SetterLoc,
-    SkipTopScopeLoc,
     TypeOfLoc,
     TypedArrayByteOffsetLoc,
     VarInjectionWatchpointLoc,

Modified: trunk/Source/_javascript_Core/dfg/DFGNodeType.h (172807 => 172808)


--- trunk/Source/_javascript_Core/dfg/DFGNodeType.h	2014-08-20 20:28:24 UTC (rev 172807)
+++ trunk/Source/_javascript_Core/dfg/DFGNodeType.h	2014-08-20 20:47:45 UTC (rev 172808)
@@ -175,7 +175,6 @@
     macro(GetTypedArrayByteOffset, NodeResultInt32) \
     macro(GetScope, NodeResultJS) \
     macro(GetMyScope, NodeResultJS) \
-    macro(SkipTopScope, NodeResultJS) \
     macro(SkipScope, NodeResultJS) \
     macro(GetClosureRegisters, NodeResultStorage) \
     macro(GetClosureVar, NodeResultJS) \

Modified: trunk/Source/_javascript_Core/dfg/DFGPredictionPropagationPhase.cpp (172807 => 172808)


--- trunk/Source/_javascript_Core/dfg/DFGPredictionPropagationPhase.cpp	2014-08-20 20:28:24 UTC (rev 172807)
+++ trunk/Source/_javascript_Core/dfg/DFGPredictionPropagationPhase.cpp	2014-08-20 20:47:45 UTC (rev 172808)
@@ -460,7 +460,6 @@
         }
             
         case GetMyScope:
-        case SkipTopScope:
         case SkipScope: {
             changed |= setPrediction(SpecObjectOther);
             break;

Modified: trunk/Source/_javascript_Core/dfg/DFGSafeToExecute.h (172807 => 172808)


--- trunk/Source/_javascript_Core/dfg/DFGSafeToExecute.h	2014-08-20 20:28:24 UTC (rev 172807)
+++ trunk/Source/_javascript_Core/dfg/DFGSafeToExecute.h	2014-08-20 20:47:45 UTC (rev 172808)
@@ -166,7 +166,6 @@
     case ArrayifyToStructure:
     case GetScope:
     case GetMyScope:
-    case SkipTopScope:
     case SkipScope:
     case GetClosureRegisters:
     case GetClosureVar:

Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp (172807 => 172808)


--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp	2014-08-20 20:28:24 UTC (rev 172807)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp	2014-08-20 20:47:45 UTC (rev 172808)
@@ -3512,16 +3512,6 @@
         break;
     }
         
-    case SkipTopScope: {
-        SpeculateCellOperand scope(this, node->child1());
-        GPRTemporary result(this, Reuse, scope);
-        GPRReg resultGPR = result.gpr();
-        m_jit.move(scope.gpr(), resultGPR);
-        m_jit.loadPtr(JITCompiler::Address(resultGPR, JSScope::offsetOfNext()), resultGPR);
-        cellResult(resultGPR, node);
-        break;
-    }
-        
     case SkipScope: {
         SpeculateCellOperand scope(this, node->child1());
         GPRTemporary result(this, Reuse, scope);

Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp (172807 => 172808)


--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp	2014-08-20 20:28:24 UTC (rev 172807)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp	2014-08-20 20:47:45 UTC (rev 172808)
@@ -3622,16 +3622,6 @@
         break;
     }
         
-    case SkipTopScope: {
-        SpeculateCellOperand scope(this, node->child1());
-        GPRTemporary result(this, Reuse, scope);
-        GPRReg resultGPR = result.gpr();
-        m_jit.move(scope.gpr(), resultGPR);
-        m_jit.loadPtr(JITCompiler::Address(resultGPR, JSScope::offsetOfNext()), resultGPR);
-        cellResult(resultGPR, node);
-        break;
-    }
-        
     case SkipScope: {
         SpeculateCellOperand scope(this, node->child1());
         GPRTemporary result(this, Reuse, scope);

Modified: trunk/Source/_javascript_Core/jit/JITPropertyAccess.cpp (172807 => 172808)


--- trunk/Source/_javascript_Core/jit/JITPropertyAccess.cpp	2014-08-20 20:28:24 UTC (rev 172807)
+++ trunk/Source/_javascript_Core/jit/JITPropertyAccess.cpp	2014-08-20 20:47:45 UTC (rev 172808)
@@ -594,10 +594,6 @@
 {
     emitVarInjectionCheck(needsVarInjectionChecks);
     emitGetVirtualRegister(JSStack::ScopeChain, regT0);
-    if (m_codeBlock->needsActivation()) {
-        emitGetVirtualRegister(m_codeBlock->activationRegister(), regT1);
-        loadPtr(Address(regT0, JSScope::offsetOfNext()), regT0);
-    }
     for (unsigned i = 0; i < depth; ++i)
         loadPtr(Address(regT0, JSScope::offsetOfNext()), regT0);
     emitPutVirtualRegister(dst);

Modified: trunk/Source/_javascript_Core/llint/LowLevelInterpreter32_64.asm (172807 => 172808)


--- trunk/Source/_javascript_Core/llint/LowLevelInterpreter32_64.asm	2014-08-20 20:28:24 UTC (rev 172807)
+++ trunk/Source/_javascript_Core/llint/LowLevelInterpreter32_64.asm	2014-08-20 20:47:45 UTC (rev 172808)
@@ -2201,11 +2201,7 @@
 macro resolveScope()
     loadp CodeBlock[cfr], t0
     loadisFromInstruction(4, t2)
-    btbz CodeBlock::m_needsActivation[t0], .resolveScopeAfterActivationCheck
-    loadis CodeBlock::m_activationRegister[t0], t1
-    addi 1, t2
 
-.resolveScopeAfterActivationCheck:
     loadp ScopeChain[cfr], t0
     btiz t2, .resolveScopeLoopEnd
 

Modified: trunk/Source/_javascript_Core/llint/LowLevelInterpreter64.asm (172807 => 172808)


--- trunk/Source/_javascript_Core/llint/LowLevelInterpreter64.asm	2014-08-20 20:28:24 UTC (rev 172807)
+++ trunk/Source/_javascript_Core/llint/LowLevelInterpreter64.asm	2014-08-20 20:47:45 UTC (rev 172808)
@@ -2034,11 +2034,6 @@
 macro resolveScope()
     loadp CodeBlock[cfr], t0
     loadisFromInstruction(4, t2)
-    btbz CodeBlock::m_needsActivation[t0], .resolveScopeAfterActivationCheck
-    loadis CodeBlock::m_activationRegister[t0], t1
-    addi 1, t2
-
-.resolveScopeAfterActivationCheck:
     loadp ScopeChain[cfr], t0
     btiz t2, .resolveScopeLoopEnd
 

Modified: trunk/Source/_javascript_Core/runtime/JSScope.cpp (172807 => 172808)


--- trunk/Source/_javascript_Core/runtime/JSScope.cpp	2014-08-20 20:28:24 UTC (rev 172807)
+++ trunk/Source/_javascript_Core/runtime/JSScope.cpp	2014-08-20 20:47:45 UTC (rev 172808)
@@ -148,13 +148,13 @@
     }
 }
 
-ResolveOp JSScope::abstractResolve(ExecState* exec, JSScope* scope, const Identifier& ident, GetOrPut getOrPut, ResolveType unlinkedType)
+ResolveOp JSScope::abstractResolve(ExecState* exec, bool hasTopActivation, JSScope* scope, const Identifier& ident, GetOrPut getOrPut, ResolveType unlinkedType)
 {
     ResolveOp op(Dynamic, 0, 0, 0, 0, 0);
     if (unlinkedType == Dynamic)
         return op;
 
-    size_t depth = 0;
+    size_t depth = hasTopActivation ? 1 : 0;
     bool needsVarInjectionChecks = JSC::needsVarInjectionChecks(unlinkedType);
     for (; scope; scope = scope->next()) {
         if (abstractAccess(exec, scope, ident, getOrPut, depth, needsVarInjectionChecks, op))

Modified: trunk/Source/_javascript_Core/runtime/JSScope.h (172807 => 172808)


--- trunk/Source/_javascript_Core/runtime/JSScope.h	2014-08-20 20:28:24 UTC (rev 172807)
+++ trunk/Source/_javascript_Core/runtime/JSScope.h	2014-08-20 20:47:45 UTC (rev 172808)
@@ -153,7 +153,7 @@
     JS_EXPORT_PRIVATE static JSObject* objectAtScope(JSScope*);
 
     static JSValue resolve(ExecState*, JSScope*, const Identifier&);
-    static ResolveOp abstractResolve(ExecState*, JSScope*, const Identifier&, GetOrPut, ResolveType);
+    static ResolveOp abstractResolve(ExecState*, bool hasTopActivation, JSScope*, const Identifier&, GetOrPut, ResolveType);
 
     static void visitChildren(JSCell*, SlotVisitor&);
 
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to