[ast] Cache allocated AstConsString

Allow caching the result of allocating AstConsStrings, to allow
sharing of inferred names between functions.

This is a partial revert of https://crrev.com/c/2020953, with
the observation that *some* AstConsStrings are always flattened,
while others are only ever used as ConsStrings, so we want to
allow the allocation to be lazy while still caching the result.

As a drive-by, cleanup the old AstConsString linked list fields.

Bug: chromium:1011762
Bug: chromium:1048082
Change-Id: Icc14342eb3f6f97359596b42b2c296cbc49fd791
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2042093
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Auto-Submit: Leszek Swirski <leszeks@chromium.org>
Reviewed-by: Victor Gomes <victorgomes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#66168}

src/ast/ast-value-factory.cc

@@ -162,6 +162,8 @@ bool AstRawString::Compare(void* a, void* b) {
 
 template <typename Isolate>
 HandleFor<Isolate, String> AstConsString::Allocate(Isolate* isolate) const {
+  DCHECK(string_.is_null());
+
   if (IsEmpty()) {
     return isolate->factory()->empty_string();
   }

src/ast/ast-value-factory.h

@@ -145,8 +145,12 @@ class AstConsString final : public ZoneObject {
   }
 
   template <typename Isolate>
-  EXPORT_TEMPLATE_DECLARE(V8_EXPORT_PRIVATE)
-  HandleFor<Isolate, String> Allocate(Isolate* isolate) const;
+  HandleFor<Isolate, String> GetString(Isolate* isolate) {
+    if (string_.is_null()) {
+      string_ = Allocate(isolate);
+    }
+    return string_;
+  }
 
   template <typename Isolate>
   EXPORT_TEMPLATE_DECLARE(V8_EXPORT_PRIVATE)
@@ -157,12 +161,13 @@ class AstConsString final : public ZoneObject {
  private:
   friend class AstValueFactory;
 
-  AstConsString() : next_(nullptr), segment_({nullptr, nullptr}) {}
+  AstConsString() : string_(), segment_({nullptr, nullptr}) {}
 
-  AstConsString* next() const { return next_; }
-  AstConsString** next_location() { return &next_; }
+  template <typename Isolate>
+  EXPORT_TEMPLATE_DECLARE(V8_EXPORT_PRIVATE)
+  HandleFor<Isolate, String> Allocate(Isolate* isolate) const;
 
-  AstConsString* next_;
+  HandleOrOffThreadHandle<String> string_;
 
   // A linked list of AstRawStrings of the contents of this AstConsString.
   // This list has several properties:
@@ -321,7 +326,7 @@ class AstValueFactory {
   }
   AST_STRING_CONSTANTS(F)
 #undef F
-  const AstConsString* empty_cons_string() const { return empty_cons_string_; }
+  AstConsString* empty_cons_string() const { return empty_cons_string_; }
 
  private:
   AstRawString* AddString(AstRawString* string) {
@@ -347,7 +352,8 @@ class AstValueFactory {
 
   // Holds constant string values which are shared across the isolate.
   const AstStringConstants* string_constants_;
-  const AstConsString* empty_cons_string_;
+
+  AstConsString* empty_cons_string_;
 
   // Caches one character lowercase strings (for minified code).
   static const int kMaxOneCharStringValue = 128;

src/ast/ast.cc

@@ -205,8 +205,7 @@ void FunctionLiteral::set_inferred_name(Handle<String> inferred_name) {
   scope()->set_has_inferred_function_name(true);
 }
 
-void FunctionLiteral::set_raw_inferred_name(
-    const AstConsString* raw_inferred_name) {
+void FunctionLiteral::set_raw_inferred_name(AstConsString* raw_inferred_name) {
   DCHECK_NOT_NULL(raw_inferred_name);
   raw_inferred_name_ = raw_inferred_name;
   DCHECK(inferred_name_.is_null());

src/ast/ast.h

@@ -2287,13 +2287,13 @@ class FunctionLiteral final : public Expression {
   // Returns either name or inferred name as a cstring.
   std::unique_ptr<char[]> GetDebugName() const;
 
-  Handle<String> GetInferredName(Isolate* isolate) const {
+  Handle<String> GetInferredName(Isolate* isolate) {
     if (!inferred_name_.is_null()) {
       DCHECK_NULL(raw_inferred_name_);
       return inferred_name_;
     }
     if (raw_inferred_name_ != nullptr) {
-      return raw_inferred_name_->Allocate(isolate);
+      return raw_inferred_name_->GetString(isolate);
     }
     UNREACHABLE();
   }
@@ -2301,7 +2301,7 @@ class FunctionLiteral final : public Expression {
 
   // Only one of {set_inferred_name, set_raw_inferred_name} should be called.
   void set_inferred_name(Handle<String> inferred_name);
-  void set_raw_inferred_name(const AstConsString* raw_inferred_name);
+  void set_raw_inferred_name(AstConsString* raw_inferred_name);
 
   bool pretenure() const { return Pretenure::decode(bit_field_); }
   void set_pretenure() { bit_field_ = Pretenure::update(bit_field_, true); }
@@ -2427,7 +2427,7 @@ class FunctionLiteral final : public Expression {
   const AstConsString* raw_name_;
   DeclarationScope* scope_;
   ZonePtrList<Statement> body_;
-  const AstConsString* raw_inferred_name_;
+  AstConsString* raw_inferred_name_;
   Handle<String> inferred_name_;
   ProducedPreparseData* produced_preparse_data_;
 };

src/parsing/func-name-inferrer.cc

@@ -45,7 +45,7 @@ void FuncNameInferrer::RemoveAsyncKeywordFromEnd() {
   }
 }
 
-const AstConsString* FuncNameInferrer::MakeNameFromStack() {
+AstConsString* FuncNameInferrer::MakeNameFromStack() {
   if (names_stack_.size() == 0) {
     return ast_value_factory_->empty_cons_string();
   }
@@ -70,7 +70,7 @@ const AstConsString* FuncNameInferrer::MakeNameFromStack() {
 }
 
 void FuncNameInferrer::InferFunctionsNames() {
-  const AstConsString* func_name = MakeNameFromStack();
+  AstConsString* func_name = MakeNameFromStack();
   for (FunctionLiteral* func : funcs_to_infer_) {
     func->set_raw_inferred_name(func_name);
   }

src/parsing/func-name-inferrer.h

@@ -110,7 +110,7 @@ class FuncNameInferrer {
   };
 
   // Constructs a full name in dotted notation from gathered names.
-  const AstConsString* MakeNameFromStack();
+  AstConsString* MakeNameFromStack();
 
   // Performs name inferring for added functions.
   void InferFunctionsNames();

test/unittests/heap/off-thread-factory-unittest.cc

@@ -153,13 +153,13 @@ TEST_F(OffThreadFactoryTest, AstConsString_CreatesConsString) {
   const AstRawString* foo_string = ast_value_factory.GetOneByteString("foo");
   const AstRawString* bar_string =
       ast_value_factory.GetOneByteString("bar-plus-padding-for-length");
-  const AstConsString* foobar_string =
+  AstConsString* foobar_string =
       ast_value_factory.NewConsString(foo_string, bar_string);
 
   ast_value_factory.Internalize(off_thread_isolate());
 
   OffThreadHandle<FixedArray> off_thread_wrapper =
-      WrapString(foobar_string->Allocate(off_thread_isolate()));
+      WrapString(foobar_string->GetString(off_thread_isolate()));
 
   off_thread_factory()->FinishOffThread();