[parser] Store function.length computed in the preparser

Previously we'd need to eagerly compile upon access to function.length for a
lazy function. The preparser already computes function.length, however, so we
can store that information in the already available preparse data.

Change-Id: I19007c9db5839e8038291fb4433866303935f089
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1564190
Commit-Queue: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#60767}

src/accessors.cc

@@ -370,10 +370,7 @@ void Accessors::FunctionLengthGetter(
   HandleScope scope(isolate);
   Handle<JSFunction> function =
       Handle<JSFunction>::cast(Utils::OpenHandle(*info.Holder()));
-  int length = 0;
-  if (!JSFunction::GetLength(isolate, function).To(&length)) {
-    isolate->OptionalRescheduleException(false);
-  }
+  int length = function->length();
   Handle<Object> result(Smi::FromInt(length), isolate);
   info.GetReturnValue().Set(Utils::ToLocal(result));
 }

src/bootstrapper.cc

@@ -660,8 +660,7 @@ Handle<JSFunction> Genesis::GetThrowTypeErrorIntrinsic() {
   }
 
   // length needs to be non configurable.
-  Handle<Object> value(Smi::FromInt(function->shared()->GetLength()),
-                       isolate());
+  Handle<Object> value(Smi::FromInt(function->length()), isolate());
   JSObject::SetOwnPropertyIgnoreAttributes(
       function, factory()->length_string(), value,
       static_cast<PropertyAttributes>(DONT_ENUM | DONT_DELETE | READ_ONLY))

src/compiler.cc

@@ -441,10 +441,6 @@ void EnsureSharedFunctionInfosArrayOnScript(ParseInfo* parse_info,
 
 void SetSharedFunctionFlagsFromLiteral(FunctionLiteral* literal,
                                        Handle<SharedFunctionInfo> shared_info) {
-  // Don't overwrite values set by the bootstrapper.
-  if (!shared_info->HasLength()) {
-    shared_info->set_length(literal->function_length());
-  }
   shared_info->set_has_duplicate_parameters(
       literal->has_duplicate_parameters());
   shared_info->set_is_oneshot_iife(literal->is_oneshot_iife());

src/objects.cc

@@ -5487,12 +5487,13 @@ void SharedFunctionInfo::InitFromFunctionLiteral(
     }
   }
 
+  shared_info->set_length(lit->function_length());
+
   // For lazy parsed functions, the following flags will be inaccurate since we
   // don't have the information yet. They're set later in
   // SetSharedFunctionFlagsFromLiteral (compiler.cc), when the function is
   // really parsed and compiled.
   if (lit->ShouldEagerCompile()) {
-    shared_info->set_length(lit->function_length());
     shared_info->set_has_duplicate_parameters(lit->has_duplicate_parameters());
     shared_info->SetExpectedNofPropertiesFromEstimate(lit);
     shared_info->set_is_safe_to_skip_arguments_adaptor(
@@ -5504,10 +5505,6 @@ void SharedFunctionInfo::InitFromFunctionLiteral(
     // than relying on a property of the literal.
     needs_position_info = false;
   } else {
-    // Set an invalid length for lazy functions. This way we can set the correct
-    // value after compiling, but avoid overwriting values set manually by the
-    // bootstrapper.
-    shared_info->set_length(SharedFunctionInfo::kInvalidLength);
     shared_info->set_is_safe_to_skip_arguments_adaptor(false);
     ProducedPreparseData* scope_data = lit->produced_preparse_data();
     if (scope_data != nullptr) {

src/objects/js-objects-inl.h

@@ -541,6 +541,8 @@ AbstractCode JSFunction::abstract_code() {
   }
 }
 
+int JSFunction::length() { return shared()->length(); }
+
 Code JSFunction::code() const {
   return Code::cast(RELAXED_READ_FIELD(*this, kCodeOffset));
 }

src/objects/js-objects.cc

@@ -4860,10 +4860,9 @@ Maybe<int> JSBoundFunction::GetLength(Isolate* isolate,
   // accessor.
   Handle<JSFunction> target(JSFunction::cast(function->bound_target_function()),
                             isolate);
-  Maybe<int> target_length = JSFunction::GetLength(isolate, target);
-  if (target_length.IsNothing()) return target_length;
+  int target_length = target->length();
 
-  int length = Max(0, target_length.FromJust() - nof_bound_arguments);
+  int length = Max(0, target_length - nof_bound_arguments);
   return Just(length);
 }
 
@@ -4882,26 +4881,6 @@ Handle<Object> JSFunction::GetName(Isolate* isolate,
   return handle(function->shared()->Name(), isolate);
 }
 
-// static
-Maybe<int> JSFunction::GetLength(Isolate* isolate,
-                                 Handle<JSFunction> function) {
-  int length = 0;
-  IsCompiledScope is_compiled_scope(function->shared()->is_compiled_scope());
-  if (is_compiled_scope.is_compiled()) {
-    length = function->shared()->GetLength();
-  } else {
-    // If the function isn't compiled yet, the length is not computed
-    // correctly yet. Compile it now and return the right length.
-    if (Compiler::Compile(function, Compiler::KEEP_EXCEPTION,
-                          &is_compiled_scope)) {
-      length = function->shared()->GetLength();
-    }
-    if (isolate->has_pending_exception()) return Nothing<int>();
-  }
-  DCHECK_GE(length, 0);
-  return Just(length);
-}
-
 // static
 Handle<NativeContext> JSFunction::GetFunctionRealm(
     Handle<JSFunction> function) {

src/objects/js-objects.h

@@ -953,9 +953,9 @@ class JSFunction : public JSObject {
   inline void set_context(Object context);
   inline JSGlobalProxy global_proxy();
   inline NativeContext native_context();
+  inline int length();
 
   static Handle<Object> GetName(Isolate* isolate, Handle<JSFunction> function);
-  static Maybe<int> GetLength(Isolate* isolate, Handle<JSFunction> function);
   static Handle<NativeContext> GetFunctionRealm(Handle<JSFunction> function);
 
   // [code]: The generated code object for this function.  Executed

src/objects/shared-function-info-inl.h

@@ -427,16 +427,6 @@ IsCompiledScope::IsCompiledScope(const SharedFunctionInfo shared,
   DCHECK_IMPLIES(!retain_bytecode_.is_null(), is_compiled());
 }
 
-uint16_t SharedFunctionInfo::GetLength() const {
-  DCHECK(is_compiled());
-  DCHECK(HasLength());
-  return length();
-}
-
-bool SharedFunctionInfo::HasLength() const {
-  return length() != kInvalidLength;
-}
-
 bool SharedFunctionInfo::has_simple_parameters() {
   return scope_info()->HasSimpleParameters();
 }

src/objects/shared-function-info.h

@@ -263,7 +263,6 @@ class SharedFunctionInfo : public HeapObject {
   static const int kInitialLength = kEntriesStart + kEntryLength;
 
   static const int kNotFound = -1;
-  static const uint16_t kInvalidLength = static_cast<uint16_t>(-1);
 
   // [scope_info]: Scope info.
   DECL_ACCESSORS(scope_info, ScopeInfo)
@@ -306,8 +305,7 @@ class SharedFunctionInfo : public HeapObject {
   // Use up to 2^16-2 parameters (16 bits of values, where one is reserved for
   // kDontAdaptArgumentsSentinel). The value is only reliable when the function
   // has been compiled.
-  inline uint16_t GetLength() const;
-  inline bool HasLength() const;
+  inline uint16_t length() const;
   inline void set_length(int value);
 
   // [internal formal parameter count]: The declared number of parameters.
@@ -730,8 +728,6 @@ class SharedFunctionInfo : public HeapObject {
   friend class V8HeapExplorer;
   FRIEND_TEST(PreParserTest, LazyFunctionLength);
 
-  inline uint16_t length() const;
-
   // Find the index of this function in the parent script. Slow path of
   // FunctionLiteralId.
   int FindIndexInScript(Isolate* isolate) const;

src/parsing/parser-base.h

@@ -4116,11 +4116,12 @@ ParserBase<Impl>::ParseArrowFunctionLiteral(
         // For arrow functions, we don't need to retrieve data about function
         // parameters.
         int dummy_num_parameters = -1;
+        int dummy_function_length = -1;
         DCHECK_NE(kind & FunctionKind::kArrowFunction, 0);
         bool did_preparse_successfully = impl()->SkipFunction(
             nullptr, kind, FunctionLiteral::kAnonymousExpression,
             formal_parameters.scope, &dummy_num_parameters,
-            &produced_preparse_data);
+            &dummy_function_length, &produced_preparse_data);
 
         DCHECK_NULL(produced_preparse_data);
 

src/parsing/parser.cc

@@ -2393,8 +2393,9 @@ FunctionLiteral* Parser::ParseFunctionLiteral(
   // which case the parser is expected to have backtracked), or if we didn't
   // try to lazy parse in the first place, we'll have to parse eagerly.
   bool did_preparse_successfully =
-      should_preparse && SkipFunction(function_name, kind, function_type, scope,
-                                      &num_parameters, &produced_preparse_data);
+      should_preparse &&
+      SkipFunction(function_name, kind, function_type, scope, &num_parameters,
+                   &function_length, &produced_preparse_data);
 
   if (!did_preparse_successfully) {
     // If skipping aborted, it rewound the scanner until before the LPAREN.
@@ -2466,6 +2467,7 @@ FunctionLiteral* Parser::ParseFunctionLiteral(
 bool Parser::SkipFunction(const AstRawString* function_name, FunctionKind kind,
                           FunctionLiteral::FunctionType function_type,
                           DeclarationScope* function_scope, int* num_parameters,
+                          int* function_length,
                           ProducedPreparseData** produced_preparse_data) {
   FunctionState function_state(&function_state_, &scope_, function_scope);
   function_scope->set_zone(&preparser_zone_);
@@ -2486,8 +2488,8 @@ bool Parser::SkipFunction(const AstRawString* function_name, FunctionKind kind,
     *produced_preparse_data =
         consumed_preparse_data_->GetDataForSkippableFunction(
             main_zone(), function_scope->start_position(), &end_position,
-            num_parameters, &num_inner_functions, &uses_super_property,
-            &language_mode);
+            num_parameters, function_length, &num_inner_functions,
+            &uses_super_property, &language_mode);
 
     function_scope->outer_scope()->SetMustUsePreparseData();
     function_scope->set_is_skipped_function(true);
@@ -2554,6 +2556,7 @@ bool Parser::SkipFunction(const AstRawString* function_name, FunctionKind kind,
     total_preparse_skipped_ +=
         function_scope->end_position() - function_scope->start_position();
     *num_parameters = logger->num_parameters();
+    *function_length = logger->function_length();
     SkipFunctionLiterals(logger->num_inner_functions());
     if (closest_class_scope != nullptr) {
       closest_class_scope->MigrateUnresolvedPrivateNameTail(

src/parsing/parser.h

@@ -32,50 +32,6 @@ class ParserTargetScope;
 class PendingCompilationErrorHandler;
 class PreparseData;
 
-class FunctionEntry {
- public:
-  enum {
-    kStartPositionIndex,
-    kEndPositionIndex,
-    kNumParametersIndex,
-    kFlagsIndex,
-    kNumInnerFunctionsIndex,
-    kSize
-  };
-
-  explicit FunctionEntry(Vector<unsigned> backing)
-    : backing_(backing) { }
-
-  FunctionEntry() : backing_() { }
-
-  class LanguageModeField : public BitField<LanguageMode, 0, 1> {};
-  class UsesSuperPropertyField
-      : public BitField<bool, LanguageModeField::kNext, 1> {};
-
-  static uint32_t EncodeFlags(LanguageMode language_mode,
-                              bool uses_super_property) {
-    return LanguageModeField::encode(language_mode) |
-           UsesSuperPropertyField::encode(uses_super_property);
-  }
-
-  int start_pos() const { return backing_[kStartPositionIndex]; }
-  int end_pos() const { return backing_[kEndPositionIndex]; }
-  int num_parameters() const { return backing_[kNumParametersIndex]; }
-  LanguageMode language_mode() const {
-    return LanguageModeField::decode(backing_[kFlagsIndex]);
-  }
-  bool uses_super_property() const {
-    return UsesSuperPropertyField::decode(backing_[kFlagsIndex]);
-  }
-  int num_inner_functions() const { return backing_[kNumInnerFunctionsIndex]; }
-
-  bool is_valid() const { return !backing_.empty(); }
-
- private:
-  Vector<unsigned> backing_;
-};
-
-
 // ----------------------------------------------------------------------------
 // JAVASCRIPT PARSING
 
@@ -455,6 +411,7 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
   bool SkipFunction(const AstRawString* function_name, FunctionKind kind,
                     FunctionLiteral::FunctionType function_type,
                     DeclarationScope* function_scope, int* num_parameters,
+                    int* function_length,
                     ProducedPreparseData** produced_preparsed_scope_data);
 
   Block* BuildParameterInitializationBlock(

src/parsing/preparse-data-impl.h

@@ -152,7 +152,7 @@ class BaseConsumedPreparseData : public ConsumedPreparseData {
 
   ProducedPreparseData* GetDataForSkippableFunction(
       Zone* zone, int start_position, int* end_position, int* num_parameters,
-      int* num_inner_functions, bool* uses_super_property,
+      int* function_length, int* num_inner_functions, bool* uses_super_property,
       LanguageMode* language_mode) final;
 
   void RestoreScopeAllocationData(DeclarationScope* scope) final;

src/parsing/preparse-data.cc

@@ -30,8 +30,10 @@ class VariableContextAllocatedField
     : public BitField8<bool, VariableMaybeAssignedField::kNext, 1> {};
 
 class HasDataField : public BitField<bool, 0, 1> {};
+class LengthEqualsParametersField
+    : public BitField<bool, HasDataField::kNext, 1> {};
 class NumberOfParametersField
-    : public BitField<uint16_t, HasDataField::kNext, 16> {};
+    : public BitField<uint16_t, LengthEqualsParametersField::kNext, 16> {};
 
 class LanguageField : public BitField8<LanguageMode, 0, 1> {};
 class UsesSuperField : public BitField8<bool, LanguageField::kNext, 1> {};
@@ -91,6 +93,7 @@ PreparseDataBuilder::PreparseDataBuilder(Zone* zone,
       byte_data_(),
       children_buffer_(children_buffer),
       function_scope_(nullptr),
+      function_length_(-1),
       num_inner_functions_(0),
       num_inner_with_data_(0),
       bailed_out_(false),
@@ -220,10 +223,12 @@ void PreparseDataBuilder::ByteData::WriteQuarter(uint8_t data) {
 }
 
 void PreparseDataBuilder::DataGatheringScope::SetSkippableFunction(
-    DeclarationScope* function_scope, int num_inner_functions) {
+    DeclarationScope* function_scope, int function_length,
+    int num_inner_functions) {
   DCHECK_NULL(builder_->function_scope_);
   builder_->function_scope_ = function_scope;
   DCHECK_EQ(builder_->num_inner_functions_, 0);
+  builder_->function_length_ = function_length;
   builder_->num_inner_functions_ = num_inner_functions;
   builder_->parent_->has_data_ = true;
 }
@@ -281,10 +286,16 @@ bool PreparseDataBuilder::SaveDataForSkippableFunction(
   byte_data_.WriteVarint32(function_scope->end_position());
 
   bool has_data = builder->HasData();
+  bool length_equals_parameters =
+      function_scope->num_parameters() == builder->function_length_;
   uint32_t has_data_and_num_parameters =
       HasDataField::encode(has_data) |
+      LengthEqualsParametersField::encode(length_equals_parameters) |
       NumberOfParametersField::encode(function_scope->num_parameters());
   byte_data_.WriteVarint32(has_data_and_num_parameters);
+  if (!length_equals_parameters) {
+    byte_data_.WriteVarint32(builder->function_length_);
+  }
   byte_data_.WriteVarint32(builder->num_inner_functions_);
 
   uint8_t language_and_super =
@@ -515,7 +526,7 @@ template <class Data>
 ProducedPreparseData*
 BaseConsumedPreparseData<Data>::GetDataForSkippableFunction(
     Zone* zone, int start_position, int* end_position, int* num_parameters,
-    int* num_inner_functions, bool* uses_super_property,
+    int* function_length, int* num_inner_functions, bool* uses_super_property,
     LanguageMode* language_mode) {
   // The skippable function *must* be the next function in the data. Use the
   // start position as a sanity check.
@@ -531,6 +542,13 @@ BaseConsumedPreparseData<Data>::GetDataForSkippableFunction(
   bool has_data = HasDataField::decode(has_data_and_num_parameters);
   *num_parameters =
       NumberOfParametersField::decode(has_data_and_num_parameters);
+  bool length_equals_parameters =
+      LengthEqualsParametersField::decode(has_data_and_num_parameters);
+  if (length_equals_parameters) {
+    *function_length = *num_parameters;
+  } else {
+    *function_length = scope_data_->ReadVarint32();
+  }
   *num_inner_functions = scope_data_->ReadVarint32();
 
   uint8_t language_and_super = scope_data_->ReadQuarter();

src/parsing/preparse-data.h

@@ -110,7 +110,7 @@ class V8_EXPORT_PRIVATE PreparseDataBuilder : public ZoneObject,
 
     void Start(DeclarationScope* function_scope);
     void SetSkippableFunction(DeclarationScope* function_scope,
-                              int num_inner_functions);
+                              int function_length, int num_inner_functions);
     inline ~DataGatheringScope() {
       if (builder_ == nullptr) return;
       Close();
@@ -200,10 +200,6 @@ class V8_EXPORT_PRIVATE PreparseDataBuilder : public ZoneObject,
   bool HasDataForParent() const;
 
   static bool ScopeNeedsData(Scope* scope);
-  void AddSkippableFunction(int start_position, int end_position,
-                            int num_parameters, int num_inner_functions,
-                            LanguageMode language_mode, bool has_data,
-                            bool uses_super_property);
 
  private:
   friend class BuilderProducedPreparseData;
@@ -229,6 +225,7 @@ class V8_EXPORT_PRIVATE PreparseDataBuilder : public ZoneObject,
   };
 
   DeclarationScope* function_scope_;
+  int function_length_;
   int num_inner_functions_;
   int num_inner_with_data_;
 
@@ -284,7 +281,7 @@ class ConsumedPreparseData {
 
   virtual ProducedPreparseData* GetDataForSkippableFunction(
       Zone* zone, int start_position, int* end_position, int* num_parameters,
-      int* num_inner_functions, bool* uses_super_property,
+      int* function_length, int* num_inner_functions, bool* uses_super_property,
       LanguageMode* language_mode) = 0;
 
   // Restores the information needed for allocating the Scope's (and its

src/parsing/preparser-logger.h

@@ -10,22 +10,30 @@ namespace internal {
 
 class PreParserLogger final {
  public:
-  PreParserLogger() : end_(-1), num_parameters_(-1), num_inner_functions_(-1) {}
-
-  void LogFunction(int end, int num_parameters, int num_inner_functions) {
+  PreParserLogger()
+      : end_(-1),
+        num_parameters_(-1),
+        function_length_(-1),
+        num_inner_functions_(-1) {}
+
+  void LogFunction(int end, int num_parameters, int function_length,
+                   int num_inner_functions) {
     end_ = end;
     num_parameters_ = num_parameters;
+    function_length_ = function_length;
     num_inner_functions_ = num_inner_functions;
   }
 
   int end() const { return end_; }
   int num_parameters() const { return num_parameters_; }
+  int function_length() const { return function_length_; }
   int num_inner_functions() const { return num_inner_functions_; }
 
  private:
   int end_;
   // For function entries.
   int num_parameters_;
+  int function_length_;
   int num_inner_functions_;
 };
 

src/parsing/preparser.cc

@@ -345,7 +345,8 @@ PreParser::Expression PreParser::ParseFunctionLiteral(
     }
     if (skippable_function) {
       preparse_data_builder_scope.SetSkippableFunction(
-          function_scope, GetLastFunctionLiteralId() - func_id);
+          function_scope, formals.function_length,
+          GetLastFunctionLiteralId() - func_id);
     }
   }
 
@@ -379,7 +380,7 @@ void PreParser::ParseStatementListAndLogFunction(
   int body_end = scanner()->peek_location().end_pos;
   DCHECK_EQ(this->scope()->is_function_scope(), formals->is_simple);
   log_.LogFunction(body_end, formals->num_parameters(),
-                   GetLastFunctionLiteralId());
+                   formals->function_length, GetLastFunctionLiteralId());
 }
 
 PreParserBlock PreParser::BuildParameterInitializationBlock(

src/parsing/preparser.h

@@ -1012,7 +1012,7 @@ class PreParser : public ParserBase<PreParser> {
   V8_INLINE bool SkipFunction(const AstRawString* name, FunctionKind kind,
                               FunctionLiteral::FunctionType function_type,
                               DeclarationScope* function_scope,
-                              int* num_parameters,
+                              int* num_parameters, int* function_length,
                               ProducedPreparseData** produced_preparse_data) {
     UNREACHABLE();
   }

test/unittests/parser/preparser-unittest.cc

@@ -26,7 +26,7 @@ TEST_F(PreParserTest, LazyFunctionLength) {
 
   Handle<SharedFunctionInfo> shared(lazy_function->shared(),
                                     lazy_function->GetIsolate());
-  CHECK_EQ(shared->length(), SharedFunctionInfo::kInvalidLength);
+  CHECK_EQ(3, shared->length());
 
   Handle<Smi> length = RunJS<Smi>("lazy.length");
   int32_t value;