Named functions should not allocate scope objects for their names
https://bugs.webkit.org/show_bug.cgi?id=95659
Reviewed by Oliver Hunt.
Source/JavaScriptCore:
In most cases, we can merge a function expression's name into its symbol
table. This reduces memory footprint per closure from three objects
(function + activation + name scope) to two (function + activation),
speeds up closure allocation, and speeds up recursive calls.
In the case of a named function expression that contains a non-strict
eval, the rules are so bat-poop crazy that I don't know how to model
them without an extra object. Since functions now default to not having
such an object, this case needs to allocate the object on function
entry.
Therefore, this patch makes the slow case a bit slower so the fast case
can be faster and more memory-efficient. (Note that the slow case already
allocates an activation on entry, and until recently also allocated a
scope chain node on entry, so adding one allocation on entry shouldn't
break the bank.)
(JSC::CodeBlock::CodeBlock): Caught a missed initializer. No behavior change.
- bytecompiler/BytecodeGenerator.cpp:
(JSC::BytecodeGenerator::BytecodeGenerator): Put the callee in static scope
during compilation so it doesn't need to be in dynamic scope at runtime.
(JSC::BytecodeGenerator::resolveCallee):
(JSC::BytecodeGenerator::addCallee): Helper functions for either statically
resolving the callee or adding a dynamic scope that will resolve to it,
depending on whether you're in the fast path.
We move the callee into a var location if it's captured because activations
prefer to have contiguous ranges of captured variables.
- bytecompiler/BytecodeGenerator.h:
(JSC::BytecodeGenerator::registerFor):
(BytecodeGenerator):
- dfg/DFGOperations.cpp:
- interpreter/Interpreter.cpp:
(JSC::Interpreter::privateExecute):
(JSC::DEFINE_STUB_FUNCTION):
- llint/LLIntSlowPaths.cpp:
(JSC::LLInt::LLINT_SLOW_PATH_DECL): This is the point of the patch: remove
one allocation in the case of a named function expression.
(JSC::::Parser):
(JSC::Scope::declareCallee):
(Scope):
(Parser):
(JSC::parse):
(JSC::EvalExecutable::compileInternal):
(JSC::ProgramExecutable::checkSyntax):
(JSC::ProgramExecutable::compileInternal):
(JSC::FunctionExecutable::produceCodeBlockFor):
(JSC::FunctionExecutable::fromGlobalCode): Pipe the callee's name through
the parser so we get accurate information on whether the callee was captured.
(JSC::FunctionExecutable::FunctionExecutable):
(JSC::EvalExecutable::compileInternal):
(JSC::ProgramExecutable::checkSyntax):
(JSC::ProgramExecutable::compileInternal):
(JSC::FunctionExecutable::produceCodeBlockFor):
(JSC::FunctionExecutable::fromGlobalCode):
(JSC::FunctionExecutable::create):
(FunctionExecutable):
(JSC::FunctionExecutable::finishCreation): I had to refactor function
creation to support the following function constructor quirk: the function
gets a name, but its name is not in lexical scope.
To simplify this, FunctionExecutable now automatically extracts all the
data it needs from the parsed node. The special "fromGlobalCode" path
used by the function constructor creates an anonymous function, and then
quirkily sets the value used by the .name property to be non-null, even
though the parsed name is null.
(JSC::JSNameScope::create):
(JSC::JSNameScope::JSNameScope): Added support for explicitly specifying
your container scope. The compiler uses this for named function expressions.
LayoutTests:
Added coverage for some extra-tricky cases.
- fast/js/named-function-expression-expected.txt:
- fast/js/script-tests/named-function-expression.js:
(shouldBeTrueWithDescription): I rolled my own shouldBeTrue() here to avoid the
built-in shouldBe()'s eval scoping, which can change the variable
resolution rules I'm trying to test.
- inspector/debugger/debugger-expand-scope-expected.txt: Not sure why this
result used to miss the function name scope, but the new result is a
progression, so I've updated the expected results.
