[heap] Make CodeObjectRegistry a separate class.

Bug: v8:9093
Change-Id: I02360627776715ae2561f8535dbf97ed0cd3c51a
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1613978
Commit-Queue: Hannes Payer <hpayer@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#61587}

src/heap/heap-inl.h

@@ -225,8 +225,9 @@ AllocationResult Heap::AllocateRaw(int size_in_bytes, AllocationType type,
       UnprotectAndRegisterMemoryChunk(object);
       ZapCodeObject(object->address(), size_in_bytes);
       if (!large_object) {
-        MemoryChunk::FromHeapObject(object)->RegisterNewlyAllocatedCodeObject(
-            object);
+        MemoryChunk::FromHeapObject(object)
+            ->GetCodeObjectRegistry()
+            ->RegisterNewlyAllocatedCodeObject(object->address());
       }
     }
     OnAllocationEvent(object, size_in_bytes);

src/heap/heap.cc

@@ -5788,8 +5788,10 @@ Code Heap::GcSafeFindCodeForInnerPointer(Address inner_pointer) {
   // after the inner pointer.
   Page* page = Page::FromAddress(inner_pointer);
 
-  HeapObject object = page->GetCodeObjectFromInnerAddress(inner_pointer);
-  return GcSafeCastToCode(object, inner_pointer);
+  Address start =
+      page->GetCodeObjectRegistry()->GetCodeObjectStartFromInnerAddress(
+          inner_pointer);
+  return GcSafeCastToCode(HeapObject::FromAddress(start), inner_pointer);
 }
 
 void Heap::WriteBarrierForCodeSlow(Code code) {

src/heap/mark-compact.cc

@@ -1279,7 +1279,8 @@ class EvacuateVisitorBase : public HeapObjectVisitor {
       MigrateObject(*target_object, object, size, target_space);
       if (target_space == CODE_SPACE)
         MemoryChunk::FromHeapObject(*target_object)
-            ->RegisterNewlyAllocatedCodeObject(*target_object);
+            ->GetCodeObjectRegistry()
+            ->RegisterNewlyAllocatedCodeObject((*target_object).address());
       return true;
     }
     return false;

src/heap/spaces.cc

@@ -614,63 +614,64 @@ void MemoryChunk::SetReadAndWritable() {
   }
 }
 
-void MemoryChunk::RegisterNewlyAllocatedCodeObject(HeapObject code) {
-  DCHECK(Contains(code->address()));
-  DCHECK(MemoryChunk::FromHeapObject(code)->owner()->identity() == CODE_SPACE);
-  auto result = code_object_registry_newly_allocated_->insert(code->address());
+void CodeObjectRegistry::RegisterNewlyAllocatedCodeObject(Address code) {
+  auto result = code_object_registry_newly_allocated_.insert(code);
   USE(result);
   DCHECK(result.second);
 }
 
-void MemoryChunk::RegisterAlreadyExistingCodeObject(HeapObject code) {
-  DCHECK(MemoryChunk::FromHeapObject(code)->owner()->identity() == CODE_SPACE);
-  code_object_registry_already_existing_->push_back(code->address());
+void CodeObjectRegistry::RegisterAlreadyExistingCodeObject(Address code) {
+  code_object_registry_already_existing_.push_back(code);
 }
 
-void MemoryChunk::ClearCodeObjectRegistries() {
-  DCHECK(code_object_registry_already_existing_);
-  DCHECK(code_object_registry_newly_allocated_);
-  code_object_registry_already_existing_->clear();
-  code_object_registry_newly_allocated_->clear();
+void CodeObjectRegistry::Clear() {
+  code_object_registry_already_existing_.clear();
+  code_object_registry_newly_allocated_.clear();
 }
 
-void MemoryChunk::FinalizeCodeObjectRegistries() {
-  DCHECK(code_object_registry_already_existing_);
-  DCHECK(code_object_registry_newly_allocated_);
-  code_object_registry_already_existing_->shrink_to_fit();
+void CodeObjectRegistry::Finalize() {
+  code_object_registry_already_existing_.shrink_to_fit();
 }
 
-bool MemoryChunk::CodeObjectRegistryContains(HeapObject object) {
-  return (code_object_registry_newly_allocated_->find(object->address()) !=
-          code_object_registry_newly_allocated_->end()) ||
-         (std::binary_search(code_object_registry_already_existing_->begin(),
-                             code_object_registry_already_existing_->end(),
-                             object->address()));
+bool CodeObjectRegistry::Contains(Address object) const {
+  return (code_object_registry_newly_allocated_.find(object) !=
+          code_object_registry_newly_allocated_.end()) ||
+         (std::binary_search(code_object_registry_already_existing_.begin(),
+                             code_object_registry_already_existing_.end(),
+                             object));
 }
 
-Code MemoryChunk::GetCodeObjectFromInnerAddress(Address address) {
-  DCHECK(Contains(address));
+Address CodeObjectRegistry::GetCodeObjectStartFromInnerAddress(
+    Address address) const {
+  // Let's first find the object which comes right before address in the vector
+  // of already existing code objects.
+  Address already_existing_set_ = 0;
+  Address newly_allocated_set_ = 0;
+  if (!code_object_registry_already_existing_.empty()) {
+    auto it =
+        std::upper_bound(code_object_registry_already_existing_.begin(),
+                         code_object_registry_already_existing_.end(), address);
+    if (it != code_object_registry_already_existing_.begin()) {
+      already_existing_set_ = *(--it);
+    }
+  }
 
-  // Let's first try the std::vector which holds object that already existed
-  // since the last GC cycle.
-  if (!code_object_registry_already_existing_->empty()) {
-    auto it = std::upper_bound(code_object_registry_already_existing_->begin(),
-                               code_object_registry_already_existing_->end(),
-                               address);
-    if (it != code_object_registry_already_existing_->begin()) {
-      Code code = Code::unchecked_cast(HeapObject::FromAddress(*(--it)));
-      if (heap_->GcSafeCodeContains(code, address)) {
-        return code;
-      }
+  // Next, let's find the object which comes right before address in the set
+  // of newly allocated code objects.
+  if (!code_object_registry_newly_allocated_.empty()) {
+    auto it = code_object_registry_newly_allocated_.upper_bound(address);
+    if (it != code_object_registry_newly_allocated_.begin()) {
+      newly_allocated_set_ = *(--it);
     }
   }
 
-  // If the address was not found, it has to be in the newly allocated code
-  // objects registry.
-  DCHECK(!code_object_registry_newly_allocated_->empty());
-  auto it = code_object_registry_newly_allocated_->upper_bound(address);
-  HeapObject obj = HeapObject::FromAddress(*(--it));
-  return heap_->GcSafeCastToCode(obj, address);
+  // The code objects which contains address has to be in one of the two
+  // data structures.
+  DCHECK(already_existing_set_ != 0 || newly_allocated_set_ != 0);
+
+  // The address which is closest to the given address is the code object.
+  return already_existing_set_ > newly_allocated_set_ ? already_existing_set_
+                                                      : newly_allocated_set_;
 }
 
 namespace {
@@ -759,11 +760,9 @@ MemoryChunk* MemoryChunk::Initialize(Heap* heap, Address base, size_t size,
   chunk->reservation_ = std::move(reservation);
 
   if (owner->identity() == CODE_SPACE) {
-    chunk->code_object_registry_newly_allocated_ = new std::set<Address>();
-    chunk->code_object_registry_already_existing_ = new std::vector<Address>();
+    chunk->code_object_registry_ = new CodeObjectRegistry();
   } else {
-    chunk->code_object_registry_newly_allocated_ = nullptr;
-    chunk->code_object_registry_already_existing_ = nullptr;
+    chunk->code_object_registry_ = nullptr;
   }
 
   return chunk;
@@ -1386,10 +1385,7 @@ void MemoryChunk::ReleaseAllocatedMemory() {
   if (local_tracker_ != nullptr) ReleaseLocalTracker();
   if (young_generation_bitmap_ != nullptr) ReleaseYoungGenerationBitmap();
   if (marking_bitmap_ != nullptr) ReleaseMarkingBitmap();
-  if (code_object_registry_newly_allocated_ != nullptr)
-    delete code_object_registry_newly_allocated_;
-  if (code_object_registry_already_existing_ != nullptr)
-    delete code_object_registry_already_existing_;
+  if (code_object_registry_ != nullptr) delete code_object_registry_;
 
   if (!IsLargePage()) {
     Page* page = static_cast<Page*>(this);

src/heap/spaces.h

@@ -230,6 +230,24 @@ class FreeListCategory {
   DISALLOW_IMPLICIT_CONSTRUCTORS(FreeListCategory);
 };
 
+// The CodeObjectRegistry holds all start addresses of code objects of a given
+// MemoryChunk. Each MemoryChunk owns a separate CodeObjectRegistry. The
+// CodeObjectRegistry allows fast lookup from an inner pointer of a code object
+// to the actual code object.
+class V8_EXPORT_PRIVATE CodeObjectRegistry {
+ public:
+  void RegisterNewlyAllocatedCodeObject(Address code);
+  void RegisterAlreadyExistingCodeObject(Address code);
+  void Clear();
+  void Finalize();
+  bool Contains(Address code) const;
+  Address GetCodeObjectStartFromInnerAddress(Address address) const;
+
+ private:
+  std::vector<Address> code_object_registry_already_existing_;
+  std::set<Address> code_object_registry_newly_allocated_;
+};
+
 class V8_EXPORT_PRIVATE MemoryChunkLayout {
  public:
   static size_t CodePageGuardStartOffset();
@@ -401,10 +419,8 @@ class MemoryChunk {
       // FreeListCategory categories_[kNumberOfCategories]
       + kSystemPointerSize  // LocalArrayBufferTracker* local_tracker_
       + kIntptrSize  // std::atomic<intptr_t> young_generation_live_byte_count_
-      + kSystemPointerSize  // Bitmap* young_generation_bitmap_
-      +
-      kSystemPointerSize  // std::vector code_object_registry_already_existing_
-      + kSystemPointerSize;  // std::set code_object_registry_newly_allocated_
+      + kSystemPointerSize   // Bitmap* young_generation_bitmap_
+      + kSystemPointerSize;  // CodeObjectRegistry* code_object_registry_
 
   // Page size in bytes.  This must be a multiple of the OS page size.
   static const int kPageSize = 1 << kPageSizeBits;
@@ -675,12 +691,7 @@ class MemoryChunk {
 
   base::ListNode<MemoryChunk>& list_node() { return list_node_; }
 
-  V8_EXPORT_PRIVATE void RegisterNewlyAllocatedCodeObject(HeapObject code);
-  V8_EXPORT_PRIVATE void RegisterAlreadyExistingCodeObject(HeapObject code);
-  V8_EXPORT_PRIVATE void ClearCodeObjectRegistries();
-  V8_EXPORT_PRIVATE void FinalizeCodeObjectRegistries();
-  V8_EXPORT_PRIVATE bool CodeObjectRegistryContains(HeapObject code);
-  V8_EXPORT_PRIVATE Code GetCodeObjectFromInnerAddress(Address address);
+  CodeObjectRegistry* GetCodeObjectRegistry() { return code_object_registry_; }
 
  protected:
   static MemoryChunk* Initialize(Heap* heap, Address base, size_t size,
@@ -788,8 +799,7 @@ class MemoryChunk {
   std::atomic<intptr_t> young_generation_live_byte_count_;
   Bitmap* young_generation_bitmap_;
 
-  std::vector<Address>* code_object_registry_already_existing_;
-  std::set<Address>* code_object_registry_newly_allocated_;
+  CodeObjectRegistry* code_object_registry_;
 
  private:
   void InitializeReservedMemory() { reservation_.Reset(); }

src/heap/sweeper.cc

@@ -249,7 +249,7 @@ int Sweeper::RawSweep(Page* p, FreeListRebuildingMode free_list_mode,
          space->identity() == CODE_SPACE || space->identity() == MAP_SPACE);
   DCHECK(!p->IsEvacuationCandidate() && !p->SweepingDone());
 
-  bool is_code_page = space->identity() == CODE_SPACE;
+  CodeObjectRegistry* code_object_registry = p->GetCodeObjectRegistry();
 
   // TODO(ulan): we don't have to clear type old-to-old slots in code space
   // because the concurrent marker doesn't mark code objects. This requires
@@ -275,12 +275,14 @@ int Sweeper::RawSweep(Page* p, FreeListRebuildingMode free_list_mode,
   // live bytes and keep track of wasted_memory_.
   p->ResetAllocationStatistics();
 
-  if (is_code_page) p->ClearCodeObjectRegistries();
+  if (code_object_registry) code_object_registry->Clear();
 
   for (auto object_and_size :
        LiveObjectRange<kBlackObjects>(p, marking_state_->bitmap(p))) {
     HeapObject const object = object_and_size.first;
-    if (is_code_page) p->RegisterAlreadyExistingCodeObject(object);
+    if (code_object_registry)
+      code_object_registry->RegisterAlreadyExistingCodeObject(
+          object->address());
     DCHECK(marking_state_->IsBlack(object));
     Address free_end = object->address();
     if (free_end != free_start) {
@@ -367,7 +369,7 @@ int Sweeper::RawSweep(Page* p, FreeListRebuildingMode free_list_mode,
     DCHECK_EQ(live_bytes, p->allocated_bytes());
   }
   p->set_concurrent_sweeping_state(Page::kSweepingDone);
-  if (is_code_page) p->FinalizeCodeObjectRegistries();
+  if (code_object_registry) code_object_registry->Finalize();
   if (free_list_mode == IGNORE_FREE_LIST) return 0;
   return static_cast<int>(FreeList::GuaranteedAllocatable(max_freed_bytes));
 }

src/snapshot/deserializer-allocator.cc

@@ -45,8 +45,9 @@ Address DeserializerAllocator::AllocateRaw(AllocationSpace space, int size) {
     DCHECK_LE(high_water_[space], reservation[chunk_index].end);
 #endif
     if (space == CODE_SPACE)
-      MemoryChunk::FromAddress(address)->RegisterNewlyAllocatedCodeObject(
-          HeapObject::FromAddress(address));
+      MemoryChunk::FromAddress(address)
+          ->GetCodeObjectRegistry()
+          ->RegisterNewlyAllocatedCodeObject(address);
     return address;
   }
 }

test/cctest/heap/test-heap.cc

@@ -6678,17 +6678,12 @@ TEST(CodeObjectRegistry) {
     // objects are on the same page.
     CHECK_EQ(MemoryChunk::FromHeapObject(*code1),
              MemoryChunk::FromHeapObject(*code2));
-    CHECK(MemoryChunk::FromHeapObject(*code1)->CodeObjectRegistryContains(
-        *code1));
-    CHECK(MemoryChunk::FromHeapObject(*code2)->CodeObjectRegistryContains(
-        *code2));
+    CHECK(MemoryChunk::FromHeapObject(*code1)->Contains(code1->address()));
+    CHECK(MemoryChunk::FromHeapObject(*code2)->Contains(code2->address()));
   }
   CcTest::CollectAllAvailableGarbage();
-  CHECK(
-      MemoryChunk::FromHeapObject(*code1)->CodeObjectRegistryContains(*code1));
-  CHECK(
-      MemoryChunk::FromAddress(code2_address)
-          ->CodeObjectRegistryContains(HeapObject::FromAddress(code2_address)));
+  CHECK(MemoryChunk::FromHeapObject(*code1)->Contains(code1->address()));
+  CHECK(MemoryChunk::FromAddress(code2_address)->Contains(code2_address));
 }
 
 }  // namespace heap

test/unittests/BUILD.gn

@@ -154,6 +154,7 @@ v8_source_set("unittests_sources") {
     "heap/barrier-unittest.cc",
     "heap/bitmap-test-utils.h",
     "heap/bitmap-unittest.cc",
+    "heap/code-object-registry-unittest.cc",
     "heap/embedder-tracing-unittest.cc",
     "heap/gc-idle-time-handler-unittest.cc",
     "heap/gc-tracer-unittest.cc",

test/unittests/heap/code-object-registry-unittest.cc

+// Copyright 2015 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/heap/spaces.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace v8 {
+namespace internal {
+
+TEST(CodeObjectRegistry, RegisterAlreadyExistingObjectsAndContains) {
+  CodeObjectRegistry registry;
+  const int elements = 10;
+  const int offset = 100;
+  for (int i = 0; i < elements; i++) {
+    registry.RegisterAlreadyExistingCodeObject(i * offset);
+  }
+
+  for (int i = 0; i < elements; i++) {
+    CHECK(registry.Contains(i * offset));
+  }
+}
+
+TEST(CodeObjectRegistry, RegisterNewlyAllocatedObjectsAndContains) {
+  CodeObjectRegistry registry;
+  const int elements = 10;
+  const int offset = 100;
+  for (int i = 0; i < elements; i++) {
+    registry.RegisterNewlyAllocatedCodeObject(i * offset);
+  }
+
+  for (int i = 0; i < elements; i++) {
+    CHECK(registry.Contains(i * offset));
+  }
+}
+
+TEST(CodeObjectRegistry, FindAlreadyExistingObjects) {
+  CodeObjectRegistry registry;
+  const int elements = 10;
+  const int offset = 100;
+  const int inner = 2;
+  for (int i = 1; i <= elements; i++) {
+    registry.RegisterAlreadyExistingCodeObject(i * offset);
+  }
+
+  for (int i = 1; i <= elements; i++) {
+    for (int j = 0; j < inner; j++) {
+      CHECK_EQ(registry.GetCodeObjectStartFromInnerAddress(i * offset + j),
+               i * offset);
+    }
+  }
+}
+
+TEST(CodeObjectRegistry, FindNewlyAllocatedObjects) {
+  CodeObjectRegistry registry;
+  const int elements = 10;
+  const int offset = 100;
+  const int inner = 2;
+  for (int i = 1; i <= elements; i++) {
+    registry.RegisterNewlyAllocatedCodeObject(i * offset);
+  }
+
+  for (int i = 1; i <= elements; i++) {
+    for (int j = 0; j < inner; j++) {
+      CHECK_EQ(registry.GetCodeObjectStartFromInnerAddress(i * offset + j),
+               i * offset);
+    }
+  }
+}
+
+TEST(CodeObjectRegistry, FindAlternatingObjects) {
+  CodeObjectRegistry registry;
+  const int elements = 10;
+  const int offset = 100;
+  const int inner = 2;
+  for (int i = 1; i <= elements; i++) {
+    if (i % 2 == 0) {
+      registry.RegisterAlreadyExistingCodeObject(i * offset);
+    } else {
+      registry.RegisterNewlyAllocatedCodeObject(i * offset);
+    }
+  }
+
+  for (int i = 1; i <= elements; i++) {
+    for (int j = 0; j < inner; j++) {
+      CHECK_EQ(registry.GetCodeObjectStartFromInnerAddress(i * offset + j),
+               i * offset);
+    }
+  }
+}
+}  // namespace internal
+}  // namespace v8