[parser] Allocate preparsed scope members in preparse zone

Before I gave the preparser its own zone this was the case. I somewhat
accidentally dropped it when I used set_zone instead; causing large regressions
for certain types of pages.

Bug: chromium:889086
Change-Id: Ib3cf1f926b5c65506c66a97981c4544dccb372aa
Reviewed-on: https://chromium-review.googlesource.com/1245767
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Commit-Queue: Igor Sheludko <ishell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56243}

src/ast/scopes.cc

@@ -1492,6 +1492,9 @@ void DeclarationScope::ResetAfterPreparsing(AstValueFactory* ast_value_factory,
   rare_data_ = nullptr;
   has_rest_ = false;
 
+  DCHECK_NE(zone_, ast_value_factory->zone());
+  zone_->ReleaseMemory();
+
   if (aborted) {
     // Prepare scope for use in the outer zone.
     zone_ = ast_value_factory->zone();

src/parsing/parser-base.h

@@ -649,10 +649,12 @@ class ParserBase {
   // Creates a function scope that always allocates in zone(). The function
   // scope itself is either allocated in zone() or in target_zone if one is
   // passed in.
-  DeclarationScope* NewFunctionScope(FunctionKind kind) const {
+  DeclarationScope* NewFunctionScope(FunctionKind kind,
+                                     Zone* parse_zone = nullptr) const {
     DCHECK(ast_value_factory());
-    DeclarationScope* result =
+    if (parse_zone == nullptr) parse_zone = zone();
-        new (zone()) DeclarationScope(zone(), scope(), FUNCTION_SCOPE, kind);
+    DeclarationScope* result = new (zone())
+        DeclarationScope(parse_zone, scope(), FUNCTION_SCOPE, kind);
 
     // Record presence of an inner function scope
     function_state_->RecordFunctionOrEvalCall();

src/parsing/parser.cc

@@ -29,21 +29,6 @@
 namespace v8 {
 namespace internal {
 
-
-
-// Helper for putting parts of the parse results into a temporary zone when
-// parsing inner function bodies.
-class PreParserZoneScope {
- public:
-  explicit PreParserZoneScope(PreParser* preparser) : preparser_(preparser) {}
-  ~PreParserZoneScope() { preparser_->zone()->ReleaseMemory(); }
-
- private:
-  PreParser* preparser_;
-
-  DISALLOW_COPY_AND_ASSIGN(PreParserZoneScope);
-};
-
 FunctionLiteral* Parser::DefaultConstructor(const AstRawString* name,
                                             bool call_super, int pos,
                                             int end_pos) {
@@ -379,6 +364,7 @@ Parser::Parser(ParseInfo* info)
                          info->is_module(), true),
       scanner_(info->unicode_cache(), info->character_stream(),
                info->is_module()),
+      preparser_zone_(info->zone()->allocator(), ZONE_NAME),
       reusable_preparser_(nullptr),
       mode_(PARSE_EAGERLY),  // Lazy mode must be set explicitly.
       source_range_map_(info->source_range_map()),
@@ -2551,17 +2537,16 @@ FunctionLiteral* Parser::ParseFunctionLiteral(
   int function_literal_id = GetNextFunctionLiteralId();
   ProducedPreParsedScopeData* produced_preparsed_scope_data = nullptr;
 
-  DeclarationScope* scope;
-
   // This Scope lives in the main zone. We'll migrate data into that zone later.
-  scope = NewFunctionScope(kind);
+  Zone* parse_zone = should_preparse ? &preparser_zone_ : zone();
+  DeclarationScope* scope = NewFunctionScope(kind, parse_zone);
   SetLanguageMode(scope, language_mode);
 #ifdef DEBUG
   scope->SetScopeName(function_name);
 #endif
 
   if (!is_wrapped) Expect(Token::LPAREN, CHECK_OK);
-  scope->set_start_position(scanner()->location().beg_pos);
+  scope->set_start_position(position());
 
   // Eager or lazy parse? If is_lazy_top_level_function, we'll parse
   // lazily. We'll call SkipFunction, which may decide to
@@ -2644,6 +2629,7 @@ bool Parser::SkipFunction(
     bool is_inner_function, bool may_abort,
     FunctionLiteral::EagerCompileHint* hint, bool* ok) {
   FunctionState function_state(&function_state_, &scope_, function_scope);
+  function_scope->set_zone(&preparser_zone_);
 
   DCHECK_NE(kNoSourcePosition, function_scope->start_position());
   DCHECK_EQ(kNoSourcePosition, parameters_end_pos_);
@@ -2678,9 +2664,6 @@ bool Parser::SkipFunction(
     return true;
   }
 
-  PreParser* preparser = reusable_preparser();
-  PreParserZoneScope zone_scope(preparser);
-
   Scanner::BookmarkScope bookmark(scanner());
   bookmark.Set();
 
@@ -2692,7 +2675,7 @@ bool Parser::SkipFunction(
   // state; we don't parse inner functions in the abortable mode anyway.
   DCHECK(!is_inner_function || !may_abort);
 
-  PreParser::PreParseResult result = preparser->PreParseFunction(
+  PreParser::PreParseResult result = reusable_preparser()->PreParseFunction(
       function_name, kind, function_type, function_scope, is_inner_function,
       may_abort, use_counts_, produced_preparsed_scope_data, this->script_id());
 

src/parsing/parser.h

@@ -242,10 +242,10 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
 
   PreParser* reusable_preparser() {
     if (reusable_preparser_ == nullptr) {
-      reusable_preparser_ =
+      reusable_preparser_ = new PreParser(
-          new PreParser(zone(), &scanner_, stack_limit_, ast_value_factory(),
+          &preparser_zone_, &scanner_, stack_limit_, ast_value_factory(),
-                        pending_error_handler(), runtime_call_stats_, logger_,
+          pending_error_handler(), runtime_call_stats_, logger_, -1,
-                        -1, parsing_module_, parsing_on_main_thread_);
+          parsing_module_, parsing_on_main_thread_);
 #define SET_ALLOW(name) reusable_preparser_->set_allow_##name(allow_##name());
       SET_ALLOW(natives);
       SET_ALLOW(harmony_do_expressions);
@@ -1095,6 +1095,7 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
   friend class PreParserZoneScope;  // Uses reusable_preparser().
 
   Scanner scanner_;
+  Zone preparser_zone_;
   PreParser* reusable_preparser_;
   Mode mode_;
 

src/parsing/preparser.cc

@@ -127,7 +127,6 @@ PreParser::PreParseResult PreParser::PreParseFunction(
 #ifdef DEBUG
   function_scope->set_is_being_lazily_parsed(true);
 #endif
-  function_scope->set_zone(zone());
 
   DCHECK(!track_unresolved_variables_);
   track_unresolved_variables_ =

src/parsing/preparser.h

@@ -992,17 +992,14 @@ class PreParser : public ParserBase<PreParser> {
             RuntimeCallStats* runtime_call_stats, Logger* logger,
             int script_id = -1, bool parsing_module = false,
             bool parsing_on_main_thread = true)
-      : ParserBase<PreParser>(new Zone(zone->allocator(), ZONE_NAME), scanner,
+      : ParserBase<PreParser>(zone, scanner, stack_limit, nullptr,
-                              stack_limit, nullptr, ast_value_factory,
+                              ast_value_factory, pending_error_handler,
-                              pending_error_handler, runtime_call_stats, logger,
+                              runtime_call_stats, logger, script_id,
-                              script_id, parsing_module,
+                              parsing_module, parsing_on_main_thread),
-                              parsing_on_main_thread),
         use_counts_(nullptr),
         track_unresolved_variables_(false),
         preparsed_scope_data_builder_(nullptr) {}
 
-  ~PreParser() { delete zone(); }
-
   static bool IsPreParser() { return true; }
 
   PreParserLogger* logger() { return &log_; }