[parser] Speed up variable allocation in Scopes

- Inline single-call-site functions
- Pull out pre-check from AllocateVariablesRecursively

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

src/ast/scopes.cc

@@ -319,10 +319,6 @@ bool Scope::HasSimpleParameters() {
   return !scope->is_function_scope() || scope->has_simple_parameters();
 }
 
-bool DeclarationScope::ShouldEagerCompile() const {
-  return force_eager_compilation_ || should_eager_compile_;
-}
-
 void DeclarationScope::set_should_eager_compile() {
   should_eager_compile_ = !was_lazily_parsed_;
 }
@@ -1197,7 +1193,9 @@ bool DeclarationScope::AllocateVariables(ParseInfo* info) {
     DCHECK(info->pending_error_handler()->has_pending_error());
     return false;
   }
-  AllocateVariablesRecursively();
+
+  // // Don't allocate variables of preparsed scopes.
+  if (!was_lazily_parsed()) AllocateVariablesRecursively();
 
   return true;
 }
@@ -1310,6 +1308,13 @@ Scope* Scope::GetOuterScopeWithContext() {
   return scope;
 }
 
+namespace {
+bool WasLazilyParsed(Scope* scope) {
+  return scope->is_declaration_scope() &&
+         scope->AsDeclarationScope()->was_lazily_parsed();
+}
+}  // namespace
+
 void Scope::CollectNonLocals(DeclarationScope* max_outer_scope,
                              Isolate* isolate, ParseInfo* info,
                              Handle<StringSet>* non_locals) {
@@ -1320,10 +1325,7 @@ void Scope::CollectNonLocals(DeclarationScope* max_outer_scope,
   // Lazy parsed declaration scopes are already partially analyzed. If there are
   // unresolved references remaining, they just need to be resolved in outer
   // scopes.
-  Scope* lookup =
-      is_declaration_scope() && AsDeclarationScope()->was_lazily_parsed()
-          ? outer_scope()
-          : this;
+  Scope* lookup = WasLazilyParsed(this) ? outer_scope() : this;
 
   for (VariableProxy* proxy : unresolved_list_) {
     DCHECK(!proxy->is_resolved());
@@ -1428,8 +1430,20 @@ void DeclarationScope::ResetAfterPreparsing(AstValueFactory* ast_value_factory,
   was_lazily_parsed_ = !aborted;
 }
 
+bool Scope::IsSkippableFunctionScope() {
+  // Lazy non-arrow function scopes are skippable. Lazy functions are exactly
+  // those Scopes which have their own PreparseDataBuilder object. This
+  // logic ensures that the scope allocation data is consistent with the
+  // skippable function data (both agree on where the lazy function boundaries
+  // are).
+  if (!is_function_scope()) return false;
+  DeclarationScope* declaration_scope = AsDeclarationScope();
+  return !declaration_scope->is_arrow_scope() &&
+         declaration_scope->preparse_data_builder() != nullptr;
+}
+
 void Scope::SavePreparseData(Parser* parser) {
-  if (PreparseDataBuilder::ScopeIsSkippableFunctionScope(this)) {
+  if (IsSkippableFunctionScope()) {
     AsDeclarationScope()->SavePreparseDataForDeclarationScope(parser);
   }
 
@@ -1698,8 +1712,7 @@ void Scope::CheckScopePositions() {
 void Scope::CheckZones() {
   DCHECK(!needs_migration_);
   for (Scope* scope = inner_scope_; scope != nullptr; scope = scope->sibling_) {
-    if (scope->is_declaration_scope() &&
-        scope->AsDeclarationScope()->was_lazily_parsed()) {
+    if (WasLazilyParsed(scope)) {
       DCHECK_NULL(scope->zone());
       DCHECK_NULL(scope->inner_scope_);
       continue;
@@ -1999,7 +2012,7 @@ bool Scope::ResolveVariablesRecursively(ParseInfo* info) {
   // Lazy parsed declaration scopes are already partially analyzed. If there are
   // unresolved references remaining, they just need to be resolved in outer
   // scopes.
-  if (is_declaration_scope() && AsDeclarationScope()->was_lazily_parsed()) {
+  if (WasLazilyParsed(this)) {
     DCHECK_EQ(variables_.occupancy(), 0);
     for (VariableProxy* proxy : unresolved_list_) {
       Variable* var = Lookup<kParsedScope>(proxy, outer_scope(), nullptr);
@@ -2118,18 +2131,15 @@ void DeclarationScope::AllocateParameterLocals() {
 }
 
 void DeclarationScope::AllocateParameter(Variable* var, int index) {
-  if (MustAllocate(var)) {
-    if (has_forced_context_allocation_for_parameters() ||
-        MustAllocateInContext(var)) {
-      DCHECK(var->IsUnallocated() || var->IsContextSlot());
-      if (var->IsUnallocated()) {
-        AllocateHeapSlot(var);
-      }
-    } else {
-      DCHECK(var->IsUnallocated() || var->IsParameter());
-      if (var->IsUnallocated()) {
-        var->AllocateTo(VariableLocation::PARAMETER, index);
-      }
+  if (!MustAllocate(var)) return;
+  if (has_forced_context_allocation_for_parameters() ||
+      MustAllocateInContext(var)) {
+    DCHECK(var->IsUnallocated() || var->IsContextSlot());
+    if (var->IsUnallocated()) AllocateHeapSlot(var);
+  } else {
+    DCHECK(var->IsUnallocated() || var->IsParameter());
+    if (var->IsUnallocated()) {
+      var->AllocateTo(VariableLocation::PARAMETER, index);
     }
   }
 }
@@ -2202,14 +2212,11 @@ void ModuleScope::AllocateModuleVariables() {
 
 void Scope::AllocateVariablesRecursively() {
   DCHECK(!already_resolved_);
-
-  // Don't allocate variables of preparsed scopes.
-  if (is_declaration_scope() && AsDeclarationScope()->was_lazily_parsed()) {
-    return;
-  }
+  DCHECK(!WasLazilyParsed(this));
 
   // Allocate variables for inner scopes.
   for (Scope* scope = inner_scope_; scope != nullptr; scope = scope->sibling_) {
+    if (WasLazilyParsed(scope)) continue;
     scope->AllocateVariablesRecursively();
   }
 
@@ -2219,9 +2226,7 @@ void Scope::AllocateVariablesRecursively() {
   // Allocate variables for this scope.
   // Parameters must be allocated first, if any.
   if (is_declaration_scope()) {
-    if (is_function_scope()) {
-      AsDeclarationScope()->AllocateParameterLocals();
-    }
+    if (is_function_scope()) AsDeclarationScope()->AllocateParameterLocals();
     AsDeclarationScope()->AllocateReceiver();
   }
   AllocateNonParameterLocalsAndDeclaredGlobals();

src/ast/scopes.h

@@ -513,6 +513,7 @@ class V8_EXPORT_PRIVATE Scope : public NON_EXPORTED_BASE(ZoneObject) {
   bool HasSimpleParameters();
   void set_is_debug_evaluate_scope() { is_debug_evaluate_scope_ = true; }
   bool is_debug_evaluate_scope() const { return is_debug_evaluate_scope_; }
+  bool IsSkippableFunctionScope();
 
   bool RemoveInnerScope(Scope* inner_scope) {
     DCHECK_NOT_NULL(inner_scope);
@@ -618,14 +619,15 @@ class V8_EXPORT_PRIVATE Scope : public NON_EXPORTED_BASE(ZoneObject) {
 
   // Variable allocation.
   void AllocateStackSlot(Variable* var);
-  void AllocateHeapSlot(Variable* var);
+  V8_INLINE void AllocateHeapSlot(Variable* var);
   void AllocateNonParameterLocal(Variable* var);
   void AllocateDeclaredGlobal(Variable* var);
-  void AllocateNonParameterLocalsAndDeclaredGlobals();
+  V8_INLINE void AllocateNonParameterLocalsAndDeclaredGlobals();
   void AllocateVariablesRecursively();
 
   void AllocateScopeInfosRecursively(Isolate* isolate,
                                      MaybeHandle<ScopeInfo> outer_scope);
+
   void AllocateDebuggerScopeInfos(Isolate* isolate,
                                   MaybeHandle<ScopeInfo> outer_scope);
 
@@ -779,7 +781,10 @@ class V8_EXPORT_PRIVATE DeclarationScope : public Scope {
     zone_ = zone;
   }
 
-  bool ShouldEagerCompile() const;
+  bool ShouldEagerCompile() const {
+    return force_eager_compilation_ || should_eager_compile_;
+  }
+
   void set_should_eager_compile();
 
   void SetScriptScopeInfo(Handle<ScopeInfo> scope_info) {
@@ -988,9 +993,9 @@ class V8_EXPORT_PRIVATE DeclarationScope : public Scope {
   void PrintParameters();
 #endif
 
-  void AllocateLocals();
-  void AllocateParameterLocals();
-  void AllocateReceiver();
+  V8_INLINE void AllocateLocals();
+  V8_INLINE void AllocateParameterLocals();
+  V8_INLINE void AllocateReceiver();
 
   void ResetAfterPreparsing(AstValueFactory* ast_value_factory, bool aborted);
 
@@ -1021,7 +1026,7 @@ class V8_EXPORT_PRIVATE DeclarationScope : public Scope {
   }
 
  private:
-  void AllocateParameter(Variable* var, int index);
+  V8_INLINE void AllocateParameter(Variable* var, int index);
 
   // Resolve and fill in the allocation information for all variables
   // in this scopes. Must be called *after* all scopes have been

src/parsing/preparse-data.cc

@@ -363,7 +363,7 @@ void PreparseDataBuilder::SaveDataForInnerScopes(Scope* scope) {
   // want to recurse here.
   for (Scope* inner = scope->inner_scope(); inner != nullptr;
        inner = inner->sibling()) {
-    if (ScopeIsSkippableFunctionScope(inner)) {
+    if (inner->IsSkippableFunctionScope()) {
       // Don't save data about function scopes, since they'll have their own
       // PreparseDataBuilder where their data is saved.
       DCHECK_NOT_NULL(inner->AsDeclarationScope()->preparse_data_builder());
@@ -374,17 +374,6 @@ void PreparseDataBuilder::SaveDataForInnerScopes(Scope* scope) {
   }
 }
 
-bool PreparseDataBuilder::ScopeIsSkippableFunctionScope(Scope* scope) {
-  // Lazy non-arrow function scopes are skippable. Lazy functions are exactly
-  // those Scopes which have their own PreparseDataBuilder object. This
-  // logic ensures that the scope allocation data is consistent with the
-  // skippable function data (both agree on where the lazy function boundaries
-  // are).
-  if (scope->scope_type() != ScopeType::FUNCTION_SCOPE) return false;
-  DeclarationScope* declaration_scope = scope->AsDeclarationScope();
-  return !declaration_scope->is_arrow_scope() &&
-         declaration_scope->preparse_data_builder() != nullptr;
-}
 
 Handle<PreparseData> PreparseDataBuilder::ByteData::CopyToHeap(
     Isolate* isolate, int children_length) {

src/parsing/preparse-data.h

@@ -180,7 +180,6 @@ class PreparseDataBuilder : public ZoneObject,
   bool HasDataForParent() const;
 
   static bool ScopeNeedsData(Scope* scope);
-  static bool ScopeIsSkippableFunctionScope(Scope* scope);
   void AddSkippableFunction(int start_position, int end_position,
                             int num_parameters, int num_inner_functions,
                             LanguageMode language_mode, bool has_data,