[cleanup] Use ZoneChunkList in the parser

Replace most uses of ZoneList in the parser with ZoneChunkList, which is
more Zone allocation friendly. Includes rewriting some index-based loops
as iterator-based, since ZoneChunkList random access isn't constant
time.

Bug: v8:7754
Change-Id: I49052b8afb90a4f3bfbe4076c2f90505b598e47a
Reviewed-on: https://chromium-review.googlesource.com/1145382Reviewed-by: Marja Hölttä <marja@chromium.org>
Reviewed-by: Peter Marshall <petermarshall@chromium.org>
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/master@{#54631}

src/parsing/func-name-inferrer.cc

@@ -14,72 +14,74 @@ namespace internal {
 FuncNameInferrer::FuncNameInferrer(AstValueFactory* ast_value_factory,
                                    Zone* zone)
     : ast_value_factory_(ast_value_factory),
-      entries_stack_(10, zone),
-      names_stack_(5, zone),
-      funcs_to_infer_(4, zone),
-      zone_(zone) {
-}
-
+      entries_stack_(zone),
+      names_stack_(zone),
+      funcs_to_infer_(zone),
+      zone_(zone) {}
 
 void FuncNameInferrer::PushEnclosingName(const AstRawString* name) {
   // Enclosing name is a name of a constructor function. To check
   // that it is really a constructor, we check that it is not empty
   // and starts with a capital letter.
   if (!name->IsEmpty() && unibrow::Uppercase::Is(name->FirstCharacter())) {
-    names_stack_.Add(Name(name, kEnclosingConstructorName), zone());
+    names_stack_.push_back(Name(name, kEnclosingConstructorName));
   }
 }
 
 
 void FuncNameInferrer::PushLiteralName(const AstRawString* name) {
   if (IsOpen() && name != ast_value_factory_->prototype_string()) {
-    names_stack_.Add(Name(name, kLiteralName), zone());
+    names_stack_.push_back(Name(name, kLiteralName));
   }
 }
 
 
 void FuncNameInferrer::PushVariableName(const AstRawString* name) {
   if (IsOpen() && name != ast_value_factory_->dot_result_string()) {
-    names_stack_.Add(Name(name, kVariableName), zone());
+    names_stack_.push_back(Name(name, kVariableName));
   }
 }
 
 void FuncNameInferrer::RemoveAsyncKeywordFromEnd() {
   if (IsOpen()) {
-    CHECK_GT(names_stack_.length(), 0);
-    CHECK(names_stack_.last().name->IsOneByteEqualTo("async"));
-    names_stack_.RemoveLast();
+    CHECK_GT(names_stack_.size(), 0);
+    CHECK(names_stack_.back().name->IsOneByteEqualTo("async"));
+    names_stack_.pop_back();
   }
 }
 
 void FuncNameInferrer::Leave() {
   DCHECK(IsOpen());
-  names_stack_.Rewind(entries_stack_.RemoveLast());
-  if (entries_stack_.is_empty()) funcs_to_infer_.Clear();
+  size_t last_entry = entries_stack_.back();
+  entries_stack_.pop_back();
+  names_stack_.Rewind(last_entry);
+  if (entries_stack_.is_empty()) funcs_to_infer_.Rewind();
 }
 
 const AstConsString* FuncNameInferrer::MakeNameFromStack() {
   AstConsString* result = ast_value_factory_->NewConsString();
-  for (int pos = 0; pos < names_stack_.length(); pos++) {
+  auto it = names_stack_.begin();
+  while (it != names_stack_.end()) {
+    // Advance the iterator to be able to peek the next value.
+    auto current = it++;
     // Skip consecutive variable declarations.
-    if (pos + 1 < names_stack_.length() &&
-        names_stack_.at(pos).type == kVariableName &&
-        names_stack_.at(pos + 1).type == kVariableName) {
+    if (it != names_stack_.end() && current->type == kVariableName &&
+        it->type == kVariableName) {
       continue;
     }
     // Add name. Separate names with ".".
     if (!result->IsEmpty()) {
       result->AddString(zone(), ast_value_factory_->dot_string());
     }
-    result->AddString(zone(), names_stack_.at(pos).name);
+    result->AddString(zone(), current->name);
   }
   return result;
 }
 
 void FuncNameInferrer::InferFunctionsNames() {
   const AstConsString* func_name = MakeNameFromStack();
-  for (int i = 0; i < funcs_to_infer_.length(); ++i) {
-    funcs_to_infer_[i]->set_raw_inferred_name(func_name);
+  for (FunctionLiteral* func : funcs_to_infer_) {
+    func->set_raw_inferred_name(func_name);
   }
   funcs_to_infer_.Rewind(0);
 }

src/parsing/func-name-inferrer.h

@@ -5,6 +5,7 @@
 #ifndef V8_PARSING_FUNC_NAME_INFERRER_H_
 #define V8_PARSING_FUNC_NAME_INFERRER_H_
 
+#include "src/zone/zone-chunk-list.h"
 #include "src/zone/zone.h"
 
 namespace v8 {
@@ -62,13 +63,13 @@ class FuncNameInferrer : public ZoneObject {
   // Adds a function to infer name for.
   void AddFunction(FunctionLiteral* func_to_infer) {
     if (IsOpen()) {
-      funcs_to_infer_.Add(func_to_infer, zone());
+      funcs_to_infer_.push_back(func_to_infer);
     }
   }
 
   void RemoveLastFunction() {
     if (IsOpen() && !funcs_to_infer_.is_empty()) {
-      funcs_to_infer_.RemoveLast();
+      funcs_to_infer_.pop_back();
     }
   }
 
@@ -94,7 +95,7 @@ class FuncNameInferrer : public ZoneObject {
     NameType type;
   };
 
-  void Enter() { entries_stack_.Add(names_stack_.length(), zone()); }
+  void Enter() { entries_stack_.push_back(names_stack_.size()); }
 
   void Leave();
 
@@ -107,9 +108,9 @@ class FuncNameInferrer : public ZoneObject {
   void InferFunctionsNames();
 
   AstValueFactory* ast_value_factory_;
-  ZoneList<int> entries_stack_;
-  ZoneList<Name> names_stack_;
-  ZoneList<FunctionLiteral*> funcs_to_infer_;
+  ZoneChunkList<size_t> entries_stack_;
+  ZoneChunkList<Name> names_stack_;
+  ZoneChunkList<FunctionLiteral*> funcs_to_infer_;
   Zone* zone_;
 
   DISALLOW_COPY_AND_ASSIGN(FuncNameInferrer);

src/parsing/parser-base.h

@@ -21,6 +21,7 @@
 #include "src/parsing/func-name-inferrer.h"
 #include "src/parsing/scanner.h"
 #include "src/parsing/token.h"
+#include "src/zone/zone-chunk-list.h"
 
 namespace v8 {
 namespace internal {
@@ -416,16 +417,18 @@ class ParserBase {
     void AdoptDestructuringAssignmentsFromParentState(int pos) {
       const auto& outer_assignments =
           outer_function_state_->destructuring_assignments_to_rewrite_;
-      DCHECK_GE(outer_assignments.length(), pos);
-      for (int i = pos; i < outer_assignments.length(); ++i) {
-        auto expr = outer_assignments[i];
+      DCHECK_GE(outer_assignments.size(), pos);
+      auto it = outer_assignments.begin();
+      it.Advance(pos);
+      for (; it != outer_assignments.end(); ++it) {
+        auto expr = *it;
         expr->set_scope(scope_);
-        destructuring_assignments_to_rewrite_.Add(expr, scope_->zone());
+        destructuring_assignments_to_rewrite_.push_back(expr);
       }
       outer_function_state_->RewindDestructuringAssignments(pos);
     }
 
-    const ZoneList<RewritableExpressionT>&
+    const ZoneChunkList<RewritableExpressionT>&
     destructuring_assignments_to_rewrite() const {
       return destructuring_assignments_to_rewrite_;
     }
@@ -472,7 +475,7 @@ class ParserBase {
 
    private:
     void AddDestructuringAssignment(RewritableExpressionT expr) {
-      destructuring_assignments_to_rewrite_.Add(expr, scope_->zone());
+      destructuring_assignments_to_rewrite_.push_back(expr);
     }
 
     // Properties count estimation.
@@ -482,7 +485,7 @@ class ParserBase {
     FunctionState* outer_function_state_;
     DeclarationScope* scope_;
 
-    ZoneList<RewritableExpressionT> destructuring_assignments_to_rewrite_;
+    ZoneChunkList<RewritableExpressionT> destructuring_assignments_to_rewrite_;
 
     ZoneList<typename ExpressionClassifier::Error> reported_errors_;
 
@@ -1577,7 +1580,7 @@ ParserBase<Impl>::FunctionState::FunctionState(
       function_state_stack_(function_state_stack),
       outer_function_state_(*function_state_stack),
       scope_(scope),
-      destructuring_assignments_to_rewrite_(16, scope->zone()),
+      destructuring_assignments_to_rewrite_(scope->zone()),
       reported_errors_(16, scope->zone()),
       dont_optimize_reason_(BailoutReason::kNoReason),
       suspend_count_(0),
@@ -2875,8 +2878,8 @@ ParserBase<Impl>::ParseAssignmentExpression(bool accept_IN, bool* ok) {
       this, classifier()->duplicate_finder());
 
   Scope::Snapshot scope_snapshot(scope());
-  int rewritable_length =
-      function_state_->destructuring_assignments_to_rewrite().length();
+  int rewritable_length = static_cast<int>(
+      function_state_->destructuring_assignments_to_rewrite().size());
 
   bool is_async = peek() == Token::ASYNC &&
                   !scanner()->HasAnyLineTerminatorAfterNext() &&

src/parsing/parser.cc

@@ -798,7 +798,7 @@ FunctionLiteral* Parser::DoParseFunction(Isolate* isolate, ParseInfo* info,
       ParserFormalParameters formals(scope);
       // The outer FunctionState should not contain destructuring assignments.
       DCHECK_EQ(0,
-                function_state.destructuring_assignments_to_rewrite().length());
+                function_state.destructuring_assignments_to_rewrite().size());
       {
         // Parsing patterns as variable reference expression creates
         // NewUnresolved references in current scope. Enter arrow function
@@ -943,10 +943,8 @@ const AstRawString* Parser::ParseModuleSpecifier(bool* ok) {
   return GetSymbol();
 }
 
-void Parser::ParseExportClause(ZonePtrList<const AstRawString>* export_names,
-                               ZoneList<Scanner::Location>* export_locations,
-                               ZonePtrList<const AstRawString>* local_names,
-                               Scanner::Location* reserved_loc, bool* ok) {
+ZoneChunkList<Parser::ExportClauseData>* Parser::ParseExportClause(
+    Scanner::Location* reserved_loc, bool* ok) {
   // ExportClause :
   //   '{' '}'
   //   '{' ExportsList '}'
@@ -959,8 +957,10 @@ void Parser::ParseExportClause(ZonePtrList<const AstRawString>* export_names,
   // ExportSpecifier :
   //   IdentifierName
   //   IdentifierName 'as' IdentifierName
+  ZoneChunkList<ExportClauseData>* export_data =
+      new (zone()) ZoneChunkList<ExportClauseData>(zone());
 
-  Expect(Token::LBRACE, CHECK_OK_VOID);
+  Expect(Token::LBRACE, CHECK_OK);
 
   Token::Value name_tok;
   while ((name_tok = peek()) != Token::RBRACE) {
@@ -971,11 +971,11 @@ void Parser::ParseExportClause(ZonePtrList<const AstRawString>* export_names,
                              parsing_module_)) {
       *reserved_loc = scanner()->location();
     }
-    const AstRawString* local_name = ParseIdentifierName(CHECK_OK_VOID);
+    const AstRawString* local_name = ParseIdentifierName(CHECK_OK);
     const AstRawString* export_name = nullptr;
     Scanner::Location location = scanner()->location();
     if (CheckContextualKeyword(Token::AS)) {
-      export_name = ParseIdentifierName(CHECK_OK_VOID);
+      export_name = ParseIdentifierName(CHECK_OK);
       // Set the location to the whole "a as b" string, so that it makes sense
       // both for errors due to "a" and for errors due to "b".
       location.end_pos = scanner()->location().end_pos;
@@ -983,14 +983,13 @@ void Parser::ParseExportClause(ZonePtrList<const AstRawString>* export_names,
     if (export_name == nullptr) {
       export_name = local_name;
     }
-    export_names->Add(export_name, zone());
-    local_names->Add(local_name, zone());
-    export_locations->Add(location, zone());
+    export_data->push_back({export_name, local_name, location});
     if (peek() == Token::RBRACE) break;
-    Expect(Token::COMMA, CHECK_OK_VOID);
+    Expect(Token::COMMA, CHECK_OK);
   }
 
-  Expect(Token::RBRACE, CHECK_OK_VOID);
+  Expect(Token::RBRACE, CHECK_OK);
+  return export_data;
 }
 
 ZonePtrList<const Parser::NamedImport>* Parser::ParseNamedImports(int pos,
@@ -1264,11 +1263,8 @@ Statement* Parser::ParseExportDeclaration(bool* ok) {
       // encountered, and then throw a SyntaxError if we are in the
       // non-FromClause case.
       Scanner::Location reserved_loc = Scanner::Location::invalid();
-      ZonePtrList<const AstRawString> export_names(1, zone());
-      ZoneList<Scanner::Location> export_locations(1, zone());
-      ZonePtrList<const AstRawString> original_names(1, zone());
-      ParseExportClause(&export_names, &export_locations, &original_names,
-                        &reserved_loc, CHECK_OK);
+      ZoneChunkList<ExportClauseData>* export_data =
+          ParseExportClause(&reserved_loc, CHECK_OK);
       const AstRawString* module_specifier = nullptr;
       Scanner::Location specifier_loc;
       if (CheckContextualKeyword(Token::FROM)) {
@@ -1281,21 +1277,18 @@ Statement* Parser::ParseExportDeclaration(bool* ok) {
         return nullptr;
       }
       ExpectSemicolon(CHECK_OK);
-      const int length = export_names.length();
-      DCHECK_EQ(length, original_names.length());
-      DCHECK_EQ(length, export_locations.length());
       if (module_specifier == nullptr) {
-        for (int i = 0; i < length; ++i) {
-          module()->AddExport(original_names[i], export_names[i],
-                              export_locations[i], zone());
+        for (const ExportClauseData& data : *export_data) {
+          module()->AddExport(data.local_name, data.export_name, data.location,
+                              zone());
         }
-      } else if (length == 0) {
+      } else if (export_data->is_empty()) {
         module()->AddEmptyImport(module_specifier, specifier_loc);
       } else {
-        for (int i = 0; i < length; ++i) {
-          module()->AddExport(original_names[i], export_names[i],
-                              module_specifier, export_locations[i],
-                              specifier_loc, zone());
+        for (const ExportClauseData& data : *export_data) {
+          module()->AddExport(data.local_name, data.export_name,
+                              module_specifier, data.location, specifier_loc,
+                              zone());
         }
       }
       return factory()->NewEmptyStatement(pos);
@@ -3652,10 +3645,11 @@ void Parser::RewriteAsyncFunctionBody(ZonePtrList<Statement>* body,
 void Parser::RewriteDestructuringAssignments() {
   const auto& assignments =
       function_state_->destructuring_assignments_to_rewrite();
-  for (int i = assignments.length() - 1; i >= 0; --i) {
+  auto it = assignments.rbegin();
+  for (; it != assignments.rend(); ++it) {
     // Rewrite list in reverse, so that nested assignment patterns are rewritten
     // correctly.
-    RewritableExpression* to_rewrite = assignments[i];
+    RewritableExpression* to_rewrite = *it;
     DCHECK_NOT_NULL(to_rewrite);
     if (!to_rewrite->is_rewritten()) {
       // Since this function is called at the end of parsing the program,

src/parsing/parser.h

@@ -17,6 +17,7 @@
 #include "src/parsing/preparse-data.h"
 #include "src/parsing/preparser.h"
 #include "src/utils.h"
+#include "src/zone/zone-chunk-list.h"
 
 namespace v8 {
 
@@ -262,10 +263,13 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
   void ParseImportDeclaration(bool* ok);
   Statement* ParseExportDeclaration(bool* ok);
   Statement* ParseExportDefault(bool* ok);
-  void ParseExportClause(ZonePtrList<const AstRawString>* export_names,
-                         ZoneList<Scanner::Location>* export_locations,
-                         ZonePtrList<const AstRawString>* local_names,
-                         Scanner::Location* reserved_loc, bool* ok);
+  struct ExportClauseData {
+    const AstRawString* export_name;
+    const AstRawString* local_name;
+    Scanner::Location location;
+  };
+  ZoneChunkList<ExportClauseData>* ParseExportClause(
+      Scanner::Location* reserved_loc, bool* ok);
   struct NamedImport : public ZoneObject {
     const AstRawString* import_name;
     const AstRawString* local_name;

src/parsing/preparser.h

@@ -823,9 +823,9 @@ class PreParserFactory {
     return PreParserStatement::Default();
   }
 
-  PreParserExpression NewCallRuntime(Runtime::FunctionId id,
-                                     ZoneList<PreParserExpression>* arguments,
-                                     int pos) {
+  PreParserExpression NewCallRuntime(
+      Runtime::FunctionId id, ZoneChunkList<PreParserExpression>* arguments,
+      int pos) {
     return PreParserExpression::Default();
   }
 

src/zone/zone-chunk-list.h

@@ -61,6 +61,7 @@ class ZoneChunkList : public ZoneObject {
   }
 
   size_t size() const { return size_; }
+  bool is_empty() const { return size() == 0; }
 
   T& front() const;
   T& back() const;
@@ -158,6 +159,7 @@ class ZoneChunkListIterator
 
  public:
   maybe_const<T>& operator*() { return current_->items()[position_]; }
+  maybe_const<T>* operator->() { return &current_->items()[position_]; }
   bool operator==(const ZoneChunkListIterator& other) const {
     return other.current_ == current_ && other.position_ == position_;
   }
@@ -187,6 +189,30 @@ class ZoneChunkListIterator
     return clone;
   }
 
+  void Advance(int amount) {
+    // Move forwards.
+    DCHECK_GE(amount, 0);
+#if DEBUG
+    ZoneChunkListIterator clone(*this);
+    for (int i = 0; i < amount; ++i) {
+      ++clone;
+    }
+#endif
+
+    position_ += amount;
+    while (position_ > 0 && position_ >= current_->capacity_) {
+      auto overshoot = position_ - current_->capacity_;
+      current_ = current_->next_;
+      position_ = overshoot;
+
+      DCHECK(position_ == 0 || current_);
+    }
+
+#if DEBUG
+    DCHECK_EQ(clone, *this);
+#endif
+  }
+
  private:
   friend class ZoneChunkList<T>;
 

test/unittests/zone/zone-chunk-list-unittest.cc

@@ -280,5 +280,60 @@ TEST(ZoneChunkList, PushBackPopBackSize) {
   CHECK_EQ(size_t(0), zone_chunk_list.size());
 }
 
+TEST(ZoneChunkList, AdvanceZeroTest) {
+  AccountingAllocator allocator;
+  Zone zone(&allocator, ZONE_NAME);
+
+  ZoneChunkList<uintptr_t> zone_chunk_list(&zone);
+
+  for (size_t i = 0; i < kItemCount; ++i) {
+    zone_chunk_list.push_back(static_cast<uintptr_t>(i));
+  }
+
+  auto iterator_advance = zone_chunk_list.begin();
+
+  iterator_advance.Advance(0);
+
+  CHECK_EQ(iterator_advance, zone_chunk_list.begin());
+}
+
+TEST(ZoneChunkList, AdvancePartwayTest) {
+  AccountingAllocator allocator;
+  Zone zone(&allocator, ZONE_NAME);
+
+  ZoneChunkList<uintptr_t> zone_chunk_list(&zone);
+
+  for (size_t i = 0; i < kItemCount; ++i) {
+    zone_chunk_list.push_back(static_cast<uintptr_t>(i));
+  }
+
+  auto iterator_advance = zone_chunk_list.begin();
+  auto iterator_one_by_one = zone_chunk_list.begin();
+
+  iterator_advance.Advance(kItemCount / 2);
+  for (size_t i = 0; i < kItemCount / 2; ++i) {
+    ++iterator_one_by_one;
+  }
+
+  CHECK_EQ(iterator_advance, iterator_one_by_one);
+}
+
+TEST(ZoneChunkList, AdvanceEndTest) {
+  AccountingAllocator allocator;
+  Zone zone(&allocator, ZONE_NAME);
+
+  ZoneChunkList<uintptr_t> zone_chunk_list(&zone);
+
+  for (size_t i = 0; i < kItemCount; ++i) {
+    zone_chunk_list.push_back(static_cast<uintptr_t>(i));
+  }
+
+  auto iterator_advance = zone_chunk_list.begin();
+
+  iterator_advance.Advance(kItemCount);
+
+  CHECK_EQ(iterator_advance, zone_chunk_list.end());
+}
+
 }  // namespace internal
 }  // namespace v8