Ignore:
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:
1 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp

    r129287 r129297  
    468468        for (unsigned argument = numArguments; argument-- > 1;)
    469469            flush(argumentToOperand(argument));
    470         for (unsigned local = m_inlineStackTop->m_codeBlock->m_numCapturedVars; local--;)
     470        for (int local = 0; local < m_inlineStackTop->m_codeBlock->m_numVars; ++local) {
     471            if (!m_inlineStackTop->m_codeBlock->isCaptured(local))
     472                continue;
    471473            flush(local);
     474        }
    472475    }
    473476
     
    32833286   
    32843287#if DFG_ENABLE(DEBUG_VERBOSE)
    3285     dataLog("Parsing code block %p. codeType = %s, numCapturedVars = %u, needsFullScopeChain = %s, needsActivation = %s, isStrictMode = %s\n",
     3288    dataLog("Parsing code block %p. codeType = %s, captureCount = %u, needsFullScopeChain = %s, needsActivation = %s, isStrictMode = %s\n",
    32863289            codeBlock,
    32873290            codeTypeToString(codeBlock->codeType()),
    3288             codeBlock->m_numCapturedVars,
     3291            codeBlock->symbolTable()->captureCount(),
    32893292            codeBlock->needsFullScopeChain()?"true":"false",
    32903293            codeBlock->ownerExecutable()->needsActivation()?"true":"false",
Note: See TracChangeset for help on using the changeset viewer.