[heap] Split string table clearing in atomic GC pause Split string table clearing into multiple parallel chunks for better parallelization during GC clearing phase. We have seen Jetstream3 runs where this phase alone took over 6ms. It is also generally the most expensive phase within ClearNonLiveReferences(). Bug: 463528049 Cq-Include-Trybots: luci.v8.try:v8_linux64_tsan_isolates_rel,v8_linux64_tsan_no_cm_rel,v8_numfuzz_tsan_rel,v8_linux64_tsan_dbg Change-Id: Iefe9f28f52dfc3b0ed598c01eb9ea80ac9ff4897 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/7523526 Reviewed-by: Michael Lippautz <mlippautz@chromium.org> Commit-Queue: Dominik Inführ <dinfuehr@chromium.org> Cr-Commit-Position: refs/heads/main@{#104964}
diff --git a/src/heap/mark-compact.cc b/src/heap/mark-compact.cc index b4d591c..9385b46 100644 --- a/src/heap/mark-compact.cc +++ b/src/heap/mark-compact.cc
@@ -3056,6 +3056,7 @@ auto parallel_clearing_job = std::make_unique<ParallelClearingJob>(this); Isolate* const isolate = heap_->isolate(); + std::atomic<int> string_table_removed_count{0}; if (isolate->OwnsStringTables()) { TRACE_GC(heap_->tracer(), @@ -3087,27 +3088,49 @@ } if (isolate->OwnsStringTables()) { - auto item = - MakeParallelItem("ClearStringTable", [this, isolate]( - ParallelItem* item, - JobDelegate* delegate) { - DCHECK(isolate->OwnsStringTables()); + StringTable* string_table = isolate->string_table(); + string_table->DropOldData(); + // Splitting the string table into chunks for parallel processing. Never + // choose more than kMaxStringTableChunks and each chunk should have at + // least kMinStringTableChunkSize entries. + constexpr int kMaxStringTableChunks = 8; + constexpr int kMinStringTableChunkSize = 1024; + const int capacity = string_table->Capacity(); + const int target_chunk_count = + (capacity + kMinStringTableChunkSize - 1) / kMinStringTableChunkSize; + const int chunk_count = + std::max(1, std::min(kMaxStringTableChunks, target_chunk_count)); + const int chunk_size = (capacity + chunk_count - 1) / chunk_count; + for (int chunk = 0; chunk < chunk_count; ++chunk) { + const int start = chunk * chunk_size; + const int end = std::min(capacity, start + chunk_size); + DCHECK_LT(start, end); + auto item = + MakeParallelItem("ClearStringTable", [this, isolate, start, end, + &string_table_removed_count]( + ParallelItem* item, + JobDelegate* delegate) { + DCHECK(isolate->OwnsStringTables()); - TRACE_GC1_WITH_FLOW(heap()->tracer(), - GCTracer::Scope::MC_CLEAR_STRING_TABLE, delegate, - item->trace_id(), TRACE_EVENT_FLAG_FLOW_IN); - // Prune the string table removing all strings only pointed to by - // the string table. Cannot use string_table() here because the - // string table is marked. - StringTable* string_table = isolate->string_table(); - InternalizedStringTableCleaner internalized_visitor(heap()); - string_table->DropOldData(); - string_table->IterateElements(&internalized_visitor); - string_table->NotifyElementsRemoved( - internalized_visitor.PointersRemoved()); - }).Enqueue(parallel_clearing_job); - TRACE_GC_NOTE_WITH_FLOW("ClearStringTableJob started", item->trace_id(), - TRACE_EVENT_FLAG_FLOW_OUT); + TRACE_GC1_WITH_FLOW( + heap()->tracer(), GCTracer::Scope::MC_CLEAR_STRING_TABLE, + delegate, item->trace_id(), TRACE_EVENT_FLAG_FLOW_IN); + // Prune the string table removing all strings only pointed to + // by the string table. Cannot use string_table() here because + // the string table is marked. + StringTable* string_table = isolate->string_table(); + InternalizedStringTableCleaner internalized_visitor(heap()); + string_table->IterateElementsRange(&internalized_visitor, start, + end); + const int removed = internalized_visitor.PointersRemoved(); + if (removed > 0) { + string_table_removed_count.fetch_add(removed, + std::memory_order_relaxed); + } + }).Enqueue(parallel_clearing_job); + TRACE_GC_NOTE_WITH_FLOW("ClearStringTableJob started", item->trace_id(), + TRACE_EVENT_FLAG_FLOW_OUT); + } } if (isolate->is_shared_space_isolate() && @@ -3418,6 +3441,15 @@ job->Join(); } + // Finish clearing the string table after all parallel jobs have completed. + if (isolate->OwnsStringTables()) { + const int removed = + string_table_removed_count.load(std::memory_order_relaxed); + if (removed > 0) { + isolate->string_table()->NotifyElementsRemoved(removed); + } + } + PROFILE(heap_->isolate(), WeakCodeClearEvent()); if (v8_flags.sticky_mark_bits) {
diff --git a/src/init/heap-symbols.h b/src/init/heap-symbols.h index d7c2d60..2ecffa6 100644 --- a/src/init/heap-symbols.h +++ b/src/init/heap-symbols.h
@@ -1072,7 +1072,6 @@ F(MC_CLEAR_SHARED_STRUCT_TYPE_REGISTRY) \ F(MC_CLEAR_SLOTS_BUFFER) \ F(MC_CLEAR_STRING_FORWARDING_TABLE) \ - F(MC_CLEAR_STRING_TABLE) \ F(MC_CLEAR_SWEEP_CODE_POINTER_TABLE) \ F(MC_CLEAR_SWEEP_EXTERNAL_POINTER_TABLE) \ F(MC_CLEAR_SWEEP_JS_DISPATCH_TABLE) \ @@ -1165,6 +1164,7 @@ F(MC_BACKGROUND_EVACUATE_UPDATE_POINTERS) \ F(MC_BACKGROUND_MARKING) \ F(MC_BACKGROUND_SWEEPING) \ + F(MC_CLEAR_STRING_TABLE) \ F(MINOR_MS_BACKGROUND_MARKING) \ F(MINOR_MS_BACKGROUND_SWEEPING) \ F(MINOR_MS_BACKGROUND_MARKING_CLOSURE) \
diff --git a/src/objects/off-heap-hash-table-inl.h b/src/objects/off-heap-hash-table-inl.h index 892eec7..28450b9 100644 --- a/src/objects/off-heap-hash-table-inl.h +++ b/src/objects/off-heap-hash-table-inl.h
@@ -121,6 +121,18 @@ } template <typename Derived> +void OffHeapHashTableBase<Derived>::IterateElementsRange(Root root, + RootVisitor* visitor, + int start, int end) { + DCHECK_LE(0, start); + DCHECK_LE(start, end); + DCHECK_LE(end, capacity_); + OffHeapObjectSlot first_slot = slot(InternalIndex(start)); + OffHeapObjectSlot end_slot = slot(InternalIndex(end)); + visitor->VisitCompressedRootPointers(root, nullptr, first_slot, end_slot); +} + +template <typename Derived> template <typename IsolateT, typename FindKey> InternalIndex OffHeapHashTableBase<Derived>::FindEntry(IsolateT* isolate, FindKey key,
diff --git a/src/objects/off-heap-hash-table.h b/src/objects/off-heap-hash-table.h index 21d83c2..00f71af 100644 --- a/src/objects/off-heap-hash-table.h +++ b/src/objects/off-heap-hash-table.h
@@ -138,6 +138,8 @@ inline void RehashInto(PtrComprCageBase cage_base, Derived* new_table); inline void IterateElements(Root root, RootVisitor* visitor); + inline void IterateElementsRange(Root root, RootVisitor* visitor, int start, + int end); protected: explicit OffHeapHashTableBase(int capacity);
diff --git a/src/objects/string-table.cc b/src/objects/string-table.cc index 98ed901..4f4808a4 100644 --- a/src/objects/string-table.cc +++ b/src/objects/string-table.cc
@@ -104,6 +104,10 @@ table_.IterateElements(Root::kStringTable, visitor); } + void IterateElementsRange(RootVisitor* visitor, int start, int end) { + table_.IterateElementsRange(Root::kStringTable, visitor, start, end); + } + Data* PreviousData() { return previous_data_.get(); } void DropPreviousData() { previous_data_.reset(); } @@ -754,6 +758,15 @@ data_.load(std::memory_order_relaxed)->IterateElements(visitor); } +void StringTable::IterateElementsRange(RootVisitor* visitor, int start, + int end) { + // This should only happen during garbage collection when background threads + // are paused, so the load can be relaxed. + isolate_->heap()->safepoint()->AssertActive(); + data_.load(std::memory_order_relaxed) + ->IterateElementsRange(visitor, start, end); +} + void StringTable::DropOldData() { // This should only happen during garbage collection when background threads // are paused, so the load can be relaxed.
diff --git a/src/objects/string-table.h b/src/objects/string-table.h index a848f89..c7fb0d6 100644 --- a/src/objects/string-table.h +++ b/src/objects/string-table.h
@@ -88,6 +88,7 @@ // The following methods must be called either while holding the write lock, // or while in a Heap safepoint. void IterateElements(RootVisitor* visitor); + void IterateElementsRange(RootVisitor* visitor, int start, int end); void DropOldData(); void NotifyElementsRemoved(int count);