From f707a4d8f0f1a474b304d29ebf6ebb3843eb6381 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dominik=20Inf=C3=BChr?= Date: Mon, 6 Feb 2023 12:46:15 +0100 Subject: [PATCH] [heap] Do not shortcut strings when shared marking is active MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We DCHECK in the scavenger that evacuated objects are not allocated on evacuation candidates. This DCHECK fails however when shortcutting ThinStrings to the actual string object when incremental marking is enabled in the shared heap. We fix this by disabling shortcutting of strings when shared incremental marking is enabled. We already do this for incremental marking in the local isolate. Bug: v8:13267, chromium:1412643 Change-Id: I2a61028ae5377c7621b917ed332e15d6b25b80ff Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4219781 Commit-Queue: Michael Lippautz Auto-Submit: Dominik Inführ Reviewed-by: Michael Lippautz Cr-Commit-Position: refs/heads/main@{#85677} --- src/heap/scavenger-inl.h | 4 ++-- src/heap/scavenger.cc | 18 ++++++++++++++++-- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/src/heap/scavenger-inl.h b/src/heap/scavenger-inl.h index bc3b97f667..96a192f076 100644 --- a/src/heap/scavenger-inl.h +++ b/src/heap/scavenger-inl.h @@ -298,7 +298,7 @@ SlotCallbackResult Scavenger::EvacuateThinString(Map map, THeapObjectSlot slot, static_assert(std::is_same::value || std::is_same::value, "Only FullHeapObjectSlot and HeapObjectSlot are expected here"); - if (!is_incremental_marking_ && shortcut_strings_) { + if (shortcut_strings_) { // The ThinString should die after Scavenge, so avoid writing the proper // forwarding pointer and instead just signal the actual object as forwarded // reference. @@ -326,7 +326,7 @@ SlotCallbackResult Scavenger::EvacuateShortcutCandidate(Map map, "Only FullHeapObjectSlot and HeapObjectSlot are expected here"); DCHECK(IsShortcutCandidate(map.instance_type())); - if (!is_incremental_marking_ && shortcut_strings_ && + if (shortcut_strings_ && object.unchecked_second() == ReadOnlyRoots(heap()).empty_string()) { HeapObject first = HeapObject::cast(object.unchecked_first()); diff --git a/src/heap/scavenger.cc b/src/heap/scavenger.cc index ec4ea57576..d7689171b4 100644 --- a/src/heap/scavenger.cc +++ b/src/heap/scavenger.cc @@ -610,6 +610,18 @@ ConcurrentAllocator* CreateSharedOldAllocator(Heap* heap) { } return nullptr; } + +// This returns true if the scavenger runs in a client isolate and incremental +// marking is enabled in the shared space isolate. +bool IsSharedIncrementalMarking(Isolate* isolate) { + return isolate->has_shared_heap() && v8_flags.shared_space && + !isolate->is_shared_space_isolate() && + isolate->shared_space_isolate() + ->heap() + ->incremental_marking() + ->IsMarking(); +} + } // namespace Scavenger::Scavenger(ScavengerCollector* collector, Heap* heap, bool is_logging, @@ -633,8 +645,10 @@ Scavenger::Scavenger(ScavengerCollector* collector, Heap* heap, bool is_logging, is_compacting_(heap->incremental_marking()->IsCompacting()), shared_string_table_(shared_old_allocator_.get() != nullptr), mark_shared_heap_(heap->isolate()->is_shared_space_isolate()), - shortcut_strings_(!heap->IsGCWithStack() || - v8_flags.shortcut_strings_with_stack) {} + shortcut_strings_( + (!heap->IsGCWithStack() || v8_flags.shortcut_strings_with_stack) && + !is_incremental_marking_ && + !IsSharedIncrementalMarking(heap->isolate())) {} void Scavenger::IterateAndScavengePromotedObject(HeapObject target, Map map, int size) {