Changeset 237054 in webkit for trunk/Source/JavaScriptCore/runtime/CodeCache.cpp

Timestamp:

Oct 11, 2018, 4:43:58 PM (7 years ago)

Author:

[email protected]

Message:

[JSC] JSC should have "parseFunction" to optimize Function constructor
​https://bugs.webkit.org/show_bug.cgi?id=190340

Reviewed by Mark Lam.

JSTests:

This patch fixes the line number of syntax errors raised by the Function constructor,
since we now parse the final code only once. And we no longer use block statement
for Function constructor's parsing.

• ChakraCore/test/Function/FuncBodyES5.baseline-jsc:
• stress/function-cache-with-parameters-end-position.js: Added.

(shouldBe):
(shouldThrow):
(i.anonymous):

• stress/function-constructor-name.js: Added.

(shouldBe):
(GeneratorFunction):
(AsyncFunction.async):
(AsyncGeneratorFunction.async):
(anonymous):
(async.anonymous):

• test262/expectations.yaml:

LayoutTests/imported/w3c:

• web-platform-tests/html/webappapis/scripting/events/inline-event-handler-ordering-expected.txt:
• web-platform-tests/html/webappapis/scripting/events/invalid-uncompiled-raw-handler-compiled-late-expected.txt:
• web-platform-tests/html/webappapis/scripting/processing-model-2/compile-error-in-attribute-expected.txt:
• web-platform-tests/html/webappapis/scripting/processing-model-2/compile-error-in-body-onerror-expected.txt:

Source/JavaScriptCore:

The current Function constructor is suboptimal. We parse the piece of the same code three times to meet
the spec requirement. (1) check parameters syntax, (2) check body syntax, and (3) parse the entire function.
And to parse 1-3 correctly, we create two strings, the parameters and the entire function. This operation
is really costly and ideally we should meet the above requirement by the one time parsing.

To meet the above requirement, we add a special function for Parser, parseSingleFunction. This function
takes std::optional<int> functionConstructorParametersEndPosition and check this end position is correct in the parser.
For example, if we run the code,

Function('/*', '*/){')

According to the spec, this should produce '/*' parameter string and '*/){' body string. And parameter
string should be syntax-checked by the parser, and raise the error since it is incorrect. Instead of doing
that, in our implementation, we first create the entire string.

function anonymous(/*) {

*/){

}

And we parse it. At that time, we also pass the end position of the parameters to the parser. In the above case,
the position of the `function anonymous(/*)' <> is passed. And in the parser, we check that the last token
offset of the parameters is the given end position. This check allows us to raise the error correctly to the
above example while we parse the entire function only once. And we do not need to create two strings too.

This improves the performance of the Function constructor significantly. And web-tooling-benchmark/uglify-js is
significantly sped up (28.2%).

Before:

uglify-js: 2.94 runs/s

After:

uglify-js: 3.77 runs/s

• bytecode/UnlinkedFunctionExecutable.cpp:

(JSC::UnlinkedFunctionExecutable::fromGlobalCode):

• bytecode/UnlinkedFunctionExecutable.h:
• parser/Parser.cpp:

(JSC::Parser<LexerType>::parseInner):
(JSC::Parser<LexerType>::parseSingleFunction):
(JSC::Parser<LexerType>::parseFunctionInfo):
(JSC::Parser<LexerType>::parseFunctionDeclaration):
(JSC::Parser<LexerType>::parseAsyncFunctionDeclaration):
(JSC::Parser<LexerType>::parseClass):
(JSC::Parser<LexerType>::parsePropertyMethod):
(JSC::Parser<LexerType>::parseGetterSetter):
(JSC::Parser<LexerType>::parseFunctionExpression):
(JSC::Parser<LexerType>::parseAsyncFunctionExpression):
(JSC::Parser<LexerType>::parseArrowFunctionExpression):

• parser/Parser.h:

(JSC::Parser<LexerType>::parse):
(JSC::parse):
(JSC::parseFunctionForFunctionConstructor):

• parser/ParserModes.h:
• parser/ParserTokens.h:

(JSC::JSTextPosition::JSTextPosition):
(JSC::JSTokenLocation::JSTokenLocation): Deleted.

• parser/SourceCodeKey.h:

(JSC::SourceCodeKey::SourceCodeKey):
(JSC::SourceCodeKey::operator== const):

• runtime/CodeCache.cpp:

(JSC::CodeCache::getUnlinkedGlobalCodeBlock):
(JSC::CodeCache::getUnlinkedGlobalFunctionExecutable):

• runtime/CodeCache.h:
• runtime/FunctionConstructor.cpp:

(JSC::constructFunctionSkippingEvalEnabledCheck):

• runtime/FunctionExecutable.cpp:

(JSC::FunctionExecutable::fromGlobalCode):

• runtime/FunctionExecutable.h:

LayoutTests:

• fast/dom/attribute-event-listener-errors-expected.txt:
• fast/events/attribute-listener-deletion-crash-expected.txt:
• fast/events/window-onerror-syntax-error-in-attr-expected.txt:
• js/dom/invalid-syntax-for-function-expected.txt:
• js/dom/script-start-end-locations-expected.txt:

File:

• trunk/Source/JavaScriptCore/runtime/CodeCache.cpp (modified) (5 diffs)

trunk/Source/JavaScriptCore/runtime/CodeCache.cpp

@@ -58 +58 @@
         derivedContextType, evalContextType, isArrowFunctionContext, debuggerMode, 
         vm.typeProfiler() ? TypeProfilerEnabled::Yes : TypeProfilerEnabled::No, 
-        vm.controlFlowProfiler() ? ControlFlowProfilerEnabled::Yes : ControlFlowProfilerEnabled::No);
+        vm.controlFlowProfiler() ? ControlFlowProfilerEnabled::Yes : ControlFlowProfilerEnabled::No,
+        std::nullopt);
     SourceCodeValue* cache = m_sourceCode.findCacheAndUpdateAge(key);
     if (cache && Options::useCodeCache()) {
@@ -96 +97 @@
 }
 
-UnlinkedFunctionExecutable* CodeCache::getUnlinkedGlobalFunctionExecutable(VM& vm, const Identifier& name, const SourceCode& source, DebuggerMode debuggerMode, ParserError& error)
+UnlinkedFunctionExecutable* CodeCache::getUnlinkedGlobalFunctionExecutable(VM& vm, const Identifier& name, const SourceCode& source, DebuggerMode debuggerMode, std::optional<int> functionConstructorParametersEndPosition, ParserError& error)
 {
     bool isArrowFunctionContext = false;
@@ -108 +109 @@
         debuggerMode, 
         vm.typeProfiler() ? TypeProfilerEnabled::Yes : TypeProfilerEnabled::No, 
-        vm.controlFlowProfiler() ? ControlFlowProfilerEnabled::Yes : ControlFlowProfilerEnabled::No);
+        vm.controlFlowProfiler() ? ControlFlowProfilerEnabled::Yes : ControlFlowProfilerEnabled::No,
+        functionConstructorParametersEndPosition);
     SourceCodeValue* cache = m_sourceCode.findCacheAndUpdateAge(key);
     if (cache && Options::useCodeCache()) {
@@ -118 +120 @@
 
     JSTextPosition positionBeforeLastNewline;
-    std::unique_ptr<ProgramNode> program = parse<ProgramNode>(
-        &vm, source, Identifier(), JSParserBuiltinMode::NotBuiltin,
-        JSParserStrictMode::NotStrict, JSParserScriptMode::Classic, SourceParseMode::ProgramMode, SuperBinding::NotNeeded,
-        error, &positionBeforeLastNewline);
+    std::unique_ptr<ProgramNode> program = parseFunctionForFunctionConstructor(vm, source, error, &positionBeforeLastNewline, functionConstructorParametersEndPosition);
     if (!program) {
         RELEASE_ASSERT(error.isValid());
@@ -128 +127 @@
 
     // This function assumes an input string that would result in a single function declaration.
-    StatementNode* statement = program->singleStatement();
-    if (UNLIKELY(!statement)) {
-        JSToken token;
-        error = ParserError(ParserError::SyntaxError, ParserError::SyntaxErrorIrrecoverable, token, "Parser error", -1);
-        return nullptr;
-    }
-    ASSERT(statement->isBlock());
-
-    StatementNode* funcDecl = static_cast<BlockNode*>(statement)->singleStatement();
+    StatementNode* funcDecl = program->singleStatement();
     if (UNLIKELY(!funcDecl)) {
         JSToken token;