[runtime] Use a hashtable in ScriptContextTable

Bug: v8:12315
Change-Id: If750c9528d4f20c6695a6b25f4d1abc8a14dba62
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3431486
Commit-Queue: Victor Gomes <victorgomes@chromium.org>
Auto-Submit: Victor Gomes <victorgomes@chromium.org>
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Commit-Queue: Igor Sheludko <ishell@chromium.org>
Cr-Commit-Position: refs/heads/main@{#78983}

src/execution/execution.cc

@@ -263,9 +263,12 @@ MaybeHandle<Context> NewScriptContext(Isolate* isolate,
       isolate->factory()->NewScriptContext(native_context, scope_info);
 
   result->Initialize(isolate);
-
+  // In REPL mode, we are allowed to add/modify let/const variables.
+  // We use the previous defined script context for those.
+  const bool ignore_duplicates = scope_info->IsReplModeScope();
   Handle<ScriptContextTable> new_script_context_table =
-      ScriptContextTable::Extend(script_context, result);
+      ScriptContextTable::Extend(isolate, script_context, result,
+                                 ignore_duplicates);
   native_context->synchronized_set_script_context_table(
       *new_script_context_table);
   return result;

src/heap/factory.cc

@@ -1207,7 +1207,9 @@ Handle<ScriptContextTable> Factory::NewScriptContextTable() {
   Handle<ScriptContextTable> context_table = Handle<ScriptContextTable>::cast(
       NewFixedArrayWithMap(read_only_roots().script_context_table_map_handle(),
                            ScriptContextTable::kMinLength));
+  Handle<NameToIndexHashTable> names = NameToIndexHashTable::New(isolate(), 16);
   context_table->set_used(0, kReleaseStore);
+  context_table->set_names_to_context_index(*names);
   return context_table;
 }
 

src/init/bootstrapper.cc

@@ -1243,7 +1243,7 @@ void Genesis::InstallGlobalThisBinding() {
   context->set(slot, native_context()->global_proxy());
 
   Handle<ScriptContextTable> new_script_contexts =
-      ScriptContextTable::Extend(script_contexts, context);
+      ScriptContextTable::Extend(isolate(), script_contexts, context);
   native_context()->set_script_context_table(*new_script_contexts);
 }
 

src/objects/contexts-inl.h

@@ -39,6 +39,9 @@ void ScriptContextTable::set_used(int used, ReleaseStoreTag tag) {
   set(kUsedSlotIndex, Smi::FromInt(used), tag);
 }
 
+ACCESSORS(ScriptContextTable, names_to_context_index, NameToIndexHashTable,
+          kHashTableOffset)
+
 // static
 Handle<Context> ScriptContextTable::GetContext(Isolate* isolate,
                                                Handle<ScriptContextTable> table,
@@ -109,9 +112,7 @@ void NativeContext::set(int index, Object value, WriteBarrierMode mode,
   Context::set(index, value, mode, tag);
 }
 
-void Context::set_scope_info(ScopeInfo scope_info, WriteBarrierMode mode) {
+ACCESSORS(Context, scope_info, ScopeInfo, kScopeInfoOffset)
-  set(SCOPE_INFO_INDEX, scope_info, mode);
-}
 
 Object Context::unchecked_previous() const { return get(PREVIOUS_INDEX); }
 
@@ -124,10 +125,6 @@ void Context::set_previous(Context context, WriteBarrierMode mode) {
   set(PREVIOUS_INDEX, context, mode);
 }
 
-ScopeInfo Context::scope_info() const {
-  return ScopeInfo::cast(get(SCOPE_INFO_INDEX));
-}
-
 Object Context::next_context_link() const {
   return get(Context::NEXT_CONTEXT_LINK);
 }

src/objects/contexts.cc

@@ -14,15 +14,40 @@
 namespace v8 {
 namespace internal {
 
+void ScriptContextTable::AddLocalNamesFromContext(
+    Isolate* isolate, Handle<ScriptContextTable> script_context_table,
+    Handle<Context> script_context, bool ignore_duplicates,
+    int script_context_index) {
+  ReadOnlyRoots roots(isolate);
+  PtrComprCageBase cage_base(isolate);
+  Handle<NameToIndexHashTable> names_table(
+      script_context_table->names_to_context_index(cage_base), isolate);
+  Handle<ScopeInfo> scope_info(script_context->scope_info(cage_base), isolate);
+  int local_count = scope_info->ContextLocalCount();
+  names_table = names_table->EnsureCapacity(isolate, names_table, local_count);
+  for (auto it : ScopeInfo::IterateLocalNames(scope_info)) {
+    Handle<Name> name(it->name(cage_base), isolate);
+    if (ignore_duplicates) {
+      int32_t hash = NameToIndexShape::Hash(roots, name);
+      if (names_table->FindEntry(cage_base, roots, name, hash).is_found()) {
+        continue;
+      }
+    }
+    names_table = NameToIndexHashTable::Add(isolate, names_table, name,
+                                            script_context_index);
+  }
+  script_context_table->set_names_to_context_index(*names_table);
+}
+
 Handle<ScriptContextTable> ScriptContextTable::Extend(
-    Handle<ScriptContextTable> table, Handle<Context> script_context) {
+    Isolate* isolate, Handle<ScriptContextTable> table,
+    Handle<Context> script_context, bool ignore_duplicates) {
   Handle<ScriptContextTable> result;
   int used = table->used(kAcquireLoad);
   int length = table->length();
   CHECK(used >= 0 && length > 0 && used < length);
   if (used + kFirstContextSlotIndex == length) {
     CHECK(length < Smi::kMaxValue / 2);
-    Isolate* isolate = script_context->GetIsolate();
     Handle<FixedArray> copy =
         isolate->factory()->CopyFixedArrayAndGrow(table, length);
     copy->set_map(ReadOnlyRoots(isolate).script_context_table_map());
@@ -31,6 +56,8 @@ Handle<ScriptContextTable> ScriptContextTable::Extend(
     result = table;
   }
   DCHECK(script_context->IsScriptContext());
+  ScriptContextTable::AddLocalNamesFromContext(isolate, result, script_context,
+                                               ignore_duplicates, used);
   result->set(used + kFirstContextSlotIndex, *script_context, kReleaseStore);
   result->set_used(used + 1, kReleaseStore);
   return result;
@@ -49,16 +76,17 @@ void Context::Initialize(Isolate* isolate) {
 bool ScriptContextTable::Lookup(Handle<String> name,
                                 VariableLookupResult* result) {
   DisallowGarbageCollection no_gc;
-  // Static variables cannot be in script contexts.
+  int index = names_to_context_index().Lookup(name);
-  for (int i = 0; i < used(kAcquireLoad); i++) {
+  if (index == -1) return false;
-    Context context = get_context(i);
+  DCHECK_LE(0, index);
-    DCHECK(context.IsScriptContext());
+  DCHECK_LT(index, used(kAcquireLoad));
-    int slot_index = context.scope_info().ContextSlotIndex(name, result);
+  Context context = get_context(index);
-    if (slot_index >= 0) {
+  DCHECK(context.IsScriptContext());
-      result->context_index = i;
+  int slot_index = context.scope_info().ContextSlotIndex(name, result);
-      result->slot_index = slot_index;
+  if (slot_index >= 0) {
-      return true;
+    result->context_index = index;
-    }
+    result->slot_index = slot_index;
+    return true;
   }
   return false;
 }

src/objects/contexts.h

@@ -386,6 +386,14 @@ class ScriptContextTable : public FixedArray {
   inline Context get_context(int i) const;
   inline Context get_context(int i, AcquireLoadTag tag) const;
 
+  DECL_ACCESSORS(names_to_context_index, NameToIndexHashTable)
+
+  // Adds local names from `script_context` to the hash table.
+  static void AddLocalNamesFromContext(
+      Isolate* isolate, Handle<ScriptContextTable> script_context_table,
+      Handle<Context> script_context, bool ignore_duplicates,
+      int script_context_index);
+
   // Lookup a variable `name` in a ScriptContextTable.
   // If it returns true, the variable is found and `result` contains
   // valid information about its location.
@@ -396,12 +404,16 @@ class ScriptContextTable : public FixedArray {
 
   V8_WARN_UNUSED_RESULT
   V8_EXPORT_PRIVATE static Handle<ScriptContextTable> Extend(
-      Handle<ScriptContextTable> table, Handle<Context> script_context);
+      Isolate* isolate, Handle<ScriptContextTable> table,
+      Handle<Context> script_context, bool ignore_duplicates = false);
 
-  static const int kUsedSlotIndex = 0;
+  static const int kHashTableIndex = 0;
-  static const int kFirstContextSlotIndex = 1;
+  static const int kUsedSlotIndex = 1;
+  static const int kFirstContextSlotIndex = 2;
   static const int kMinLength = kFirstContextSlotIndex;
 
+  static const int kHashTableOffset = OffsetOfElementAt(kHashTableIndex);
+
   OBJECT_CONSTRUCTORS(ScriptContextTable, FixedArray);
 };
 
@@ -562,8 +574,7 @@ class Context : public TorqueGeneratedContext<Context, HeapObject> {
   static const int kInvalidContext = 1;
 
   // Direct slot access.
-  inline void set_scope_info(ScopeInfo scope_info,
+  DECL_ACCESSORS(scope_info, ScopeInfo)
-                             WriteBarrierMode mode = UPDATE_WRITE_BARRIER);
 
   inline Object unchecked_previous() const;
   inline Context previous() const;
@@ -576,7 +587,6 @@ class Context : public TorqueGeneratedContext<Context, HeapObject> {
       HeapObject object, WriteBarrierMode mode = UPDATE_WRITE_BARRIER);
   JSObject extension_object() const;
   JSReceiver extension_receiver() const;
-  V8_EXPORT_PRIVATE inline ScopeInfo scope_info() const;
 
   // Find the module context (assuming there is one) and return the associated
   // module object.

src/objects/scope-info-inl.h

@@ -64,12 +64,18 @@ class ScopeInfo::LocalNamesRange {
       return !(a == b);
     }
 
-    String name() const {
+    String name(PtrComprCageBase cage_base) const {
       DCHECK_LT(index_, range_->max_index());
       if (range_->inlined()) {
-        return scope_info()->ContextInlinedLocalName(index_.as_int());
+        return scope_info()->ContextInlinedLocalName(cage_base,
+                                                     index_.as_int());
       }
-      return String::cast(table().KeyAt(index_));
+      return String::cast(table().KeyAt(cage_base, index_));
+    }
+
+    String name() const {
+      PtrComprCageBase cage_base = GetPtrComprCageBase(*scope_info());
+      return name(cage_base);
     }
 
     const Iterator* operator*() const { return this; }

test/cctest/test-concurrent-script-context-table.cc

@@ -98,6 +98,7 @@ TEST(ScriptContextTable_Extend) {
   CcTest::InitializeVM();
   v8::HandleScope scope(CcTest::isolate());
   Isolate* isolate = CcTest::i_isolate();
+  const bool kIgnoreDuplicateNames = true;
 
   Factory* factory = isolate->factory();
   Handle<NativeContext> native_context = factory->NewNativeContext();
@@ -116,8 +117,8 @@ TEST(ScriptContextTable_Extend) {
     Handle<Context> script_context =
         factory->NewScriptContext(native_context, scope_info);
 
-    script_context_table =
+    script_context_table = ScriptContextTable::Extend(
-        ScriptContextTable::Extend(script_context_table, script_context);
+        isolate, script_context_table, script_context, kIgnoreDuplicateNames);
   }
 
   std::unique_ptr<PersistentHandles> ph = isolate->NewPersistentHandles();
@@ -137,8 +138,8 @@ TEST(ScriptContextTable_Extend) {
   for (int i = 0; i < 100; ++i) {
     Handle<Context> context =
         factory->NewScriptContext(native_context, scope_info);
-    script_context_table =
+    script_context_table = ScriptContextTable::Extend(
-        ScriptContextTable::Extend(script_context_table, context);
+        isolate, script_context_table, context, kIgnoreDuplicateNames);
   }
 
   thread->Join();
@@ -164,7 +165,7 @@ TEST(ScriptContextTable_AccessScriptContextTable) {
   Handle<Context> context =
       factory->NewScriptContext(native_context, scope_info);
   script_context_table =
-      ScriptContextTable::Extend(script_context_table, context);
+      ScriptContextTable::Extend(isolate, script_context_table, context);
   int initialized_entries = 1;
   g_initialized_entries.store(initialized_entries, std::memory_order_release);
 
@@ -183,11 +184,12 @@ TEST(ScriptContextTable_AccessScriptContextTable) {
 
   sema_started.Wait();
 
+  const bool kIgnoreDuplicateNames = true;
   for (; initialized_entries < 1000; ++initialized_entries) {
     Handle<Context> new_context =
         factory->NewScriptContext(native_context, scope_info);
-    script_context_table =
+    script_context_table = ScriptContextTable::Extend(
-        ScriptContextTable::Extend(script_context_table, new_context);
+        isolate, script_context_table, new_context, kIgnoreDuplicateNames);
     native_context->synchronized_set_script_context_table(
         *script_context_table);
     // Update with relaxed semantics to not introduce ordering constraints.