From cf7234cc51bdda11cbe849ff3e70094dd5e9d105 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dominik=20Inf=C3=BChr?= Date: Thu, 10 Feb 2022 10:59:38 +0000 Subject: [PATCH] Revert "Reland "Reland "[heap] Support client-to-shared refs in Code objects""" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 2694b75eb97b3d97d1f0b842702a423c0ea667dd. Reason for revert: Causes timeouts on waterfall (https://ci.chromium.org/ui/p/v8/builders/ci/V8%20Linux%20-%20debug/38375/overview) Original change's description: > Reland "Reland "[heap] Support client-to-shared refs in Code objects"" > > This is a reland of 4b8f1b1cff5b3ad70b1d0a8671a371079c380b6a > > After landing https://crrev.com/c/3447371, we can reland this CL as-is > correctness-wise. > > What's new in this CL is that we now treat references from client > objects into the shared heap as roots for the --track-retaining-path > feature. > > Original change's description: > > Reland "[heap] Support client-to-shared refs in Code objects" > > > > This is a reland of 12e46091a00b7e6e4bcde27ed36b7426c82b91a5 > > > > Original change's description: > > > [heap] Support client-to-shared refs in Code objects > > > > > > Support references from code objects in the client heaps to shared heap objects. Such references are stored in a remembered set during marking, which is later used for updating pointers. > > > > > > Bug: v8:11708 > > > Change-Id: I8aeb508ddd14514ca65fa5acf3030dd8c2040168 > > > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3401588 > > > Reviewed-by: Michael Lippautz > > > Reviewed-by: Camillo Bruni > > > Commit-Queue: Dominik Inführ > > > Cr-Commit-Position: refs/heads/main@{#78819} > > > > Bug: v8:11708 > > Change-Id: I47bcf44b452fcffe8675fba03244b736ede14247 > > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3422630 > > Reviewed-by: Camillo Bruni > > Reviewed-by: Michael Lippautz > > Commit-Queue: Dominik Inführ > > Cr-Commit-Position: refs/heads/main@{#78838} > > Bug: v8:11708 > Change-Id: I5b48e942fa469eabb40e797e221d06c25af16443 > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3425358 > Reviewed-by: Michael Lippautz > Reviewed-by: Camillo Bruni > Commit-Queue: Dominik Inführ > Cr-Commit-Position: refs/heads/main@{#79023} Bug: v8:11708 Change-Id: I3c5cb945261882122cd76a50aba5237106a25b65 No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3451719 Auto-Submit: Dominik Inführ Bot-Commit: Rubber Stamper Reviewed-by: Michael Lippautz Reviewed-by: Toon Verwaest Commit-Queue: Toon Verwaest Cr-Commit-Position: refs/heads/main@{#79026} --- src/d8/d8.cc | 7 --- src/heap/heap.cc | 18 ++----- src/heap/mark-compact.cc | 49 ++++--------------- src/heap/marking-visitor-inl.h | 3 +- src/heap/memory-chunk.cc | 5 -- src/objects/visitors.h | 1 - src/runtime/runtime-test.cc | 6 --- src/runtime/runtime.h | 1 - .../shared-string-in-code-object.js | 18 ------- 9 files changed, 15 insertions(+), 93 deletions(-) delete mode 100644 test/mjsunit/shared-memory/shared-string-in-code-object.js diff --git a/src/d8/d8.cc b/src/d8/d8.cc index e0bd604cd9..c0150d3356 100644 --- a/src/d8/d8.cc +++ b/src/d8/d8.cc @@ -50,7 +50,6 @@ #include "src/execution/vm-state-inl.h" #include "src/flags/flags.h" #include "src/handles/maybe-handles.h" -#include "src/heap/parked-scope.h" #include "src/init/v8.h" #include "src/interpreter/interpreter.h" #include "src/logging/counters.h" @@ -4627,12 +4626,6 @@ int Shell::RunMain(Isolate* isolate, bool last_run) { } } CollectGarbage(isolate); - - // Park the main thread here to prevent deadlocks in shared GCs when waiting - // in JoinThread. - i::Isolate* i_isolate = reinterpret_cast(isolate); - i::ParkedScope parked(i_isolate->main_thread_local_isolate()); - for (int i = 1; i < options.num_isolates; ++i) { if (last_run) { options.isolate_sources[i].JoinThread(); diff --git a/src/heap/heap.cc b/src/heap/heap.cc index 8a813ef66d..e6b263e7a0 100644 --- a/src/heap/heap.cc +++ b/src/heap/heap.cc @@ -2385,9 +2385,8 @@ void Heap::PerformSharedGarbageCollection(Isolate* initiator, v8::Locker locker(reinterpret_cast(isolate())); v8::Isolate::Scope isolate_scope(reinterpret_cast(isolate())); - const GarbageCollector collector = GarbageCollector::MARK_COMPACTOR; - - tracer()->StartObservablePause(collector, gc_reason, nullptr); + tracer()->StartObservablePause(GarbageCollector::MARK_COMPACTOR, gc_reason, + nullptr); DCHECK_NOT_NULL(isolate()->global_safepoint()); @@ -2397,23 +2396,12 @@ void Heap::PerformSharedGarbageCollection(Isolate* initiator, // As long as we need to iterate the client heap to find references into the // shared heap, all client heaps need to be iterable. client->heap()->MakeHeapIterable(); - - if (FLAG_concurrent_marking) { - client->heap()->concurrent_marking()->Pause(); - } }); PerformGarbageCollection(GarbageCollector::MARK_COMPACTOR, gc_reason, nullptr); - tracer()->StopObservablePause(collector); - - isolate()->global_safepoint()->IterateClientIsolates([](Isolate* client) { - if (FLAG_concurrent_marking && - client->heap()->incremental_marking()->IsMarking()) { - client->heap()->concurrent_marking()->RescheduleJobIfNeeded(); - } - }); + tracer()->StopObservablePause(GarbageCollector::MARK_COMPACTOR); } void Heap::CompleteSweepingYoung(GarbageCollector collector) { diff --git a/src/heap/mark-compact.cc b/src/heap/mark-compact.cc index c2e3faa03e..84ad47eb11 100644 --- a/src/heap/mark-compact.cc +++ b/src/heap/mark-compact.cc @@ -34,7 +34,6 @@ #include "src/heap/marking-barrier.h" #include "src/heap/marking-visitor-inl.h" #include "src/heap/marking-visitor.h" -#include "src/heap/memory-chunk-layout.h" #include "src/heap/memory-measurement-inl.h" #include "src/heap/memory-measurement.h" #include "src/heap/object-stats.h" @@ -1178,9 +1177,7 @@ class MarkCompactCollector::CustomRootBodyMarkingVisitor final private: V8_INLINE void MarkObject(HeapObject host, Object object) { if (!object.IsHeapObject()) return; - HeapObject heap_object = HeapObject::cast(object); - if (!collector_->is_shared_heap() && heap_object.InSharedHeap()) return; - collector_->MarkObject(host, heap_object); + collector_->MarkObject(host, HeapObject::cast(object)); } MarkCompactCollector* const collector_; @@ -1232,38 +1229,28 @@ class MarkCompactCollector::SharedHeapObjectVisitor final } void VisitCodeTarget(Code host, RelocInfo* rinfo) override { +#if DEBUG Code target = Code::GetCodeFromTargetAddress(rinfo->target_address()); - RecordRelocSlot(host, rinfo, target); + DCHECK(!BasicMemoryChunk::FromHeapObject(target)->InSharedHeap()); +#endif // DEBUG } void VisitEmbeddedPointer(Code host, RelocInfo* rinfo) override { +#if DEBUG HeapObject target = rinfo->target_object(cage_base()); - RecordRelocSlot(host, rinfo, target); + DCHECK(!BasicMemoryChunk::FromHeapObject(target)->InSharedHeap()); +#endif // DEBUG } private: V8_INLINE void MarkObject(HeapObject host, ObjectSlot slot, Object object) { - DCHECK(!host.InSharedHeap()); + DCHECK(!BasicMemoryChunk::FromHeapObject(host)->InSharedHeap()); if (!object.IsHeapObject()) return; HeapObject heap_object = HeapObject::cast(object); - if (!heap_object.InSharedHeap()) return; + if (!BasicMemoryChunk::FromHeapObject(heap_object)->InSharedHeap()) return; RememberedSet::Insert( MemoryChunk::FromHeapObject(host), slot.address()); - collector_->MarkRootObject(Root::kClientHeap, heap_object); - } - - V8_INLINE void RecordRelocSlot(Code host, RelocInfo* rinfo, - HeapObject target) { - if (ShouldRecordRelocSlot(host, rinfo, target)) { - RecordRelocSlotInfo info = ProcessRelocInfo(host, rinfo, target); - RememberedSet::InsertTyped(info.memory_chunk, - info.slot_type, info.offset); - } - } - - V8_INLINE bool ShouldRecordRelocSlot(Code host, RelocInfo* rinfo, - HeapObject target) { - return BasicMemoryChunk::FromHeapObject(target)->InSharedHeap(); + collector_->MarkObject(host, heap_object); } MarkCompactCollector* const collector_; @@ -4635,8 +4622,6 @@ void MarkCompactCollector::UpdatePointersInClientHeap(Isolate* client) { while (chunk_iterator.HasNext()) { MemoryChunk* chunk = chunk_iterator.Next(); - CodePageMemoryModificationScope unprotect_code_page(chunk); - RememberedSet::Iterate( chunk, [cage_base](MaybeObjectSlot slot) { @@ -4645,20 +4630,6 @@ void MarkCompactCollector::UpdatePointersInClientHeap(Isolate* client) { SlotSet::KEEP_EMPTY_BUCKETS); chunk->ReleaseSlotSet(); - - RememberedSet::IterateTyped( - chunk, [this](SlotType slot_type, Address slot) { - // Using UpdateStrongSlot is OK here, because there are no weak - // typed slots. - PtrComprCageBase cage_base = heap_->isolate(); - return UpdateTypedSlotHelper::UpdateTypedSlot( - heap_, slot_type, slot, [cage_base](FullMaybeObjectSlot slot) { - return UpdateStrongSlot(cage_base, - slot); - }); - }); - - chunk->ReleaseTypedSlotSet(); } #ifdef VERIFY_HEAP diff --git a/src/heap/marking-visitor-inl.h b/src/heap/marking-visitor-inl.h index 9171ffc9f7..d73d7c519c 100644 --- a/src/heap/marking-visitor-inl.h +++ b/src/heap/marking-visitor-inl.h @@ -43,7 +43,8 @@ template void MarkingVisitorBase::ProcessStrongHeapObject( HeapObject host, THeapObjectSlot slot, HeapObject heap_object) { concrete_visitor()->SynchronizePageAccess(heap_object); - if (!is_shared_heap_ && heap_object.InSharedHeap()) return; + BasicMemoryChunk* target_page = BasicMemoryChunk::FromHeapObject(heap_object); + if (!is_shared_heap_ && target_page->InSharedHeap()) return; MarkObject(host, heap_object); concrete_visitor()->RecordSlot(host, slot, heap_object); } diff --git a/src/heap/memory-chunk.cc b/src/heap/memory-chunk.cc index 46872dcd8e..ae9259b8d5 100644 --- a/src/heap/memory-chunk.cc +++ b/src/heap/memory-chunk.cc @@ -9,7 +9,6 @@ #include "src/heap/code-object-registry.h" #include "src/heap/memory-allocator.h" #include "src/heap/memory-chunk-inl.h" -#include "src/heap/memory-chunk-layout.h" #include "src/heap/spaces.h" #include "src/objects/heap-object.h" @@ -134,8 +133,6 @@ MemoryChunk* MemoryChunk::Initialize(BasicMemoryChunk* basic_chunk, Heap* heap, nullptr); base::AsAtomicPointer::Release_Store(&chunk->typed_slot_set_[OLD_TO_OLD], nullptr); - base::AsAtomicPointer::Release_Store( - &chunk->typed_slot_set_[CLIENT_TO_SHARED], nullptr); chunk->invalidated_slots_[OLD_TO_NEW] = nullptr; chunk->invalidated_slots_[OLD_TO_OLD] = nullptr; if (V8_EXTERNAL_CODE_SPACE_BOOL) { @@ -316,7 +313,6 @@ void MemoryChunk::ReleaseSlotSet(SlotSet** slot_set) { template TypedSlotSet* MemoryChunk::AllocateTypedSlotSet(); template TypedSlotSet* MemoryChunk::AllocateTypedSlotSet(); -template TypedSlotSet* MemoryChunk::AllocateTypedSlotSet(); template TypedSlotSet* MemoryChunk::AllocateTypedSlotSet() { @@ -333,7 +329,6 @@ TypedSlotSet* MemoryChunk::AllocateTypedSlotSet() { template void MemoryChunk::ReleaseTypedSlotSet(); template void MemoryChunk::ReleaseTypedSlotSet(); -template void MemoryChunk::ReleaseTypedSlotSet(); template void MemoryChunk::ReleaseTypedSlotSet() { diff --git a/src/objects/visitors.h b/src/objects/visitors.h index e0725d5a29..b8100fdf6d 100644 --- a/src/objects/visitors.h +++ b/src/objects/visitors.h @@ -42,7 +42,6 @@ class CodeDataContainer; V(kWrapperTracing, "(Wrapper tracing)") \ V(kWriteBarrier, "(Write barrier)") \ V(kRetainMaps, "(Retain maps)") \ - V(kClientHeap, "(Client heap)") \ V(kUnknown, "(Unknown)") class VisitorSynchronization : public AllStatic { diff --git a/src/runtime/runtime-test.cc b/src/runtime/runtime-test.cc index 72770b289d..74de1eb77a 100644 --- a/src/runtime/runtime-test.cc +++ b/src/runtime/runtime-test.cc @@ -1476,11 +1476,5 @@ RUNTIME_FUNCTION(Runtime_IsSharedString) { Handle::cast(obj)->IsShared()); } -RUNTIME_FUNCTION(Runtime_SharedGC) { - SealHandleScope scope(isolate); - isolate->heap()->CollectSharedGarbage(GarbageCollectionReason::kTesting); - return ReadOnlyRoots(isolate).undefined_value(); -} - } // namespace internal } // namespace v8 diff --git a/src/runtime/runtime.h b/src/runtime/runtime.h index d78125c860..7259b66d3f 100644 --- a/src/runtime/runtime.h +++ b/src/runtime/runtime.h @@ -545,7 +545,6 @@ namespace internal { F(SetAllocationTimeout, -1 /* 2 || 3 */, 1) \ F(SetForceSlowPath, 1, 1) \ F(SetIteratorProtector, 0, 1) \ - F(SharedGC, 0, 1) \ F(SimulateNewspaceFull, 0, 1) \ F(StringIteratorProtector, 0, 1) \ F(SystemBreak, 0, 1) \ diff --git a/test/mjsunit/shared-memory/shared-string-in-code-object.js b/test/mjsunit/shared-memory/shared-string-in-code-object.js deleted file mode 100644 index 18b3051f55..0000000000 --- a/test/mjsunit/shared-memory/shared-string-in-code-object.js +++ /dev/null @@ -1,18 +0,0 @@ -// Copyright 2022 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. -// -// Flags: --shared-string-table --allow-natives-syntax --stress-compaction - -function foo() { return "foo"; } - -%PrepareFunctionForOptimization(foo); -let value = foo(); -assertTrue(%IsSharedString(value)); -%OptimizeFunctionOnNextCall(foo); -value = foo(); -assertTrue(%IsSharedString(value)); -%SharedGC(); -value = foo(); -assertTrue(%IsSharedString(value)); -assertEquals("foo", value);