[parser] Use shared data buffer for PreparseData generation

By using a shared byte buffer on the preparser we can drastically
reduce the number of ZoneChunkLists.

Each PreparseDataBuilder now explicitly keeps track of all inner
builders/functions and writes out the data in consecutive order.

Change-Id: I0aada118d869b150108c1f633d9960474ad2f9a1
Reviewed-on: https://chromium-review.googlesource.com/c/1411600
Commit-Queue: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58926}

src/ast/scopes.cc

@@ -16,6 +16,7 @@
 #include "src/objects/module-inl.h"
 #include "src/objects/scope-info.h"
 #include "src/parsing/parse-info.h"
+#include "src/parsing/parser.h"
 #include "src/parsing/preparse-data.h"
 #include "src/zone/zone-list-inl.h"
 
@@ -1417,28 +1418,29 @@ void DeclarationScope::ResetAfterPreparsing(AstValueFactory* ast_value_factory,
   was_lazily_parsed_ = !aborted;
 }
 
-void Scope::SavePreparseData() {
+void Scope::SavePreparseData(Parser* parser) {
   if (PreparseDataBuilder::ScopeIsSkippableFunctionScope(this)) {
-    AsDeclarationScope()->SavePreparseDataForDeclarationScope();
+    AsDeclarationScope()->SavePreparseDataForDeclarationScope(parser);
   }
 
   for (Scope* scope = inner_scope_; scope != nullptr; scope = scope->sibling_) {
-    scope->SavePreparseData();
+    scope->SavePreparseData(parser);
   }
 }
 
-void DeclarationScope::SavePreparseDataForDeclarationScope() {
+void DeclarationScope::SavePreparseDataForDeclarationScope(Parser* parser) {
   if (preparse_data_builder_ == nullptr) return;
-  preparse_data_builder_->SaveScopeAllocationData(this);
+  preparse_data_builder_->SaveScopeAllocationData(this, parser);
 }
 
-void DeclarationScope::AnalyzePartially(AstNodeFactory* ast_node_factory) {
+void DeclarationScope::AnalyzePartially(Parser* parser,
+                                        AstNodeFactory* ast_node_factory) {
   DCHECK(!force_eager_compilation_);
   UnresolvedList new_unresolved_list;
   if (!IsArrowFunction(function_kind_) &&
       (!outer_scope_->is_script_scope() ||
        (preparse_data_builder_ != nullptr &&
-        preparse_data_builder_->ContainsInnerFunctions()))) {
+        preparse_data_builder_->HasInnerFunctions()))) {
     // Try to resolve unresolved variables for this Scope and migrate those
     // which cannot be resolved inside. It doesn't make sense to try to resolve
     // them in the outer Scopes here, because they are incomplete.
@@ -1449,7 +1451,7 @@ void DeclarationScope::AnalyzePartially(AstNodeFactory* ast_node_factory) {
       function_ = ast_node_factory->CopyVariable(function_);
     }
 
-    SavePreparseData();
+    SavePreparseData(parser);
   }
 
 #ifdef DEBUG

src/ast/scopes.h

@@ -21,6 +21,7 @@ class AstValueFactory;
 class AstRawString;
 class Declaration;
 class ParseInfo;
+class Parser;
 class PreparseDataBuilder;
 class SloppyBlockFunctionStatement;
 class Statement;
@@ -571,7 +572,7 @@ class V8_EXPORT_PRIVATE Scope : public NON_EXPORTED_BASE(ZoneObject) {
 
   // Walk the scope chain to find DeclarationScopes; call
   // SavePreparseDataForDeclarationScope for each.
-  void SavePreparseData();
+  void SavePreparseData(Parser* parser);
 
   // Create a non-local variable with a given name.
   // These variables are looked up dynamically at runtime.
@@ -949,7 +950,7 @@ class V8_EXPORT_PRIVATE DeclarationScope : public Scope {
   // this records variables which cannot be resolved inside the Scope (we don't
   // yet know what they will resolve to since the outer Scopes are incomplete)
   // and recreates them with the correct Zone with ast_node_factory.
-  void AnalyzePartially(AstNodeFactory* ast_node_factory);
+  void AnalyzePartially(Parser* parser, AstNodeFactory* ast_node_factory);
 
   // Allocate ScopeInfos for top scope and any inner scopes that need them.
   // Does nothing if ScopeInfo is already allocated.
@@ -998,7 +999,7 @@ class V8_EXPORT_PRIVATE DeclarationScope : public Scope {
   // Save data describing the context allocation of the variables in this scope
   // and its subscopes (except scopes at the laziness boundary). The data is
   // saved in produced_preparse_data_.
-  void SavePreparseDataForDeclarationScope();
+  void SavePreparseDataForDeclarationScope(Parser* parser);
 
   void set_preparse_data_builder(PreparseDataBuilder* preparse_data_builder) {
     preparse_data_builder_ = preparse_data_builder;

src/objects/fixed-array.h

@@ -560,15 +560,20 @@ class PodArray : public ByteArray {
     ByteArray::copy_out(index * sizeof(T), reinterpret_cast<byte*>(result),
                         sizeof(T));
   }
+
+  void copy_in(int index, const T* buffer, int length) {
+    ByteArray::copy_in(index * sizeof(T), reinterpret_cast<const byte*>(buffer),
+                       length * sizeof(T));
+  }
+
   T get(int index) {
     T result;
     copy_out(index, &result);
     return result;
   }
-  void set(int index, const T& value) {
-    copy_in(index * sizeof(T), reinterpret_cast<const byte*>(&value),
-            sizeof(T));
-  }
+
+  void set(int index, const T& value) { copy_in(index, &value, 1); }
+
   inline int length() const;
   DECL_CAST(PodArray<T>)
 

src/parsing/parser.cc

@@ -395,6 +395,7 @@ Parser::Parser(ParseInfo* info)
       target_stack_(nullptr),
       total_preparse_skipped_(0),
       consumed_preparse_data_(info->consumed_preparse_data()),
+      preparse_data_buffer_(),
       parameters_end_pos_(info->parameters_end_pos()) {
   // Even though we were passed ParseInfo, we should not store it in
   // Parser - this makes sure that Isolate is not accidentally accessed via
@@ -2545,7 +2546,7 @@ bool Parser::SkipFunction(const AstRawString* function_name, FunctionKind kind,
         function_scope->end_position() - function_scope->start_position();
     *num_parameters = logger->num_parameters();
     SkipFunctionLiterals(logger->num_inner_functions());
-    function_scope->AnalyzePartially(factory());
+    function_scope->AnalyzePartially(this, factory());
   }
 
   return true;

src/parsing/parser.h

@@ -282,6 +282,7 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
       SET_ALLOW(harmony_private_methods);
       SET_ALLOW(eval_cache);
 #undef SET_ALLOW
+      preparse_data_buffer_.reserve(128);
     }
     return reusable_preparser_;
   }
@@ -1055,8 +1056,13 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
 
   ParseInfo* info() const { return info_; }
 
+  std::vector<uint8_t>* preparse_data_buffer() {
+    return &preparse_data_buffer_;
+  }
+
   // Parser's private field members.
   friend class PreParserZoneScope;  // Uses reusable_preparser().
+  friend class PreparseDataBuilder;  // Uses preparse_data_buffer()
 
   ParseInfo* info_;
   Scanner scanner_;
@@ -1082,6 +1088,7 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
   bool allow_lazy_;
   bool temp_zoned_;
   ConsumedPreparseData* consumed_preparse_data_;
+  std::vector<uint8_t> preparse_data_buffer_;
 
   // If not kNoSourcePosition, indicates that the first function literal
   // encountered is a dynamic function, see CreateDynamicFunction(). This field

src/parsing/preparse-data-impl.h

@@ -15,51 +15,6 @@ namespace internal {
 // Classes which are internal to prepared-scope-data.cc, but are exposed in
 // a header for tests.
 
-struct PreparseByteDataConstants {
-#ifdef DEBUG
-  static constexpr int kMagicValue = 0xC0DE0DE;
-
-  static constexpr size_t kUint32Size = 5;
-  static constexpr size_t kUint8Size = 2;
-  static constexpr size_t kQuarterMarker = 0;
-  static constexpr size_t kPlaceholderSize = kUint32Size;
-#else
-  static constexpr size_t kUint32Size = 4;
-  static constexpr size_t kUint8Size = 1;
-  static constexpr size_t kPlaceholderSize = 0;
-#endif
-
-  static const size_t kSkippableFunctionDataSize =
-      4 * kUint32Size + 1 * kUint8Size;
-};
-
-class PreparseDataBuilder::ByteData : public ZoneObject,
-                                      public PreparseByteDataConstants {
- public:
-  explicit ByteData(Zone* zone)
-      : free_quarters_in_last_byte_(0), backing_store_(zone) {}
-  void WriteUint32(uint32_t data);
-  void WriteUint8(uint8_t data);
-  void WriteQuarter(uint8_t data);
-
-#ifdef DEBUG
-  // For overwriting previously written data at position 0.
-  void OverwriteFirstUint32(uint32_t data);
-#endif
-
-  void StoreInto(PreparseData data);
-
-  size_t size() const { return backing_store_.size(); }
-
-  ZoneChunkList<uint8_t>::iterator begin() { return backing_store_.begin(); }
-
-  ZoneChunkList<uint8_t>::iterator end() { return backing_store_.end(); }
-
- private:
-  uint8_t free_quarters_in_last_byte_;
-  ZoneChunkList<uint8_t> backing_store_;
-};
-
 // Wraps a ZoneVector<uint8_t> to have with functions named the same as
 // PodArray<uint8_t>.
 class ZoneVectorWrapper {
@@ -124,6 +79,7 @@ class BaseConsumedPreparseData : public ConsumedPreparseData {
     }
 
     int32_t ReadUint32() {
+      DCHECK(has_data_);
       DCHECK(HasRemainingBytes(kUint32Size));
       // Check that there indeed is an integer following.
       DCHECK_EQ(data_.get(index_++), kUint32Size);
@@ -208,7 +164,7 @@ class OnHeapConsumedPreparseData final
   OnHeapConsumedPreparseData(Isolate* isolate, Handle<PreparseData> data);
 
   PreparseData GetScopeData() final;
-  ProducedPreparseData* GetChildData(Zone* zone, int index) final;
+  ProducedPreparseData* GetChildData(Zone* zone, int child_index) final;
 
  private:
   Isolate* isolate_;
@@ -218,16 +174,16 @@ class OnHeapConsumedPreparseData final
 // A serialized PreparseData in zone memory (as apposed to being on-heap).
 class ZonePreparseData : public ZoneObject {
  public:
-  ZonePreparseData(Zone* zone, PreparseDataBuilder::ByteData* byte_data,
-                   int child_length);
+  ZonePreparseData(Zone* zone, Vector<uint8_t>* byte_data, int child_length);
 
   Handle<PreparseData> Serialize(Isolate* isolate);
 
-  int child_length() const { return static_cast<int>(children_.size()); }
+  int children_length() const { return static_cast<int>(children_.size()); }
 
   ZonePreparseData* get_child(int index) { return children_[index]; }
 
   void set_child(int index, ZonePreparseData* child) {
+    DCHECK_NOT_NULL(child);
     children_[index] = child;
   }
 

src/parsing/preparse-data.h

@@ -17,6 +17,7 @@ namespace internal {
 template <typename T>
 class PodArray;
 
+class Parser;
 class PreParser;
 class PreparseData;
 class ZonePreparseData;
@@ -62,13 +63,30 @@ class ZonePreparseData;
 
  */
 
-class PreparseDataBuilder : public ZoneObject {
- public:
-  class ByteData;
+struct PreparseByteDataConstants {
+#ifdef DEBUG
+  static constexpr int kMagicValue = 0xC0DE0DE;
+
+  static constexpr size_t kUint32Size = 5;
+  static constexpr size_t kUint8Size = 2;
+  static constexpr size_t kQuarterMarker = 0;
+  static constexpr size_t kPlaceholderSize = kUint32Size;
+#else
+  static constexpr size_t kUint32Size = 4;
+  static constexpr size_t kUint8Size = 1;
+  static constexpr size_t kPlaceholderSize = 0;
+#endif
+
+  static const size_t kSkippableFunctionDataSize =
+      4 * kUint32Size + 1 * kUint8Size;
+};
 
+class PreparseDataBuilder : public ZoneObject,
+                            public PreparseByteDataConstants {
+ public:
   // Create a PreparseDataBuilder object which will collect data as we
   // parse.
-  PreparseDataBuilder(Zone* zone, PreparseDataBuilder* parent);
+  explicit PreparseDataBuilder(Zone* zone, PreparseDataBuilder* parent_builder);
 
   PreparseDataBuilder* parent() const { return parent_; }
 
@@ -81,8 +99,8 @@ class PreparseDataBuilder : public ZoneObject {
         : preparser_(preparser), builder_(nullptr) {}
 
     void Start(DeclarationScope* function_scope);
-    void AddSkippableFunction(DeclarationScope* function_scope,
-                              int end_position, int num_inner_functions);
+    void SetSkippableFunction(DeclarationScope* function_scope,
+                              int num_inner_functions);
     ~DataGatheringScope();
 
    private:
@@ -92,9 +110,46 @@ class PreparseDataBuilder : public ZoneObject {
     DISALLOW_COPY_AND_ASSIGN(DataGatheringScope);
   };
 
+  class ByteData : public ZoneObject, public PreparseByteDataConstants {
+   public:
+    ByteData() : byte_data_(nullptr), free_quarters_in_last_byte_(0) {}
+
+    ~ByteData() {}
+
+    void Start(std::vector<uint8_t>* buffer);
+    void Finalize(Zone* zone);
+
+    Handle<PreparseData> CopyToHeap(Isolate* isolate, int children_length);
+    ZonePreparseData* CopyToZone(Zone* zone, int children_length);
+
+    void WriteUint32(uint32_t data);
+    void WriteUint8(uint8_t data);
+    void WriteQuarter(uint8_t data);
+
+#ifdef DEBUG
+    // For overwriting previously written data at position 0.
+    void SaveCurrentSizeAtFirstUint32();
+    int length() const;
+#endif
+
+   private:
+    union {
+      // Only used during construction (is_finalized_ == false).
+      std::vector<uint8_t>* byte_data_;
+      // Once the data is finalized, it lives in a Zone, this implies
+      // is_finalized_ == true.
+      Vector<uint8_t> zone_byte_data_;
+    };
+    uint8_t free_quarters_in_last_byte_;
+
+#ifdef DEBUG
+    bool is_finalized_ = false;
+#endif
+  };
+
   // Saves the information needed for allocating the Scope's (and its
   // subscopes') variables.
-  void SaveScopeAllocationData(DeclarationScope* scope);
+  void SaveScopeAllocationData(DeclarationScope* scope, Parser* parser);
 
   // In some cases, PreParser cannot produce the same Scope structure as
   // Parser. If it happens, we're unable to produce the data that would enable
@@ -116,8 +171,9 @@ class PreparseDataBuilder : public ZoneObject {
   }
 #endif  // DEBUG
 
-  bool ContainsInnerFunctions() const;
+  bool HasInnerFunctions() const;
   bool HasData() const;
+  bool HasDataForParent() const;
 
   static bool ScopeNeedsData(Scope* scope);
   static bool ScopeIsSkippableFunctionScope(Scope* scope);
@@ -135,14 +191,21 @@ class PreparseDataBuilder : public ZoneObject {
   void SaveDataForScope(Scope* scope);
   void SaveDataForVariable(Variable* var);
   void SaveDataForInnerScopes(Scope* scope);
+  bool SaveDataForSkippableFunction(PreparseDataBuilder* builder);
+
+  void CopyByteData(Zone* zone);
 
   PreparseDataBuilder* parent_;
+  ByteData byte_data_;
+  ZoneChunkList<PreparseDataBuilder*> children_;
 
-  ByteData* byte_data_;
-  ZoneChunkList<PreparseDataBuilder*> data_for_inner_functions_;
+  DeclarationScope* function_scope_;
+  int num_inner_functions_;
+  int num_inner_with_data_;
 
   // Whether we've given up producing the data for this function.
-  bool bailed_out_;
+  bool bailed_out_ : 1;
+  bool has_data_ : 1;
 
   DISALLOW_COPY_AND_ASSIGN(PreparseDataBuilder);
 };

src/parsing/preparser.cc

@@ -342,8 +342,8 @@ PreParser::Expression PreParser::ParseFunctionLiteral(
       CheckStrictOctalLiteral(start_position, end_position());
     }
     if (skippable_function) {
-      preparse_data_builder_scope.AddSkippableFunction(
-          function_scope, end_position(), GetLastFunctionLiteralId() - func_id);
+      preparse_data_builder_scope.SetSkippableFunction(
+          function_scope, GetLastFunctionLiteralId() - func_id);
     }
   }
 

test/cctest/parsing/test-preparser.cc

@@ -801,7 +801,9 @@ TEST(ProducingAndConsumingByteData) {
   LocalContext env;
 
   i::Zone zone(isolate->allocator(), ZONE_NAME);
-  i::PreparseDataBuilder::ByteData bytes(&zone);
+  std::vector<uint8_t> buffer;
+  i::PreparseDataBuilder::ByteData bytes;
+  bytes.Start(&buffer);
   // Write some data.
   bytes.WriteUint32(1983);  // This will be overwritten.
   bytes.WriteUint32(2147483647);
@@ -810,7 +812,9 @@ TEST(ProducingAndConsumingByteData) {
   bytes.WriteUint32(0);
   bytes.WriteUint8(0);
 #ifdef DEBUG
-  bytes.OverwriteFirstUint32(2017);
+  bytes.SaveCurrentSizeAtFirstUint32();
+  int saved_size = 21;
+  CHECK_EQ(buffer.size(), saved_size);
 #endif
   bytes.WriteUint8(100);
   // Write quarter bytes between uint8s and uint32s to verify they're stored
@@ -828,16 +832,35 @@ TEST(ProducingAndConsumingByteData) {
   // End with a lonely quarter.
   bytes.WriteQuarter(2);
 
+#ifdef DEBUG
+  CHECK_EQ(buffer.size(), 38);
+#else
+  CHECK_EQ(buffer.size(), 25);
+#endif
+
+  // Copy buffer for sanity checks later-on.
+  std::vector<uint8_t> copied_buffer(buffer);
+
+  // Move the data from the temporary buffer into the zone for later
+  // serialization.
+  bytes.Finalize(&zone);
+  CHECK_EQ(buffer.size(), 0);
+  CHECK_LT(0, copied_buffer.size());
+
   {
     // Serialize as a ZoneConsumedPreparseData, and read back data.
-    i::ZonePreparseData zone_serialized(&zone, &bytes, 0);
+    i::ZonePreparseData* data_in_zone = bytes.CopyToZone(&zone, 0);
     i::ZoneConsumedPreparseData::ByteData bytes_for_reading;
-    i::ZoneVectorWrapper wrapper(zone_serialized.byte_data());
+    i::ZoneVectorWrapper wrapper(data_in_zone->byte_data());
     i::ZoneConsumedPreparseData::ByteData::ReadingScope reading_scope(
         &bytes_for_reading, wrapper);
 
+    for (int i = 0; i < static_cast<int>(copied_buffer.size()); i++) {
+      CHECK_EQ(copied_buffer.at(i), wrapper.get(i));
+    }
+
 #ifdef DEBUG
-    CHECK_EQ(bytes_for_reading.ReadUint32(), 2017);
+    CHECK_EQ(bytes_for_reading.ReadUint32(), saved_size);
 #else
     CHECK_EQ(bytes_for_reading.ReadUint32(), 1983);
 #endif
@@ -858,19 +881,25 @@ TEST(ProducingAndConsumingByteData) {
     CHECK_EQ(bytes_for_reading.ReadQuarter(), 2);
     CHECK_EQ(bytes_for_reading.ReadUint32(), 50);
     CHECK_EQ(bytes_for_reading.ReadQuarter(), 2);
+    // We should have consumed all data at this point.
+    CHECK(!bytes_for_reading.HasRemainingBytes(1));
   }
 
   {
     // Serialize as an OnHeapConsumedPreparseData, and read back data.
-    i::Handle<i::PreparseData> data_on_heap =
-        isolate->factory()->NewPreparseData(static_cast<int>(bytes.size()), 0);
-    bytes.StoreInto(*data_on_heap);
+    i::Handle<i::PreparseData> data_on_heap = bytes.CopyToHeap(isolate, 0);
+    CHECK_EQ(copied_buffer.size(), data_on_heap->data_length());
+    CHECK_EQ(data_on_heap->children_length(), 0);
     i::OnHeapConsumedPreparseData::ByteData bytes_for_reading;
     i::OnHeapConsumedPreparseData::ByteData::ReadingScope reading_scope(
         &bytes_for_reading, *data_on_heap);
 
+    for (int i = 0; i < static_cast<int>(copied_buffer.size()); i++) {
+      CHECK_EQ(copied_buffer[i], data_on_heap->get(i));
+    }
+
 #ifdef DEBUG
-    CHECK_EQ(bytes_for_reading.ReadUint32(), 2017);
+    CHECK_EQ(bytes_for_reading.ReadUint32(), saved_size);
 #else
     CHECK_EQ(bytes_for_reading.ReadUint32(), 1983);
 #endif
@@ -891,5 +920,7 @@ TEST(ProducingAndConsumingByteData) {
     CHECK_EQ(bytes_for_reading.ReadQuarter(), 2);
     CHECK_EQ(bytes_for_reading.ReadUint32(), 50);
     CHECK_EQ(bytes_for_reading.ReadQuarter(), 2);
+    // We should have consumed all data at this point.
+    CHECK(!bytes_for_reading.HasRemainingBytes(1));
   }
 }