Changeset 129297 in webkit for trunk/Source/JavaScriptCore/bytecompiler

Timestamp:

Sep 21, 2012, 11:34:59 PM (13 years ago)

Author:

[email protected]

Message:

Optimized closures that capture arguments
​https://bugs.webkit.org/show_bug.cgi?id=97358

Reviewed by Oliver Hunt.

Source/JavaScriptCore:

Previously, the activation object was responsible for capturing all
arguments in a way that was convenient for the arguments object. Now,
we move all captured variables into a contiguous region in the stack,
allocate an activation for exactly that size, and make the arguments
object responsible for knowing all the places to which arguments could
have moved.

This seems like the right tradeoff because

(a) Closures are common and long-lived, so we want them to be small.

(b) Our primary strategy for optimizing the arguments object is to make
it go away. If you're allocating arguments objects, you're already having
a bad time.

(c) It's common to use either the arguments object or named argument
closure, but not both.

• bytecode/CodeBlock.cpp:

(JSC::CodeBlock::dump):
(JSC::CodeBlock::CodeBlock):

• bytecode/CodeBlock.h:

(JSC::CodeBlock::argumentsRegister):
(JSC::CodeBlock::activationRegister):
(JSC::CodeBlock::isCaptured):
(JSC::CodeBlock::argumentIndexAfterCapture): m_numCapturedVars is gone
now -- we have an explicit range instead.

• bytecompiler/BytecodeGenerator.cpp:

(JSC::BytecodeGenerator::BytecodeGenerator): Move captured arguments
into the captured region of local variables for space efficiency. Record
precise data about where they moved for the sake of the arguments object.

Some of this data was previously wrong, but it didn't cause any problems
because the arguments weren't actually moving.

• dfg/DFGByteCodeParser.cpp:

(JSC::DFG::ByteCodeParser::flushArgumentsAndCapturedVariables): Don't
assume that captured vars are in any particular location -- always ask
the CodeBlock. This is better encapsulation.

(JSC::DFG::ByteCodeParser::parseCodeBlock):

• dfg/DFGSpeculativeJIT32_64.cpp:

(JSC::DFG::SpeculativeJIT::compile):

• dfg/DFGSpeculativeJIT64.cpp:

(JSC::DFG::SpeculativeJIT::compile): I rename things sometimes.

• runtime/Arguments.cpp:

(JSC::Arguments::tearOff): Account for a particularly nasty edge case.

(JSC::Arguments::didTearOffActivation): Don't allocate our slow arguments
data on tear-off. We need to allocate it eagerly instead, since we need
to know about displaced, captured arguments during access before tear-off.

• runtime/Arguments.h:

(JSC::Arguments::allocateSlowArguments):
(JSC::Arguments::argument): Tell our slow arguments array where all arguments
are, even if they are not captured. This simplifies some things, so we don't
have to account explicitly for the full matrix of (not torn off, torn off)

• (captured, not captured).

(JSC::Arguments::finishCreation): Allocate our slow arguments array eagerly
because we need to know about displaced, captured arguments during access
before tear-off.

• runtime/Executable.cpp:

(JSC::FunctionExecutable::FunctionExecutable):
(JSC::FunctionExecutable::compileForCallInternal):
(JSC::FunctionExecutable::compileForConstructInternal):

• runtime/Executable.h:

(JSC::FunctionExecutable::parameterCount):
(FunctionExecutable):

• runtime/JSActivation.cpp:

(JSC::JSActivation::visitChildren):

• runtime/JSActivation.h:

(JSActivation):
(JSC::JSActivation::create):
(JSC::JSActivation::JSActivation):
(JSC::JSActivation::registerOffset):
(JSC::JSActivation::tearOff):
(JSC::JSActivation::allocationSize):
(JSC::JSActivation::isValid): This is really the point of the patch. All
the pointer math in Activations basically boils away, since we always
copy a contiguous region of captured variables now.

• runtime/SymbolTable.h:

(JSC::SlowArgument::SlowArgument):
(SlowArgument):
(SharedSymbolTable):
(JSC::SharedSymbolTable::captureCount):
(JSC::SharedSymbolTable::SharedSymbolTable): AllOfTheThings capture mode
is gone now -- that's the point of the patch. indexIfCaptured gets renamed
to index because we always have an index, even if not captured. (The only
time when the index is meaningless is when we're Deleted.)

LayoutTests:

• fast/js/dfg-arguments-alias-activation-expected.txt:
• fast/js/dfg-arguments-alias-activation.html:

File:

• trunk/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp (modified) (7 diffs)

trunk/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp

@@ -305 +305 @@
     // FIXME: Move code that modifies the global object to Interpreter::execute.
     
-    codeBlock->m_numCapturedVars = codeBlock->m_numVars;
-    
     if (compilationKind == OptimizingCompilation)
         return;
@@ -393 +391 @@
     }
 
+    symbolTable->setCaptureStart(m_codeBlock->m_numVars);
+
     if (functionBody->usesArguments() || codeBlock->usesEval() || m_shouldEmitDebugHooks) { // May reify arguments object.
         RegisterID* unmodifiedArgumentsRegister = addVar(); // Anonymous, so it can't be modified by user code.
@@ -424 +424 @@
     }
 
-    bool mayReifyArgumentsObject = codeBlock->usesArguments() || codeBlock->usesEval() || m_shouldEmitDebugHooks;
+    bool shouldCaptureAllTheThings = m_shouldEmitDebugHooks || codeBlock->usesEval();
+
     bool capturesAnyArgumentByName = false;
-    if (functionBody->hasCapturedVariables()) {
+    Vector<RegisterID*> capturedArguments;
+    if (functionBody->hasCapturedVariables() || shouldCaptureAllTheThings) {
         FunctionParameters& parameters = *functionBody->parameters();
+        capturedArguments.resize(parameters.size());
         for (size_t i = 0; i < parameters.size(); ++i) {
-            if (!functionBody->captures(parameters[i]))
+            capturedArguments[i] = 0;
+            if (!functionBody->captures(parameters[i]) && !shouldCaptureAllTheThings)
                 continue;
             capturesAnyArgumentByName = true;
-            break;
-        }
-    }
-
-    if (mayReifyArgumentsObject || capturesAnyArgumentByName) {
-        symbolTable->setCaptureMode(SharedSymbolTable::AllOfTheThings);
-        symbolTable->setCaptureStart(-CallFrame::offsetFor(symbolTable->parameterCountIncludingThis()));
-    } else {
-        symbolTable->setCaptureMode(SharedSymbolTable::SomeOfTheThings);
-        symbolTable->setCaptureStart(m_codeBlock->m_numVars);
-    }
-
-    if (mayReifyArgumentsObject && capturesAnyArgumentByName) {
+            capturedArguments[i] = addVar();
+        }
+    }
+
+    if (capturesAnyArgumentByName && !codeBlock->isStrictMode()) {
         size_t parameterCount = symbolTable->parameterCount();
         OwnArrayPtr<SlowArgument> slowArguments = adoptArrayPtr(new SlowArgument[parameterCount]);
         for (size_t i = 0; i < parameterCount; ++i) {
+            if (!capturedArguments[i]) {
+                ASSERT(slowArguments[i].status == SlowArgument::Normal);
+                slowArguments[i].index = CallFrame::argumentOffset(i);
+                continue;
+            }
             slowArguments[i].status = SlowArgument::Captured;
-            slowArguments[i].indexIfCaptured = CallFrame::argumentOffset(i);
+            slowArguments[i].index = capturedArguments[i]->index();
         }
         symbolTable->setSlowArguments(slowArguments.release());
@@ -492 +493 @@
     }
 
-    codeBlock->m_numCapturedVars = codeBlock->m_numVars;
+    symbolTable->setCaptureEnd(codeBlock->m_numVars);
 
     m_firstLazyFunction = codeBlock->m_numVars;
@@ -519 +520 @@
     }
 
-    if (m_shouldEmitDebugHooks || codeBlock->usesEval())
-        codeBlock->m_numCapturedVars = codeBlock->m_numVars;
-
-    symbolTable->setCaptureEnd(codeBlock->m_numCapturedVars);
+    if (shouldCaptureAllTheThings)
+        symbolTable->setCaptureEnd(codeBlock->m_numVars);
 
     FunctionParameters& parameters = *functionBody->parameters();
@@ -532 +531 @@
     m_codeBlock->addParameter();
     
-    for (size_t i = 0; i < parameters.size(); ++i)
-        addParameter(parameters[i], nextParameterIndex--);
-
+    for (size_t i = 0; i < parameters.size(); ++i, --nextParameterIndex) {
+        int index = nextParameterIndex;
+        if (capturedArguments.size() && capturedArguments[i]) {
+            ASSERT((functionBody->hasCapturedVariables() && functionBody->captures(parameters[i])) || shouldCaptureAllTheThings);
+            index = capturedArguments[i]->index();
+            RegisterID original(nextParameterIndex);
+            emitMove(capturedArguments[i], &original);
+        }
+        addParameter(parameters[i], index);
+    }
     preserveLastVar();
 
@@ -604 +610 @@
         variables.append(*varStack[i].first);
     codeBlock->adoptVariables(variables);
-    codeBlock->m_numCapturedVars = codeBlock->m_numVars;
     preserveLastVar();
 }