[templates] Clean up TemplateObjectDescription::GetTemplateObject

- Avoid lookup if there is no template_weakmap yet
- More explicit DisallowGarbageCollection scopes
- Avoid handles when settings properties
- Speed up Object::GetSimpleHash by loading only instance_type once

Change-Id: Ib588607340a0c56dc1ba26c3e89485560222a688
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3463717Reviewed-by: Marja Hölttä <marja@chromium.org>
Commit-Queue: Camillo Bruni <cbruni@chromium.org>
Cr-Commit-Position: refs/heads/main@{#79189}

src/ast/ast.cc

@@ -770,36 +770,44 @@ template EXPORT_TEMPLATE_DEFINE(
 template <typename IsolateT>
 Handle<TemplateObjectDescription> GetTemplateObject::GetOrBuildDescription(
     IsolateT* isolate) {
-  Handle<FixedArray> raw_strings = isolate->factory()->NewFixedArray(
+  Handle<FixedArray> raw_strings_handle = isolate->factory()->NewFixedArray(
       this->raw_strings()->length(), AllocationType::kOld);
   bool raw_and_cooked_match = true;
-  for (int i = 0; i < raw_strings->length(); ++i) {
-    if (this->raw_strings()->at(i) != this->cooked_strings()->at(i)) {
-      // If the AstRawStrings don't match, then neither should the allocated
-      // Strings, since the AstValueFactory should have deduplicated them
-      // already.
-      DCHECK_IMPLIES(this->cooked_strings()->at(i) != nullptr,
-                     *this->cooked_strings()->at(i)->string() !=
-                         *this->raw_strings()->at(i)->string());
-
-      raw_and_cooked_match = false;
+  {
+    DisallowGarbageCollection no_gc;
+    FixedArray raw_strings = *raw_strings_handle;
+
+    for (int i = 0; i < raw_strings.length(); ++i) {
+      if (this->raw_strings()->at(i) != this->cooked_strings()->at(i)) {
+        // If the AstRawStrings don't match, then neither should the allocated
+        // Strings, since the AstValueFactory should have deduplicated them
+        // already.
+        DCHECK_IMPLIES(this->cooked_strings()->at(i) != nullptr,
+                       *this->cooked_strings()->at(i)->string() !=
+                           *this->raw_strings()->at(i)->string());
+
+        raw_and_cooked_match = false;
+      }
+      raw_strings.set(i, *this->raw_strings()->at(i)->string());
     }
-    raw_strings->set(i, *this->raw_strings()->at(i)->string());
   }
-  Handle<FixedArray> cooked_strings = raw_strings;
+  Handle<FixedArray> cooked_strings_handle = raw_strings_handle;
   if (!raw_and_cooked_match) {
-    cooked_strings = isolate->factory()->NewFixedArray(
+    cooked_strings_handle = isolate->factory()->NewFixedArray(
         this->cooked_strings()->length(), AllocationType::kOld);
-    for (int i = 0; i < cooked_strings->length(); ++i) {
+    DisallowGarbageCollection no_gc;
+    FixedArray cooked_strings = *cooked_strings_handle;
+    ReadOnlyRoots roots(isolate);
+    for (int i = 0; i < cooked_strings.length(); ++i) {
       if (this->cooked_strings()->at(i) != nullptr) {
-        cooked_strings->set(i, *this->cooked_strings()->at(i)->string());
+        cooked_strings.set(i, *this->cooked_strings()->at(i)->string());
       } else {
-        cooked_strings->set(i, ReadOnlyRoots(isolate).undefined_value());
+        cooked_strings.set_undefined(roots, i);
       }
     }
   }
-  return isolate->factory()->NewTemplateObjectDescription(raw_strings,
-                                                          cooked_strings);
+  return isolate->factory()->NewTemplateObjectDescription(
+      raw_strings_handle, cooked_strings_handle);
 }
 template EXPORT_TEMPLATE_DEFINE(V8_BASE_EXPORT)
     Handle<TemplateObjectDescription> GetTemplateObject::GetOrBuildDescription(

src/objects/fixed-array.h

@@ -141,10 +141,13 @@ class FixedArray
   // Setters for frequently used oddballs located in old space.
   inline void set_undefined(int index);
   inline void set_undefined(Isolate* isolate, int index);
+  inline void set_undefined(ReadOnlyRoots ro_roots, int index);
   inline void set_null(int index);
   inline void set_null(Isolate* isolate, int index);
+  inline void set_null(ReadOnlyRoots ro_roots, int index);
   inline void set_the_hole(int index);
   inline void set_the_hole(Isolate* isolate, int index);
+  inline void set_the_hole(ReadOnlyRoots ro_roots, int index);
 
   inline ObjectSlot GetFirstElementAddress();
   inline bool ContainsOnlySmisOrHoles();
@@ -215,10 +218,6 @@ class FixedArray
  private:
   STATIC_ASSERT(kHeaderSize == Internals::kFixedArrayHeaderSize);
 
-  inline void set_undefined(ReadOnlyRoots ro_roots, int index);
-  inline void set_null(ReadOnlyRoots ro_roots, int index);
-  inline void set_the_hole(ReadOnlyRoots ro_roots, int index);
-
   TQ_OBJECT_CONSTRUCTORS(FixedArray)
 };
 

src/objects/objects-inl.h

@@ -1120,7 +1120,8 @@ Object Object::GetSimpleHash(Object object) {
     uint32_t hash = ComputeUnseededHash(Smi::ToInt(object));
     return Smi::FromInt(hash & Smi::kMaxValue);
   }
-  if (object.IsHeapNumber()) {
+  auto instance_type = HeapObject::cast(object).map().instance_type();
+  if (InstanceTypeChecker::IsHeapNumber(instance_type)) {
     double num = HeapNumber::cast(object).value();
     if (std::isnan(num)) return Smi::FromInt(Smi::kMaxValue);
     // Use ComputeUnseededHash for all values in Signed32 range, including -0,
@@ -1133,20 +1134,16 @@ Object Object::GetSimpleHash(Object object) {
       hash = ComputeLongHash(base::double_to_uint64(num));
     }
     return Smi::FromInt(hash & Smi::kMaxValue);
-  }
-  if (object.IsName()) {
+  } else if (InstanceTypeChecker::IsName(instance_type)) {
     uint32_t hash = Name::cast(object).EnsureHash();
     return Smi::FromInt(hash);
-  }
-  if (object.IsOddball()) {
+  } else if (InstanceTypeChecker::IsOddball(instance_type)) {
     uint32_t hash = Oddball::cast(object).to_string().EnsureHash();
     return Smi::FromInt(hash);
-  }
-  if (object.IsBigInt()) {
+  } else if (InstanceTypeChecker::IsBigInt(instance_type)) {
     uint32_t hash = BigInt::cast(object).Hash();
     return Smi::FromInt(hash & Smi::kMaxValue);
-  }
-  if (object.IsSharedFunctionInfo()) {
+  } else if (InstanceTypeChecker::IsSharedFunctionInfo(instance_type)) {
     uint32_t hash = SharedFunctionInfo::cast(object).Hash();
     return Smi::FromInt(hash & Smi::kMaxValue);
   }

src/objects/template-objects.cc

@@ -19,22 +19,27 @@ Handle<JSArray> TemplateObjectDescription::GetTemplateObject(
     Isolate* isolate, Handle<NativeContext> native_context,
     Handle<TemplateObjectDescription> description,
     Handle<SharedFunctionInfo> shared_info, int slot_id) {
-  // Check the template weakmap to see if the template object already exists.
-  Handle<EphemeronHashTable> template_weakmap =
-      native_context->template_weakmap().IsUndefined(isolate)
-          ? EphemeronHashTable::New(isolate, 0)
-          : handle(EphemeronHashTable::cast(native_context->template_weakmap()),
-                   isolate);
-
   uint32_t hash = shared_info->Hash();
-  Object maybe_cached_template = template_weakmap->Lookup(shared_info, hash);
-  while (!maybe_cached_template.IsTheHole()) {
-    CachedTemplateObject cached_template =
-        CachedTemplateObject::cast(maybe_cached_template);
-    if (cached_template.slot_id() == slot_id)
-      return handle(cached_template.template_object(), isolate);
 
-    maybe_cached_template = cached_template.next();
+  // Check the template weakmap to see if the template object already exists.
+  Handle<EphemeronHashTable> template_weakmap;
+
+  if (native_context->template_weakmap().IsUndefined(isolate)) {
+    template_weakmap = EphemeronHashTable::New(isolate, 1);
+  } else {
+    DisallowGarbageCollection no_gc;
+    ReadOnlyRoots roots(isolate);
+    template_weakmap = handle(
+        EphemeronHashTable::cast(native_context->template_weakmap()), isolate);
+    Object maybe_cached_template = template_weakmap->Lookup(shared_info, hash);
+    while (!maybe_cached_template.IsTheHole(roots)) {
+      CachedTemplateObject cached_template =
+          CachedTemplateObject::cast(maybe_cached_template);
+      if (cached_template.slot_id() == slot_id) {
+        return handle(cached_template.template_object(), isolate);
+      }
+      maybe_cached_template = cached_template.next();
+    }
   }
 
   // Create the raw object from the {raw_strings}.
@@ -83,13 +88,17 @@ Handle<CachedTemplateObject> CachedTemplateObject::New(
     Isolate* isolate, int slot_id, Handle<JSArray> template_object,
     Handle<HeapObject> next) {
   DCHECK(next->IsCachedTemplateObject() || next->IsTheHole());
-  Factory* factory = isolate->factory();
-  Handle<CachedTemplateObject> result = Handle<CachedTemplateObject>::cast(
-      factory->NewStruct(CACHED_TEMPLATE_OBJECT_TYPE, AllocationType::kOld));
-  result->set_slot_id(slot_id);
-  result->set_template_object(*template_object);
-  result->set_next(*next);
-  return result;
+  Handle<CachedTemplateObject> result_handle =
+      Handle<CachedTemplateObject>::cast(isolate->factory()->NewStruct(
+          CACHED_TEMPLATE_OBJECT_TYPE, AllocationType::kOld));
+  {
+    DisallowGarbageCollection no_gc;
+    auto result = *result_handle;
+    result.set_slot_id(slot_id);
+    result.set_template_object(*template_object);
+    result.set_next(*next);
+  }
+  return result_handle;
 }
 
 }  // namespace internal