[parser] Give the preparser its own zone

Simplify zone discarding for preparsed functions by simply giving the preparser
its own zone that we reset whenever we finish preparsing something.

Change-Id: I3135fbbcd6caefa4654b1ae2b2207377e51bee26
Reviewed-on: https://chromium-review.googlesource.com/1238614
Commit-Queue: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Marja Hölttä <marja@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56146}

src/ast/ast.h

@@ -3343,7 +3343,6 @@ class AstNodeFactory final {
   }
 
   Zone* zone() const { return zone_; }
-  void set_zone(Zone* zone) { zone_ = zone; }
 
  private:
   // This zone may be deallocated upon returning from parsing a function body

src/ast/scopes.h

@@ -684,6 +684,7 @@ class V8_EXPORT_PRIVATE DeclarationScope : public Scope {
   }
   bool is_being_lazily_parsed() const { return is_being_lazily_parsed_; }
 #endif
+  void set_zone(Zone* zone) { zone_ = zone; }
 
   bool ShouldEagerCompile() const;
   void set_should_eager_compile();

src/parsing/func-name-inferrer.h

@@ -79,8 +79,6 @@ class FuncNameInferrer : public ZoneObject {
     }
   }
 
-  void set_zone(Zone* new_zone) { zone_ = new_zone; }
-
  private:
   enum NameType {
     kEnclosingConstructorName,

src/parsing/parser-base.h

@@ -647,12 +647,10 @@ 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,
-                                     Zone* target_zone = nullptr) const {
+  DeclarationScope* NewFunctionScope(FunctionKind kind) const {
     DCHECK(ast_value_factory());
-    if (target_zone == nullptr) target_zone = zone();
-    DeclarationScope* result = new (target_zone)
-        DeclarationScope(zone(), scope(), FUNCTION_SCOPE, kind);
+    DeclarationScope* result =
+        new (zone()) DeclarationScope(zone(), scope(), FUNCTION_SCOPE, kind);
 
     // Record presence of an inner function scope
     function_state_->RecordFunctionOrEvalCall();
@@ -1552,8 +1550,6 @@ class ParserBase {
   bool allow_harmony_import_meta_;
   bool allow_harmony_private_fields_;
   bool allow_eval_cache_;
-
-  friend class DiscardableZoneScope;
 };
 
 template <typename Impl>

src/parsing/parser.cc

@@ -33,46 +33,21 @@ namespace internal {
 
 // Helper for putting parts of the parse results into a temporary zone when
 // parsing inner function bodies.
-class DiscardableZoneScope {
+class PreParserZoneScope {
  public:
-  DiscardableZoneScope(Parser* parser, Zone* temp_zone, bool use_temp_zone)
-      : parser_(parser),
-        prev_zone_(parser->zone_),
-        prev_allow_lazy_(parser->allow_lazy_),
-        prev_temp_zoned_(parser->temp_zoned_) {
-    if (use_temp_zone) {
-      DCHECK(!parser_->temp_zoned_);
-      parser_->allow_lazy_ = false;
-      parser_->temp_zoned_ = true;
-      parser_->fni_.set_zone(temp_zone);
-      parser_->zone_ = temp_zone;
-      parser_->factory()->set_zone(temp_zone);
-      if (parser_->reusable_preparser_ != nullptr) {
-        parser_->reusable_preparser_->zone_ = temp_zone;
-        parser_->reusable_preparser_->factory()->set_zone(temp_zone);
-      }
-    }
-  }
-  void Reset() {
-    parser_->fni_.set_zone(prev_zone_);
-    parser_->zone_ = prev_zone_;
-    parser_->factory()->set_zone(prev_zone_);
-    parser_->allow_lazy_ = prev_allow_lazy_;
-    parser_->temp_zoned_ = prev_temp_zoned_;
-    if (parser_->reusable_preparser_ != nullptr) {
-      parser_->reusable_preparser_->zone_ = prev_zone_;
-      parser_->reusable_preparser_->factory()->set_zone(prev_zone_);
-    }
-  }
-  ~DiscardableZoneScope() { Reset(); }
+  PreParserZoneScope(Parser* parser, bool should_preparse)
+      : parser_(parser), should_preparse_(should_preparse) {}
+  ~PreParserZoneScope() {
+    if (should_preparse_) {
+      parser_->reusable_preparser()->zone()->DeleteAll();
+    }
+  }
 
  private:
   Parser* parser_;
-  Zone* prev_zone_;
-  bool prev_allow_lazy_;
-  bool prev_temp_zoned_;
+  bool should_preparse_;
 
-  DISALLOW_COPY_AND_ASSIGN(DiscardableZoneScope);
+  DISALLOW_COPY_AND_ASSIGN(PreParserZoneScope);
 };
 
 FunctionLiteral* Parser::DefaultConstructor(const AstRawString* name,
@@ -415,7 +390,6 @@ Parser::Parser(ParseInfo* info)
       source_range_map_(info->source_range_map()),
       target_stack_(nullptr),
       total_preparse_skipped_(0),
-      temp_zoned_(false),
       consumed_preparsed_scope_data_(info->consumed_preparsed_scope_data()),
       parameters_end_pos_(info->parameters_end_pos()) {
   // Even though we were passed ParseInfo, we should not store it in
@@ -2583,25 +2557,14 @@ FunctionLiteral* Parser::ParseFunctionLiteral(
   int function_literal_id = GetNextFunctionLiteralId();
   ProducedPreParsedScopeData* produced_preparsed_scope_data = nullptr;
 
-  Zone* outer_zone = zone();
   DeclarationScope* scope;
 
   {
-    // Temporary zones can nest. When we migrate free variables (see below), we
-    // need to recreate them in the previous Zone.
-    AstNodeFactory previous_zone_ast_node_factory(ast_value_factory(), zone());
-
-    // Open a new zone scope, which sets our AstNodeFactory to allocate in the
-    // new temporary zone if the preconditions are satisfied, and ensures that
-    // the previous zone is always restored after parsing the body. To be able
-    // to do scope analysis correctly after full parsing, we migrate needed
-    // information when the function is parsed.
-    Zone temp_zone(zone()->allocator(), ZONE_NAME);
-    DiscardableZoneScope zone_scope(this, &temp_zone, should_preparse);
+    PreParserZoneScope zone_scope(this, should_preparse);
 
     // This Scope lives in the main zone. We'll migrate data into that zone
     // later.
-    scope = NewFunctionScope(kind, outer_zone);
+    scope = NewFunctionScope(kind);
     SetLanguageMode(scope, language_mode);
 #ifdef DEBUG
     scope->SetScopeName(function_name);
@@ -2634,14 +2597,13 @@ FunctionLiteral* Parser::ParseFunctionLiteral(
         // should also eager-compile this function.
         eager_compile_hint = FunctionLiteral::kShouldEagerCompile;
         scope->ResetAfterPreparsing(ast_value_factory(), true);
-        zone_scope.Reset();
         // Trigger eager (re-)parsing, just below this block.
         should_preparse = false;
       }
     }
 
     if (should_preparse) {
-      scope->AnalyzePartially(&previous_zone_ast_node_factory);
+      scope->AnalyzePartially(factory());
     } else {
       body = ParseFunction(
           function_name, pos, kind, function_type, scope, &num_parameters,
@@ -2649,7 +2611,6 @@ FunctionLiteral* Parser::ParseFunctionLiteral(
           &suspend_count, arguments_for_wrapped_function, CHECK_OK);
     }
 
-    DCHECK_EQ(should_preparse, temp_zoned_);
     if (V8_UNLIKELY(FLAG_log_function_events)) {
       double ms = timer.Elapsed().InMillisecondsF();
       const char* event_name = should_preparse
@@ -2689,7 +2650,7 @@ FunctionLiteral* Parser::ParseFunctionLiteral(
                               CHECK_OK);
     }
     CheckConflictingVarDeclarations(scope, CHECK_OK);
-  }  // DiscardableZoneScope goes out of scope.
+  }  // PreParserZoneScope goes out of scope.
 
   FunctionLiteral::ParameterFlag duplicate_parameters =
       has_duplicate_parameters ? FunctionLiteral::kHasDuplicateParameters

src/parsing/parser.h

@@ -1090,9 +1090,7 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
   }
 
   // Parser's private field members.
-  friend class DiscardableZoneScope;  // Uses reusable_preparser_.
-  // FIXME(marja): Make reusable_preparser_ always use its own temp Zone (call
-  // DeleteAll after each function), so this won't be needed.
+  friend class PreParserZoneScope;  // Uses reusable_preparser().
 
   Scanner scanner_;
   PreParser* reusable_preparser_;

src/parsing/preparser.cc

@@ -127,6 +127,7 @@ 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

@@ -566,11 +566,6 @@ class PreParserFactory {
   explicit PreParserFactory(AstValueFactory* ast_value_factory, Zone* zone)
       : ast_node_factory_(ast_value_factory, zone), zone_(zone) {}
 
-  void set_zone(Zone* zone) {
-    ast_node_factory_.set_zone(zone);
-    zone_ = zone;
-  }
-
   AstNodeFactory* ast_node_factory() { return &ast_node_factory_; }
 
   PreParserExpression NewStringLiteral(const PreParserIdentifier& identifier,
@@ -972,14 +967,17 @@ 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>(zone, scanner, stack_limit, nullptr,
-                              ast_value_factory, pending_error_handler,
-                              runtime_call_stats, logger, script_id,
-                              parsing_module, parsing_on_main_thread),
+      : ParserBase<PreParser>(new Zone(zone->allocator(), ZONE_NAME), scanner,
+                              stack_limit, nullptr, ast_value_factory,
+                              pending_error_handler, runtime_call_stats, logger,
+                              script_id, parsing_module,
+                              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_; }

src/zone/zone.h

@@ -57,6 +57,9 @@ class V8_EXPORT_PRIVATE Zone final {
   // Seals the zone to prevent any further allocation.
   void Seal() { sealed_ = true; }
 
+  // Deletes all objects and free all memory allocated in the Zone.
+  void DeleteAll();
+
   // Returns true if more memory has been allocated in zones than
   // the limit allows.
   bool excess_allocation() const {
@@ -82,9 +85,6 @@ class V8_EXPORT_PRIVATE Zone final {
   // Report zone excess when allocation exceeds this limit.
   static const size_t kExcessLimit = 256 * MB;
 
-  // Deletes all objects and free all memory allocated in the Zone.
-  void DeleteAll();
-
   // The number of bytes allocated in this zone so far.
   size_t allocation_size_;