[parsing] Always stack-allocate DataGatheringScopes

Avoiding malloc and unique_ptr seems to have positive effects on a
representative facebook and cnn workload.
This makes arrow functions and eager functions potentially more
expensive since the DataGatheringScope is now always stack-allocated
and only the full initialization happens conditionally.

Change-Id: Ibf1c1308a7db464f7c5d2bafd61560e4cabf0ce9
Reviewed-on: https://chromium-review.googlesource.com/c/1382733Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Commit-Queue: Camillo Bruni <cbruni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58340}

src/parsing/preparsed-scope-data.cc

@@ -42,10 +42,10 @@ STATIC_ASSERT(LanguageModeSize <= LanguageField::kNumValues);
   ------------------------------------
   | scope_data_start (debug only)    |
   ------------------------------------
-  | data for inner function 1        |
+  | data for inner function n        |
   | ...                              |
   ------------------------------------
-  | data for inner function n        |
+  | data for inner function 1        |
   | ...                              |
   ------------------------------------
   (Scope allocation data:)             << scope_data_start points here in debug
@@ -62,12 +62,12 @@ STATIC_ASSERT(LanguageModeSize <= LanguageField::kNumValues);
   | ----------------------           |
   ------------------------------------
   ------------------------------------
-  | data for inner scope 1           | << but not for function scopes
+  | data for inner scope m           | << but not for function scopes
   | ...                              |
   ------------------------------------
   ...
   ------------------------------------
-  | data for inner scope m           |
+  | data for inner scope 1           |
   | ...                              |
   ------------------------------------
 
@@ -161,17 +161,18 @@ PreParsedScopeDataBuilder::PreParsedScopeDataBuilder(
 #endif
 }
 
-PreParsedScopeDataBuilder::DataGatheringScope::DataGatheringScope(
-    DeclarationScope* function_scope, PreParser* preparser)
-    : preparser_(preparser), builder_(nullptr) {
-  PreParsedScopeDataBuilder* parent = preparser->preparsed_scope_data_builder();
-  Zone* main_zone = preparser->main_zone();
+void PreParsedScopeDataBuilder::DataGatheringScope::Start(
+    DeclarationScope* function_scope) {
+  PreParsedScopeDataBuilder* parent =
+      preparser_->preparsed_scope_data_builder();
+  Zone* main_zone = preparser_->main_zone();
   builder_ = new (main_zone) PreParsedScopeDataBuilder(main_zone, parent);
-  preparser->set_preparsed_scope_data_builder(builder_);
+  preparser_->set_preparsed_scope_data_builder(builder_);
   function_scope->set_preparsed_scope_data_builder(builder_);
 }
 
 PreParsedScopeDataBuilder::DataGatheringScope::~DataGatheringScope() {
+  if (builder_ == nullptr) return;
   preparser_->set_preparsed_scope_data_builder(builder_->parent_);
 }
 

src/parsing/preparsed-scope-data.h

@@ -77,7 +77,10 @@ class PreParsedScopeDataBuilder : public ZoneObject {
   // ProducedPreParsedScopeData, and so do all lazy functions inside it.
   class DataGatheringScope {
    public:
-    DataGatheringScope(DeclarationScope* function_scope, PreParser* preparser);
+    explicit DataGatheringScope(PreParser* preparser)
+        : preparser_(preparser), builder_(nullptr) {}
+
+    void Start(DeclarationScope* function_scope);
     ~DataGatheringScope();
 
    private:
@@ -96,7 +99,6 @@ class PreParsedScopeDataBuilder : public ZoneObject {
   // skipping the inner functions of that function.
   void Bailout() {
     bailed_out_ = true;
-
     // We don't need to call Bailout on existing / future children: the only way
     // to try to retrieve their data is through calling Serialize on the parent,
     // and if the parent is bailed out, it won't call Serialize on its children.
@@ -106,12 +108,8 @@ class PreParsedScopeDataBuilder : public ZoneObject {
 
 #ifdef DEBUG
   bool ThisOrParentBailedOut() const {
-    if (bailed_out_) {
-      return true;
-    }
-    if (parent_ == nullptr) {
-      return false;
-    }
+    if (bailed_out_) return true;
+    if (parent_ == nullptr) return false;
     return parent_->ThisOrParentBailedOut();
   }
 #endif  // DEBUG

src/parsing/preparser.cc

@@ -118,12 +118,10 @@ PreParser::PreParseResult PreParser::PreParseFunction(
 
   // Start collecting data for a new function which might contain skippable
   // functions.
-  std::unique_ptr<PreParsedScopeDataBuilder::DataGatheringScope>
-      preparsed_scope_data_builder_scope;
+  PreParsedScopeDataBuilder::DataGatheringScope
+      preparsed_scope_data_builder_scope(this);
   if (!IsArrowFunction(kind)) {
-    preparsed_scope_data_builder_scope.reset(
-        new PreParsedScopeDataBuilder::DataGatheringScope(function_scope,
-                                                          this));
+    preparsed_scope_data_builder_scope.Start(function_scope);
   }
 
   // In the preparser, we use the function literal ids to count how many
@@ -288,14 +286,12 @@ PreParser::Expression PreParser::ParseFunctionLiteral(
   // Start collecting data for a new function which might contain skippable
   // functions.
   {
-    std::unique_ptr<PreParsedScopeDataBuilder::DataGatheringScope>
-        preparsed_scope_data_builder_scope;
-    if (!function_state_->next_function_is_likely_called() &&
-        preparsed_scope_data_builder_ != nullptr) {
-      skippable_function = true;
-      preparsed_scope_data_builder_scope.reset(
-          new PreParsedScopeDataBuilder::DataGatheringScope(function_scope,
-                                                            this));
+    PreParsedScopeDataBuilder::DataGatheringScope
+        preparsed_scope_data_builder_scope(this);
+    skippable_function = !function_state_->next_function_is_likely_called() &&
+                         preparsed_scope_data_builder_ != nullptr;
+    if (skippable_function) {
+      preparsed_scope_data_builder_scope.Start(function_scope);
     }
 
     FunctionState function_state(&function_state_, &scope_, function_scope);