[Parser] Remove aborting of preparsing for trivial long functions.

Real world websites don't benifit from aborting preparsing to eagerly compile
long trivial functions, and it adds unecessary complexity to the parser and
doesn't work well with bytecode flushing, so we remove it.

Perf Sheriffs: this is expected to regress the MandreelLatency benchmark on
Octane.

BUG=v8:8395

Change-Id: Ia60cd67d4dd100376d2a366939a1d2a97cbc2b0d
Reviewed-on: https://chromium-review.googlesource.com/c/1394297
Commit-Queue: Ross McIlroy <rmcilroy@chromium.org>
Reviewed-by: Marja Hölttä <marja@chromium.org>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58568}

src/parsing/parser-base.h

@@ -336,8 +336,6 @@ class ParserBase {
   friend class v8::internal::ExpressionParsingScope<ParserTypes<Impl>>;
   friend class v8::internal::ArrowHeadParsingScope<ParserTypes<Impl>>;
 
-  enum LazyParsingResult { kLazyParsingComplete, kLazyParsingAborted };
-
   enum VariableDeclarationContext {
     kStatementListItem,
     kStatement,
@@ -1082,26 +1080,12 @@ class ParserBase {
                          FunctionLiteral::FunctionType function_type,
                          FunctionBodyType body_type);
 
-  // Under some circumstances, we allow preparsing to abort if the preparsed
-  // function is "long and trivial", and fully parse instead. Our current
-  // definition of "long and trivial" is:
-  // - over kLazyParseTrialLimit statements
-  // - all starting with an identifier (i.e., no if, for, while, etc.)
-  static const int kLazyParseTrialLimit = 200;
-
   // TODO(nikolaos, marja): The first argument should not really be passed
   // by value. The method is expected to add the parsed statements to the
   // list. This works because in the case of the parser, StatementListT is
   // a pointer whereas the preparser does not really modify the body.
   V8_INLINE void ParseStatementList(StatementListT* body,
-                                    Token::Value end_token) {
-    LazyParsingResult result = ParseStatementList(body, end_token, false);
-    USE(result);
-    DCHECK_EQ(result, kLazyParsingComplete);
-  }
-  V8_INLINE LazyParsingResult ParseStatementList(StatementListT* body,
-                                                 Token::Value end_token,
-                                                 bool may_abort);
+                                    Token::Value end_token);
   StatementT ParseStatementListItem();
 
   StatementT ParseStatement(ZonePtrList<const AstRawString>* labels,
@@ -4057,11 +4041,10 @@ ParserBase<Impl>::ParseArrowFunctionLiteral(
         // parameters.
         int dummy_num_parameters = -1;
         DCHECK_NE(kind & FunctionKind::kArrowFunction, 0);
-        FunctionLiteral::EagerCompileHint hint;
         bool did_preparse_successfully = impl()->SkipFunction(
             nullptr, kind, FunctionLiteral::kAnonymousExpression,
             formal_parameters.scope, &dummy_num_parameters,
-            &produced_preparsed_scope_data, false, &hint);
+            &produced_preparsed_scope_data);
 
         DCHECK_NULL(produced_preparsed_scope_data);
 
@@ -4497,9 +4480,8 @@ typename ParserBase<Impl>::ExpressionT ParserBase<Impl>::ParseV8Intrinsic() {
 }
 
 template <typename Impl>
-typename ParserBase<Impl>::LazyParsingResult
-ParserBase<Impl>::ParseStatementList(StatementListT* body,
-                                     Token::Value end_token, bool may_abort) {
+void ParserBase<Impl>::ParseStatementList(StatementListT* body,
+                                          Token::Value end_token) {
   // StatementList ::
   //   (StatementListItem)* <end_token>
   DCHECK_NOT_NULL(body);
@@ -4517,10 +4499,9 @@ ParserBase<Impl>::ParseStatementList(StatementListT* body,
     }
 
     StatementT stat = ParseStatementListItem();
-    if (impl()->IsNull(stat)) return kLazyParsingComplete;
+    if (impl()->IsNull(stat)) return;
 
     body->Add(stat);
-    may_abort = false;
 
     if (!impl()->IsStringLiteral(stat)) break;
 
@@ -4534,7 +4515,7 @@ ParserBase<Impl>::ParseStatementList(StatementListT* body,
         impl()->ReportMessageAt(token_loc,
                                 MessageTemplate::kIllegalLanguageModeDirective,
                                 "use strict");
-        return kLazyParsingComplete;
+        return;
       }
     } else if (use_asm) {
       // Directive "use asm".
@@ -4551,28 +4532,12 @@ ParserBase<Impl>::ParseStatementList(StatementListT* body,
   // all scripts and functions get their own target stack thus avoiding illegal
   // breaks and continues across functions.
   TargetScopeT target_scope(this);
-  int count_statements = 0;
-
-  if (may_abort) {
-    while (peek() == Token::IDENTIFIER) {
-      StatementT stat = ParseStatementListItem();
-      // If we're allowed to abort, we will do so when we see a "long and
-      // trivial" function. Our current definition of "long and trivial" is:
-      // - over kLazyParseTrialLimit statements
-      // - all starting with an identifier (i.e., no if, for, while, etc.)
-      if (++count_statements > kLazyParseTrialLimit) return kLazyParsingAborted;
-      body->Add(stat);
-    }
-  }
-
   while (peek() != end_token) {
     StatementT stat = ParseStatementListItem();
-    if (impl()->IsNull(stat)) return kLazyParsingComplete;
+    if (impl()->IsNull(stat)) return;
     if (stat->IsEmptyStatement()) continue;
     body->Add(stat);
   }
-
-  return kLazyParsingComplete;
 }
 
 template <typename Impl>

src/parsing/parser.cc

@@ -2585,8 +2585,7 @@ FunctionLiteral* Parser::ParseFunctionLiteral(
   bool did_preparse_successfully =
       should_preparse &&
       SkipFunction(function_name, kind, function_type, scope, &num_parameters,
-                   &produced_preparsed_scope_data, is_lazy_top_level_function,
-                   &eager_compile_hint);
+                   &produced_preparsed_scope_data);
   if (!did_preparse_successfully) {
     // If skipping aborted, it rewound the scanner until before the LPAREN.
     // Consume it in that case.
@@ -2658,8 +2657,7 @@ bool Parser::SkipFunction(
     const AstRawString* function_name, FunctionKind kind,
     FunctionLiteral::FunctionType function_type,
     DeclarationScope* function_scope, int* num_parameters,
-    ProducedPreParsedScopeData** produced_preparsed_scope_data, bool may_abort,
-    FunctionLiteral::EagerCompileHint* hint) {
+    ProducedPreParsedScopeData** produced_preparsed_scope_data) {
   FunctionState function_state(&function_state_, &scope_, function_scope);
   function_scope->set_zone(&preparser_zone_);
 
@@ -2706,16 +2704,8 @@ bool Parser::SkipFunction(
   TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("v8.compile"), "V8.PreParse");
 
   PreParser::PreParseResult result = reusable_preparser()->PreParseFunction(
-      function_name, kind, function_type, function_scope, may_abort,
-      use_counts_, produced_preparsed_scope_data, this->script_id());
-
-  // Return immediately if pre-parser decided to abort parsing.
-  if (result == PreParser::kPreParseAbort) {
-    bookmark.Apply();
-    function_scope->ResetAfterPreparsing(ast_value_factory(), true);
-    *hint = FunctionLiteral::kShouldEagerCompile;
-    return false;
-  }
+      function_name, kind, function_type, function_scope, use_counts_,
+      produced_preparsed_scope_data, this->script_id());
 
   if (result == PreParser::kPreParseStackOverflow) {
     // Propagate stack overflow.

src/parsing/parser.h

@@ -465,8 +465,6 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
 
   // Skip over a lazy function, either using cached data if we have it, or
   // by parsing the function with PreParser. Consumes the ending }.
-  // If may_abort == true, the (pre-)parser may decide to abort skipping
-  // in order to force the function to be eagerly parsed, after all.
   // In case the preparser detects an error it cannot identify, it resets the
   // scanner- and preparser state to the initial one, before PreParsing the
   // function.
@@ -477,8 +475,7 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
   bool SkipFunction(const AstRawString* function_name, FunctionKind kind,
                     FunctionLiteral::FunctionType function_type,
                     DeclarationScope* function_scope, int* num_parameters,
-                    ProducedPreParsedScopeData** produced_preparsed_scope_data,
-                    bool may_abort, FunctionLiteral::EagerCompileHint* hint);
+                    ProducedPreParsedScopeData** produced_preparsed_scope_data);
 
   Block* BuildParameterInitializationBlock(
       const ParserFormalParameters& parameters);

src/parsing/preparser.cc

@@ -107,7 +107,7 @@ void PreParserFormalParameters::ValidateStrictMode(PreParser* preparser) const {
 PreParser::PreParseResult PreParser::PreParseFunction(
     const AstRawString* function_name, FunctionKind kind,
     FunctionLiteral::FunctionType function_type,
-    DeclarationScope* function_scope, bool may_abort, int* use_counts,
+    DeclarationScope* function_scope, int* use_counts,
     ProducedPreParsedScopeData** produced_preparsed_scope_data, int script_id) {
   DCHECK_EQ(FUNCTION_SCOPE, function_scope->scope_type());
   use_counts_ = use_counts;
@@ -161,7 +161,6 @@ PreParser::PreParseResult PreParser::PreParseFunction(
 
   Expect(Token::LBRACE);
   DeclarationScope* inner_scope = function_scope;
-  LazyParsingResult result;
 
   if (!formals.is_simple) {
     inner_scope = NewVarblockScope();
@@ -170,7 +169,7 @@ PreParser::PreParseResult PreParser::PreParseFunction(
 
   {
     BlockState block_state(&scope_, inner_scope);
-    result = ParseStatementListAndLogFunction(&formals, may_abort);
+    ParseStatementListAndLogFunction(&formals);
   }
 
   bool allow_duplicate_parameters = false;
@@ -205,12 +204,8 @@ PreParser::PreParseResult PreParser::PreParseFunction(
     return kPreParseNotIdentifiableError;
   } else if (has_error()) {
     DCHECK(pending_error_handler()->has_pending_error());
-  } else if (result == kLazyParsingAborted) {
-    DCHECK(!pending_error_handler()->has_error_unidentifiable_by_preparser());
-    return kPreParseAbort;
   } else {
     DCHECK_EQ(Token::RBRACE, scanner()->peek());
-    DCHECK(result == kLazyParsingComplete);
 
     if (!IsArrowFunction(kind)) {
       // Validate parameter names. We can do this only after parsing the
@@ -372,12 +367,10 @@ PreParser::Expression PreParser::ParseFunctionLiteral(
   return Expression::Default();
 }
 
-PreParser::LazyParsingResult PreParser::ParseStatementListAndLogFunction(
-    PreParserFormalParameters* formals, bool may_abort) {
+void PreParser::ParseStatementListAndLogFunction(
+    PreParserFormalParameters* formals) {
   PreParserScopedStatementList body(pointer_buffer());
-  LazyParsingResult result =
-      ParseStatementList(&body, Token::RBRACE, may_abort);
-  if (result == kLazyParsingAborted) return result;
+  ParseStatementList(&body, Token::RBRACE);
 
   // Position right after terminal '}'.
   DCHECK_IMPLIES(!has_error(), scanner()->peek() == Token::RBRACE);
@@ -385,7 +378,6 @@ PreParser::LazyParsingResult PreParser::ParseStatementListAndLogFunction(
   DCHECK_EQ(this->scope()->is_function_scope(), formals->is_simple);
   log_.LogFunction(body_end, formals->num_parameters(),
                    GetLastFunctionLiteralId());
-  return kLazyParsingComplete;
 }
 
 PreParserBlock PreParser::BuildParameterInitializationBlock(

src/parsing/preparser.h

@@ -991,7 +991,6 @@ class PreParser : public ParserBase<PreParser> {
 
   enum PreParseResult {
     kPreParseStackOverflow,
-    kPreParseAbort,
     kPreParseNotIdentifiableError,
     kPreParseSuccess
   };
@@ -1030,7 +1029,7 @@ class PreParser : public ParserBase<PreParser> {
   PreParseResult PreParseFunction(
       const AstRawString* function_name, FunctionKind kind,
       FunctionLiteral::FunctionType function_type,
-      DeclarationScope* function_scope, bool may_abort, int* use_counts,
+      DeclarationScope* function_scope, int* use_counts,
       ProducedPreParsedScopeData** produced_preparser_scope_data,
       int script_id);
 
@@ -1069,8 +1068,7 @@ class PreParser : public ParserBase<PreParser> {
       const AstRawString* name, FunctionKind kind,
       FunctionLiteral::FunctionType function_type,
       DeclarationScope* function_scope, int* num_parameters,
-      ProducedPreParsedScopeData** produced_preparsed_scope_data,
-      bool may_abort, FunctionLiteral::EagerCompileHint* hint) {
+      ProducedPreParsedScopeData** produced_preparsed_scope_data) {
     UNREACHABLE();
   }
 
@@ -1085,8 +1083,7 @@ class PreParser : public ParserBase<PreParser> {
     return literal;
   }
 
-  LazyParsingResult ParseStatementListAndLogFunction(
-      PreParserFormalParameters* formals, bool maybe_abort);
+  void ParseStatementListAndLogFunction(PreParserFormalParameters* formals);
 
   struct TemplateLiteralState {};