Changeset 237254 in webkit for trunk/Source/JavaScriptCore/parser/Parser.h

Timestamp:

Oct 18, 2018, 6:04:22 AM (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):

• 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/parser/Parser.h (modified) (10 diffs)

trunk/Source/JavaScriptCore/parser/Parser.h

@@ -860 +860 @@
 };
 
-enum class ArgumentType {
-    Normal,
-    Spread
-};
+enum class ArgumentType { Normal, Spread };
+enum class ParsingContext { Program, FunctionConstructor, Eval };
 
 template <typename LexerType>
@@ -875 +873 @@
 
     template <class ParsedNode>
-    std::unique_ptr<ParsedNode> parse(ParserError&, const Identifier&, SourceParseMode);
+    std::unique_ptr<ParsedNode> parse(ParserError&, const Identifier&, SourceParseMode, ParsingContext, std::optional<int> functionConstructorParametersEndPosition = std::nullopt);
 
     JSTextPosition positionBeforeLastNewline() const { return m_lexer->positionBeforeLastNewline(); }
@@ -1313 +1311 @@
 
     Parser();
-    String parseInner(const Identifier&, SourceParseMode);
+
+    String parseInner(const Identifier&, SourceParseMode, ParsingContext, std::optional<int> functionConstructorParametersEndPosition = std::nullopt);
 
     void didFinishParsing(SourceElements*, DeclarationStacks::FunctionStack&&, VariableEnvironment&, UniquedStringImplPtrSet&&, CodeFeatures, int);
@@ -1525 +1524 @@
     template <class TreeBuilder> TreeSourceElements parseAsyncFunctionSourceElements(TreeBuilder&, SourceParseMode, bool isArrowFunctionBodyExpression, SourceElementsMode);
     template <class TreeBuilder> TreeSourceElements parseAsyncGeneratorFunctionSourceElements(TreeBuilder&, SourceParseMode, bool isArrowFunctionBodyExpression, SourceElementsMode);
+    template <class TreeBuilder> TreeSourceElements parseSingleFunction(TreeBuilder&, std::optional<int> functionConstructorParametersEndPosition);
     template <class TreeBuilder> TreeStatement parseStatementListItem(TreeBuilder&, const Identifier*& directive, unsigned* directiveLiteralLength);
     template <class TreeBuilder> TreeStatement parseStatement(TreeBuilder&, const Identifier*& directive, unsigned* directiveLiteralLength = 0);
     enum class ExportType { Exported, NotExported };
     template <class TreeBuilder> TreeStatement parseClassDeclaration(TreeBuilder&, ExportType = ExportType::NotExported, DeclarationDefaultContext = DeclarationDefaultContext::Standard);
-    template <class TreeBuilder> TreeStatement parseFunctionDeclaration(TreeBuilder&, ExportType = ExportType::NotExported, DeclarationDefaultContext = DeclarationDefaultContext::Standard);
+    template <class TreeBuilder> TreeStatement parseFunctionDeclaration(TreeBuilder&, ExportType = ExportType::NotExported, DeclarationDefaultContext = DeclarationDefaultContext::Standard, std::optional<int> functionConstructorParametersEndPosition = std::nullopt);
     template <class TreeBuilder> TreeStatement parseFunctionDeclarationStatement(TreeBuilder&, bool isAsync, bool parentAllowsFunctionDeclarationAsStatement);
-    template <class TreeBuilder> TreeStatement parseAsyncFunctionDeclaration(TreeBuilder&, ExportType = ExportType::NotExported, DeclarationDefaultContext = DeclarationDefaultContext::Standard);
+    template <class TreeBuilder> TreeStatement parseAsyncFunctionDeclaration(TreeBuilder&, ExportType = ExportType::NotExported, DeclarationDefaultContext = DeclarationDefaultContext::Standard, std::optional<int> functionConstructorParametersEndPosition = std::nullopt);
     template <class TreeBuilder> NEVER_INLINE bool maybeParseAsyncFunctionDeclarationStatement(TreeBuilder& context, TreeStatement& result, bool parentAllowsFunctionDeclarationAsStatement);
     template <class TreeBuilder> TreeStatement parseVariableDeclaration(TreeBuilder&, DeclarationType, ExportType = ExportType::NotExported);
@@ -1600 +1600 @@
 
     enum class FunctionDefinitionType { Expression, Declaration, Method };
-    template <class TreeBuilder> NEVER_INLINE bool parseFunctionInfo(TreeBuilder&, FunctionNameRequirements, SourceParseMode, bool nameIsInContainingScope, ConstructorKind, SuperBinding, int functionKeywordStart, ParserFunctionInfo<TreeBuilder>&, FunctionDefinitionType);
+    template <class TreeBuilder> NEVER_INLINE bool parseFunctionInfo(TreeBuilder&, FunctionNameRequirements, SourceParseMode, bool nameIsInContainingScope, ConstructorKind, SuperBinding, int functionKeywordStart, ParserFunctionInfo<TreeBuilder>&, FunctionDefinitionType, std::optional<int> functionConstructorParametersEndPosition = std::nullopt);
     
     ALWAYS_INLINE bool isArrowFunctionParameters();
@@ -1848 +1848 @@
 template <typename LexerType>
 template <class ParsedNode>
-std::unique_ptr<ParsedNode> Parser<LexerType>::parse(ParserError& error, const Identifier& calleeName, SourceParseMode parseMode)
+std::unique_ptr<ParsedNode> Parser<LexerType>::parse(ParserError& error, const Identifier& calleeName, SourceParseMode parseMode, ParsingContext parsingContext, std::optional<int> functionConstructorParametersEndPosition)
 {
     int errLine;
@@ -1865 +1865 @@
     unsigned startColumn = m_source->startColumn().zeroBasedInt();
 
-    String parseError = parseInner(calleeName, parseMode);
+    String parseError = parseInner(calleeName, parseMode, parsingContext, functionConstructorParametersEndPosition);
 
     int lineNumber = m_lexer->lineNumber();
@@ -1960 +1960 @@
     if (source.provider()->source().is8Bit()) {
         Parser<Lexer<LChar>> parser(vm, source, builtinMode, strictMode, scriptMode, parseMode, superBinding, defaultConstructorKind, derivedContextType, isEvalNode<ParsedNode>(), evalContextType, debuggerParseData);
-        result = parser.parse<ParsedNode>(error, name, parseMode);
+        result = parser.parse<ParsedNode>(error, name, parseMode, isEvalNode<ParsedNode>() ? ParsingContext::Eval : ParsingContext::Program);
         if (positionBeforeLastNewline)
             *positionBeforeLastNewline = parser.positionBeforeLastNewline();
@@ -1973 +1973 @@
         ASSERT_WITH_MESSAGE(defaultConstructorKind == ConstructorKind::None, "BuiltinExecutables::createDefaultConstructor should always use a 8-bit string");
         Parser<Lexer<UChar>> parser(vm, source, builtinMode, strictMode, scriptMode, parseMode, superBinding, defaultConstructorKind, derivedContextType, isEvalNode<ParsedNode>(), evalContextType, debuggerParseData);
-        result = parser.parse<ParsedNode>(error, name, parseMode);
+        result = parser.parse<ParsedNode>(error, name, parseMode, isEvalNode<ParsedNode>() ? ParsingContext::Eval : ParsingContext::Program);
         if (positionBeforeLastNewline)
             *positionBeforeLastNewline = parser.positionBeforeLastNewline();
@@ -1987 +1987 @@
 }
 
+inline std::unique_ptr<ProgramNode> parseFunctionForFunctionConstructor(VM& vm, const SourceCode& source, ParserError& error, JSTextPosition* positionBeforeLastNewline, std::optional<int> functionConstructorParametersEndPosition)
+{
+    ASSERT(!source.provider()->source().isNull());
+
+    MonotonicTime before;
+    if (UNLIKELY(Options::reportParseTimes()))
+        before = MonotonicTime::now();
+
+    Identifier name;
+    bool isEvalNode = false;
+    std::unique_ptr<ProgramNode> result;
+    if (source.provider()->source().is8Bit()) {
+        Parser<Lexer<LChar>> parser(&vm, source, JSParserBuiltinMode::NotBuiltin, JSParserStrictMode::NotStrict, JSParserScriptMode::Classic, SourceParseMode::ProgramMode, SuperBinding::NotNeeded, ConstructorKind::None, DerivedContextType::None, isEvalNode, EvalContextType::None, nullptr);
+        result = parser.parse<ProgramNode>(error, name, SourceParseMode::ProgramMode, ParsingContext::FunctionConstructor, functionConstructorParametersEndPosition);
+        if (positionBeforeLastNewline)
+            *positionBeforeLastNewline = parser.positionBeforeLastNewline();
+    } else {
+        Parser<Lexer<UChar>> parser(&vm, source, JSParserBuiltinMode::NotBuiltin, JSParserStrictMode::NotStrict, JSParserScriptMode::Classic, SourceParseMode::ProgramMode, SuperBinding::NotNeeded, ConstructorKind::None, DerivedContextType::None, isEvalNode, EvalContextType::None, nullptr);
+        result = parser.parse<ProgramNode>(error, name, SourceParseMode::ProgramMode, ParsingContext::FunctionConstructor, functionConstructorParametersEndPosition);
+        if (positionBeforeLastNewline)
+            *positionBeforeLastNewline = parser.positionBeforeLastNewline();
+    }
+
+    if (UNLIKELY(Options::reportParseTimes())) {
+        MonotonicTime after = MonotonicTime::now();
+        ParseHash hash(source);
+        dataLogLn(result ? "Parsed #" : "Failed to parse #", hash.hashForCall(), "/#", hash.hashForConstruct(), " in ", (after - before).milliseconds(), " ms.");
+    }
+
+    return result;
+}
+
+
 } // namespace