[turbofan] Concurrent ScriptContextTable access

This CL makes the ScriptContextTable concurrently accessible from the
background thread (in particular ScriptContextTable::get_context).
A cctest is added to check synchronization with tsan.

Bug: v8:7790
Change-Id: I2e2dc8c6a7cfa369787959c4d5ed5f357f4720fa
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2260567Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Reviewed-by: Georg Neis <neis@chromium.org>
Commit-Queue: Nico Hartmann <nicohartmann@chromium.org>
Cr-Commit-Position: refs/heads/master@{#68979}

src/api/api.cc

@@ -10128,7 +10128,7 @@ void debug::GlobalLexicalScopeNames(
   i::Handle<i::ScriptContextTable> table(
       context->global_object().native_context().script_context_table(),
       isolate);
-  for (int i = 0; i < table->used(); i++) {
+  for (int i = 0; i < table->synchronized_used(); i++) {
     i::Handle<i::Context> context =
         i::ScriptContextTable::GetContext(isolate, table, i);
     DCHECK(context->IsScriptContext());

src/debug/debug-scopes.cc

@@ -721,8 +721,8 @@ void ScopeIterator::VisitScriptScope(const Visitor& visitor) const {
       global->native_context().script_context_table(), isolate_);
 
   // Skip the first script since that just declares 'this'.
-  for (int context_index = 1; context_index < script_contexts->used();
-       context_index++) {
+  for (int context_index = 1;
+       context_index < script_contexts->synchronized_used(); context_index++) {
     Handle<Context> context = ScriptContextTable::GetContext(
         isolate_, script_contexts, context_index);
     Handle<ScopeInfo> scope_info(context->scope_info(), isolate_);

src/heap/factory.cc

@@ -1124,7 +1124,7 @@ Handle<ScriptContextTable> Factory::NewScriptContextTable() {
   Handle<ScriptContextTable> context_table = Handle<ScriptContextTable>::cast(
       NewFixedArrayWithMap(read_only_roots().script_context_table_map_handle(),
                            ScriptContextTable::kMinLength));
-  context_table->set_used(0);
+  context_table->synchronized_set_used(0);
   return context_table;
 }
 

src/objects/contexts-inl.h

@@ -27,10 +27,12 @@ namespace internal {
 OBJECT_CONSTRUCTORS_IMPL(ScriptContextTable, FixedArray)
 CAST_ACCESSOR(ScriptContextTable)
 
-int ScriptContextTable::used() const { return Smi::ToInt(get(kUsedSlotIndex)); }
+int ScriptContextTable::synchronized_used() const {
+  return Smi::ToInt(synchronized_get(kUsedSlotIndex));
+}
 
-void ScriptContextTable::set_used(int used) {
-  set(kUsedSlotIndex, Smi::FromInt(used));
+void ScriptContextTable::synchronized_set_used(int used) {
+  synchronized_set(kUsedSlotIndex, Smi::FromInt(used));
 }
 
 // static
@@ -41,7 +43,7 @@ Handle<Context> ScriptContextTable::GetContext(Isolate* isolate,
 }
 
 Context ScriptContextTable::get_context(int i) const {
-  DCHECK_LT(i, used());
+  DCHECK_LT(i, synchronized_used());
   return Context::cast(this->get(i + kFirstContextSlotIndex));
 }
 

src/objects/contexts.cc

@@ -16,7 +16,7 @@ namespace internal {
 Handle<ScriptContextTable> ScriptContextTable::Extend(
     Handle<ScriptContextTable> table, Handle<Context> script_context) {
   Handle<ScriptContextTable> result;
-  int used = table->used();
+  int used = table->synchronized_used();
   int length = table->length();
   CHECK(used >= 0 && length > 0 && used < length);
   if (used + kFirstContextSlotIndex == length) {
@@ -29,10 +29,10 @@ Handle<ScriptContextTable> ScriptContextTable::Extend(
   } else {
     result = table;
   }
-  result->set_used(used + 1);
-
   DCHECK(script_context->IsScriptContext());
   result->set(used + kFirstContextSlotIndex, *script_context);
+
+  result->synchronized_set_used(used + 1);
   return result;
 }
 
@@ -51,7 +51,7 @@ bool ScriptContextTable::Lookup(Isolate* isolate, ScriptContextTable table,
   DisallowHeapAllocation no_gc;
   // Static variables cannot be in script contexts.
   IsStaticFlag is_static_flag;
-  for (int i = 0; i < table.used(); i++) {
+  for (int i = 0; i < table.synchronized_used(); i++) {
     Context context = table.get_context(i);
     DCHECK(context.IsScriptContext());
     int slot_index = ScopeInfo::ContextSlotIndex(

src/objects/contexts.h

@@ -352,8 +352,8 @@ class ScriptContextTable : public FixedArray {
     MaybeAssignedFlag maybe_assigned_flag;
   };
 
-  inline int used() const;
-  inline void set_used(int used);
+  inline int synchronized_used() const;
+  inline void synchronized_set_used(int used);
 
   static inline Handle<Context> GetContext(Isolate* isolate,
                                            Handle<ScriptContextTable> table,
@@ -370,8 +370,8 @@ class ScriptContextTable : public FixedArray {
                                        LookupResult* result);
 
   V8_WARN_UNUSED_RESULT
-  static Handle<ScriptContextTable> Extend(Handle<ScriptContextTable> table,
-                                           Handle<Context> script_context);
+  V8_EXPORT_PRIVATE static Handle<ScriptContextTable> Extend(
+      Handle<ScriptContextTable> table, Handle<Context> script_context);
 
   static const int kUsedSlotIndex = 0;
   static const int kFirstContextSlotIndex = 1;

src/objects/fixed-array-inl.h

@@ -123,6 +123,23 @@ void FixedArray::NoWriteBarrierSet(FixedArray array, int index, Object value) {
   RELAXED_WRITE_FIELD(array, offset, value);
 }
 
+Object FixedArray::synchronized_get(int index) const {
+  const Isolate* isolate = GetIsolateForPtrCompr(*this);
+  return synchronized_get(isolate, index);
+}
+
+Object FixedArray::synchronized_get(const Isolate* isolate, int index) const {
+  DCHECK_LT(static_cast<unsigned>(index), static_cast<unsigned>(length()));
+  return ACQUIRE_READ_FIELD(*this, OffsetOfElementAt(index));
+}
+
+void FixedArray::synchronized_set(int index, Smi value) {
+  DCHECK_NE(map(), GetReadOnlyRoots().fixed_cow_array_map());
+  DCHECK_LT(static_cast<unsigned>(index), static_cast<unsigned>(length()));
+  DCHECK(Object(value).IsSmi());
+  RELEASE_WRITE_FIELD(*this, OffsetOfElementAt(index), value);
+}
+
 void FixedArray::set_undefined(int index) {
   set_undefined(GetReadOnlyRoots(), index);
 }

src/objects/fixed-array.h

@@ -111,6 +111,13 @@ class FixedArray
       Isolate* isolate, Handle<FixedArray> array, int index,
       Handle<Object> value);
 
+  // Synchronized setters and getters.
+  inline Object synchronized_get(int index) const;
+  inline Object synchronized_get(const Isolate* isolate, int index) const;
+  // Currently only Smis are written with release semantics, hence we can avoid
+  // a write barrier.
+  inline void synchronized_set(int index, Smi value);
+
   // Setter that uses write barrier.
   inline void set(int index, Object value);
   inline bool is_the_hole(Isolate* isolate, int index);

test/cctest/BUILD.gn

@@ -199,6 +199,7 @@ v8_source_set("cctest_sources") {
     "test-compiler.cc",
     "test-concurrent-descriptor-array.cc",
     "test-concurrent-prototype.cc",
+    "test-concurrent-script-context-table.cc",
     "test-concurrent-transition-array.cc",
     "test-constantpool.cc",
     "test-conversions.cc",

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

+// Copyright 2020 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "src/api/api.h"
+#include "src/base/platform/semaphore.h"
+#include "src/handles/handles-inl.h"
+#include "src/handles/local-handles-inl.h"
+#include "src/handles/persistent-handles.h"
+#include "src/heap/heap.h"
+#include "src/heap/local-heap.h"
+#include "src/objects/contexts.h"
+#include "test/cctest/cctest.h"
+#include "test/cctest/heap/heap-utils.h"
+
+namespace v8 {
+namespace internal {
+
+namespace {
+
+class ScriptContextTableAccessUsedThread final : public v8::base::Thread {
+ public:
+  ScriptContextTableAccessUsedThread(
+      Isolate* isolate, Heap* heap, base::Semaphore* sema_started,
+      std::unique_ptr<PersistentHandles> ph,
+      Handle<ScriptContextTable> script_context_table)
+      : v8::base::Thread(
+            base::Thread::Options("ScriptContextTableAccessUsedThread")),
+        isolate_(isolate),
+        heap_(heap),
+        sema_started_(sema_started),
+        ph_(std::move(ph)),
+        script_context_table_(script_context_table) {}
+
+  void Run() override {
+    LocalHeap local_heap(heap_, std::move(ph_));
+    LocalHandleScope scope(&local_heap);
+
+    sema_started_->Signal();
+
+    for (int i = 0; i < script_context_table_->synchronized_used(); ++i) {
+      Context context = script_context_table_->get_context(i);
+      CHECK(context.IsScriptContext());
+    }
+
+    CHECK(!ph_);
+    ph_ = local_heap.DetachPersistentHandles();
+  }
+
+  Isolate* isolate_;
+  Heap* heap_;
+  base::Semaphore* sema_started_;
+  std::unique_ptr<PersistentHandles> ph_;
+  Handle<ScriptContextTable> script_context_table_;
+};
+
+TEST(ScriptContextTable_Extend) {
+  CcTest::InitializeVM();
+  v8::HandleScope scope(CcTest::isolate());
+  Isolate* isolate = CcTest::i_isolate();
+
+  Factory* factory = isolate->factory();
+  Handle<NativeContext> native_context = factory->NewNativeContext();
+  Handle<Map> script_context_map =
+      factory->NewMap(SCRIPT_CONTEXT_TYPE, kVariableSizeSentinel);
+  script_context_map->set_native_context(*native_context);
+  native_context->set_script_context_map(*script_context_map);
+
+  Handle<ScriptContextTable> script_context_table =
+      factory->NewScriptContextTable();
+
+  Handle<ScopeInfo> scope_info =
+      ReadOnlyRoots(isolate).global_this_binding_scope_info_handle();
+
+  for (int i = 0; i < 10; ++i) {
+    Handle<Context> script_context =
+        factory->NewScriptContext(native_context, scope_info);
+
+    script_context_table =
+        ScriptContextTable::Extend(script_context_table, script_context);
+  }
+
+  std::unique_ptr<PersistentHandles> ph = isolate->NewPersistentHandles();
+  Handle<ScriptContextTable> persistent_script_context_table =
+      ph->NewHandle(script_context_table);
+
+  base::Semaphore sema_started(0);
+
+  auto thread = std::make_unique<ScriptContextTableAccessUsedThread>(
+      isolate, isolate->heap(), &sema_started, std::move(ph),
+      persistent_script_context_table);
+
+  CHECK(thread->Start());
+
+  sema_started.Wait();
+
+  for (int i = 0; i < 10; ++i) {
+    Handle<Context> context =
+        factory->NewScriptContext(native_context, scope_info);
+    script_context_table =
+        ScriptContextTable::Extend(script_context_table, context);
+  }
+
+  thread->Join();
+}
+
+}  // anonymous namespace
+
+}  // namespace internal
+}  // namespace v8