[compiler] Test linear searches in a DescriptorArray in the background

This CL adds a linear search test in a DescriptorArray in a known flat
object in the background thread, while the main thread exercises the
same DescriptorArray.

Also sets the foundation for the follow-ups tests in background threads.

Bug: v8:7790
Change-Id: I0e99508204808baaf605161d2eeb717eabe712fb
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2207147
Commit-Queue: Santiago Aboy Solanes <solanes@chromium.org>
Reviewed-by: Georg Neis <neis@chromium.org>
Reviewed-by: Dominik Inführ <dinfuehr@chromium.org>
Reviewed-by: Michael Stanton <mvstanton@chromium.org>
Cr-Commit-Position: refs/heads/master@{#68299}

src/common/checks.h

@@ -18,9 +18,11 @@ namespace internal {
 #ifdef ENABLE_SLOW_DCHECKS
 #define SLOW_DCHECK(condition) \
   CHECK(!v8::internal::FLAG_enable_slow_asserts || (condition))
+#define SLOW_DCHECK_IMPLIES(lhs, rhs) SLOW_DCHECK(!(lhs) || (rhs))
 V8_EXPORT_PRIVATE extern bool FLAG_enable_slow_asserts;
 #else
 #define SLOW_DCHECK(condition) ((void)0)
+#define SLOW_DCHECK_IMPLIES(v1, v2) ((void)0)
 static const bool FLAG_enable_slow_asserts = false;
 #endif
 

src/objects/descriptor-array-inl.h

@@ -55,17 +55,19 @@ void DescriptorArray::CopyEnumCacheFrom(DescriptorArray array) {
   set_enum_cache(array.enum_cache());
 }
 
-InternalIndex DescriptorArray::Search(Name name, int valid_descriptors) {
+InternalIndex DescriptorArray::Search(Name name, int valid_descriptors,
+                                      bool concurrent_search) {
   DCHECK(name.IsUniqueName());
-  return InternalIndex(
-      internal::Search<VALID_ENTRIES>(this, name, valid_descriptors, nullptr));
+  return InternalIndex(internal::Search<VALID_ENTRIES>(
+      this, name, valid_descriptors, nullptr, concurrent_search));
 }
 
-InternalIndex DescriptorArray::Search(Name name, Map map) {
+InternalIndex DescriptorArray::Search(Name name, Map map,
+                                      bool concurrent_search) {
   DCHECK(name.IsUniqueName());
   int number_of_own_descriptors = map.NumberOfOwnDescriptors();
   if (number_of_own_descriptors == 0) return InternalIndex::NotFound();
-  return Search(name, number_of_own_descriptors);
+  return Search(name, number_of_own_descriptors, concurrent_search);
 }
 
 InternalIndex DescriptorArray::SearchWithCache(Isolate* isolate, Name name,

src/objects/descriptor-array.h

@@ -115,9 +115,13 @@ class DescriptorArray
   // Sort the instance descriptors by the hash codes of their keys.
   void Sort();
 
-  // Search the instance descriptors for given name.
-  V8_INLINE InternalIndex Search(Name name, int number_of_own_descriptors);
-  V8_INLINE InternalIndex Search(Name name, Map map);
+  // Search the instance descriptors for given name. {concurrent_search} signals
+  // if we are doing the search on a background thread. If so, we will sacrifice
+  // speed for thread-safety.
+  V8_INLINE InternalIndex Search(Name name, int number_of_own_descriptors,
+                                 bool concurrent_search = false);
+  V8_INLINE InternalIndex Search(Name name, Map map,
+                                 bool concurrent_search = false);
 
   // As the above, but uses DescriptorLookupCache and updates it when
   // necessary.

src/objects/fixed-array-inl.h

@@ -294,8 +294,9 @@ int LinearSearch(T* array, Name name, int valid_entries,
 }
 
 template <SearchMode search_mode, typename T>
-int Search(T* array, Name name, int valid_entries, int* out_insertion_index) {
-  SLOW_DCHECK(array->IsSortedNoDuplicates());
+int Search(T* array, Name name, int valid_entries, int* out_insertion_index,
+           bool concurrent_search) {
+  SLOW_DCHECK_IMPLIES(!concurrent_search, array->IsSortedNoDuplicates());
 
   if (valid_entries == 0) {
     if (search_mode == ALL_ENTRIES && out_insertion_index != nullptr) {
@@ -304,14 +305,14 @@ int Search(T* array, Name name, int valid_entries, int* out_insertion_index) {
     return T::kNotFound;
   }
 
-  // Fast case: do linear search for small arrays.
+  // Do linear search for small arrays, and for searches in the background
+  // thread.
   const int kMaxElementsForLinearSearch = 8;
-  if (valid_entries <= kMaxElementsForLinearSearch) {
+  if (valid_entries <= kMaxElementsForLinearSearch || concurrent_search) {
     return LinearSearch<search_mode>(array, name, valid_entries,
                                      out_insertion_index);
   }
 
-  // Slow case: perform binary search.
   return BinarySearch<search_mode>(array, name, valid_entries,
                                    out_insertion_index);
 }

src/objects/fixed-array.h

@@ -465,7 +465,8 @@ enum SearchMode { ALL_ENTRIES, VALID_ENTRIES };
 
 template <SearchMode search_mode, typename T>
 inline int Search(T* array, Name name, int valid_entries = 0,
-                  int* out_insertion_index = nullptr);
+                  int* out_insertion_index = nullptr,
+                  bool concurrent_search = false);
 
 // ByteArray represents fixed sized byte arrays.  Used for the relocation info
 // that is attached to code objects.

src/objects/map.h

@@ -594,6 +594,7 @@ class Map : public HeapObject {
                              WriteBarrierMode mode = UPDATE_WRITE_BARRIER);
 
   // [instance descriptors]: describes the object.
+  DECL_GETTER(synchronized_instance_descriptors, DescriptorArray)
   DECL_GETTER(instance_descriptors, DescriptorArray)
   V8_EXPORT_PRIVATE void SetInstanceDescriptors(Isolate* isolate,
                                                 DescriptorArray descriptors,
@@ -976,7 +977,8 @@ class Map : public HeapObject {
       MaybeHandle<Object> new_value);
 
   // Use the high-level instance_descriptors/SetInstanceDescriptors instead.
-  DECL_ACCESSORS(synchronized_instance_descriptors, DescriptorArray)
+  inline void set_synchronized_instance_descriptors(
+      DescriptorArray value, WriteBarrierMode mode = UPDATE_WRITE_BARRIER);
 
   static const int kFastPropertiesSoftLimit = 12;
   static const int kMaxFastProperties = 128;

test/cctest/BUILD.gn

@@ -196,6 +196,7 @@ v8_source_set("cctest_sources") {
     "test-code-pages.cc",
     "test-code-stub-assembler.cc",
     "test-compiler.cc",
+    "test-concurrent-descriptor-array.cc",
     "test-constantpool.cc",
     "test-conversions.cc",
     "test-cpu-profiler.cc",

test/cctest/test-concurrent-descriptor-array.cc

@@ -0,0 +1,128 @@
+// 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 "test/cctest/cctest.h"
+#include "test/cctest/heap/heap-utils.h"
+
+namespace v8 {
+namespace internal {
+
+static constexpr int kNumHandles = kHandleBlockSize * 2 + kHandleBlockSize / 2;
+
+namespace {
+
+class PersistentHandlesThread final : public v8::base::Thread {
+ public:
+  PersistentHandlesThread(Heap* heap, std::vector<Handle<JSObject>> handles,
+                          std::unique_ptr<PersistentHandles> ph,
+                          Handle<Name> name, base::Semaphore* sema_started)
+      : v8::base::Thread(base::Thread::Options("ThreadWithLocalHeap")),
+        heap_(heap),
+        handles_(std::move(handles)),
+        ph_(std::move(ph)),
+        name_(name),
+        sema_started_(sema_started) {}
+
+  void Run() override {
+    LocalHeap local_heap(heap_, std::move(ph_));
+    LocalHandleScope scope(&local_heap);
+    Address object = handles_[0]->ptr();
+
+    for (int i = 0; i < kNumHandles; i++) {
+      handles_.push_back(
+          Handle<JSObject>::cast(local_heap.NewPersistentHandle(object)));
+    }
+
+    sema_started_->Signal();
+
+    for (Handle<JSObject> handle : handles_) {
+      // Lookup the named property on the {map}.
+      CHECK(name_->IsUniqueName());
+      Handle<Map> map(handle->map(), &local_heap);
+
+      Handle<DescriptorArray> descriptors(
+          map->synchronized_instance_descriptors(), &local_heap);
+      bool is_background_thread = true;
+      InternalIndex const number =
+          descriptors->Search(*name_, *map, is_background_thread);
+      CHECK(number.is_found());
+    }
+
+    CHECK_EQ(handles_.size(), kNumHandles * 2);
+
+    CHECK(!ph_);
+    ph_ = local_heap.DetachPersistentHandles();
+  }
+
+  Heap* heap_;
+  std::vector<Handle<JSObject>> handles_;
+  std::unique_ptr<PersistentHandles> ph_;
+  Handle<Name> name_;
+  base::Semaphore* sema_started_;
+};
+
+// Uses linear search on a flat object, with up to 8 elements.
+TEST(LinearSearchFlatObject) {
+  CcTest::InitializeVM();
+  FLAG_local_heaps = true;
+  Isolate* isolate = CcTest::i_isolate();
+
+  std::unique_ptr<PersistentHandles> ph = isolate->NewPersistentHandles();
+  std::vector<Handle<JSObject>> handles;
+
+  auto factory = isolate->factory();
+  HandleScope handle_scope(isolate);
+
+  Handle<JSFunction> function =
+      factory->NewFunctionForTest(factory->empty_string());
+  Handle<JSObject> js_object = factory->NewJSObject(function);
+  Handle<String> name = CcTest::MakeString("property");
+  Handle<Object> value = CcTest::MakeString("dummy_value");
+  // For the default constructor function no in-object properties are reserved
+  // hence adding a single property will initialize the property-array.
+  JSObject::DefinePropertyOrElementIgnoreAttributes(js_object, name, value,
+                                                    NONE)
+      .Check();
+
+  Address object = js_object->ptr();
+  for (int i = 0; i < kNumHandles; i++) {
+    handles.push_back(Handle<JSObject>::cast(ph->NewHandle(object)));
+  }
+
+  Handle<Name> persistent_name = Handle<Name>::cast(ph->NewHandle(name->ptr()));
+
+  base::Semaphore sema_started(0);
+
+  // Pass persistent handles to background thread.
+  std::unique_ptr<PersistentHandlesThread> thread(new PersistentHandlesThread(
+      isolate->heap(), std::move(handles), std::move(ph), persistent_name,
+      &sema_started));
+  CHECK(thread->Start());
+
+  sema_started.Wait();
+
+  // Exercise descriptor in main thread too.
+  for (int i = 0; i < 7; ++i) {
+    Handle<String> filler_name = CcTest::MakeName("filler_property_", i);
+    Handle<Object> filler_value = CcTest::MakeString("dummy_value");
+    JSObject::DefinePropertyOrElementIgnoreAttributes(js_object, filler_name,
+                                                      filler_value, NONE)
+        .Check();
+  }
+  CHECK_EQ(js_object->map().NumberOfOwnDescriptors(), 8);
+
+  thread->Join();
+}
+
+}  // anonymous namespace
+
+}  // namespace internal
+}  // namespace v8

test/cctest/test-local-handles.cc

@@ -20,6 +20,8 @@
 namespace v8 {
 namespace internal {
 
+namespace {
+
 class LocalHandlesThread final : public v8::base::Thread {
  public:
   LocalHandlesThread(Heap* heap, Address object, base::Semaphore* sema_started,
@@ -92,5 +94,7 @@ TEST(CreateLocalHandles) {
   thread->Join();
 }
 
+}  // anonymous namespace
+
 }  // namespace internal
 }  // namespace v8

test/cctest/test-persistent-handles.cc

@@ -23,6 +23,8 @@ namespace internal {
 
 static constexpr int kNumHandles = kHandleBlockSize * 2 + kHandleBlockSize / 2;
 
+namespace {
+
 class PersistentHandlesThread final : public v8::base::Thread {
  public:
   PersistentHandlesThread(Heap* heap, std::vector<Handle<HeapNumber>> handles,
@@ -110,5 +112,7 @@ TEST(CreatePersistentHandles) {
   ph->NewHandle(number->ptr());
 }
 
+}  // anonymous namespace
+
 }  // namespace internal
 }  // namespace v8