Skip to content

Commit f81e87e

Browse files
omerktzV8 LUCI CQ
authored andcommitted
[heap] Implement object pinning in Scavenger
Object reachable from stack are pinned in place and not moved. Pages containing pinned objects are quarantined and not used for further allocations until they become empty and are freed. See design doc for more details: https://docs.google.com/document/d/1UHe6Ac60lnOshlgfP4XG6GXeh7szqbxPl4uu1x4i1EY/edit?tab=t.0#heading=h.n1atlriavj6v Bug: 379788114 Change-Id: I9acac6a26a16e617a93c967e134e7569a46acbc9 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6038834 Reviewed-by: Dominik Inführ <[email protected]> Reviewed-by: Michael Lippautz <[email protected]> Commit-Queue: Omer Katz <[email protected]> Cr-Commit-Position: refs/heads/main@{#97535}
1 parent 9094fb5 commit f81e87e

34 files changed

Lines changed: 687 additions & 170 deletions

BUILD.bazel

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1635,6 +1635,8 @@ filegroup(
16351635
"src/heap/combined-heap.h",
16361636
"src/heap/concurrent-marking.cc",
16371637
"src/heap/concurrent-marking.h",
1638+
"src/heap/conservative-stack-visitor.cc",
1639+
"src/heap/conservative-stack-visitor.h",
16381640
"src/heap/cppgc-js/cpp-heap.cc",
16391641
"src/heap/cppgc-js/cpp-heap.h",
16401642
"src/heap/cppgc-js/cpp-marking-state.h",
@@ -1936,6 +1938,7 @@ filegroup(
19361938
"src/objects/call-site-info.h",
19371939
"src/objects/call-site-info-inl.h",
19381940
"src/objects/casting.h",
1941+
"src/objects/casting-inl.h",
19391942
"src/objects/cell.h",
19401943
"src/objects/cell-inl.h",
19411944
"src/objects/code.cc",

BUILD.gn

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1894,6 +1894,7 @@ if (v8_postmortem_support) {
18941894
"src/objects/instruction-stream.h",
18951895
"src/objects/instruction-stream-inl.h",
18961896
"src/objects/casting.h",
1897+
"src/objects/casting-inl.h",
18971898
"src/objects/code.h",
18981899
"src/objects/code-inl.h",
18991900
"src/objects/data-handler.h",
@@ -3789,6 +3790,7 @@ v8_header_set("v8_internal_headers") {
37893790
"src/heap/collection-barrier.h",
37903791
"src/heap/combined-heap.h",
37913792
"src/heap/concurrent-marking.h",
3793+
"src/heap/conservative-stack-visitor.h",
37923794
"src/heap/cppgc-js/cpp-heap.h",
37933795
"src/heap/cppgc-js/cpp-marking-state-inl.h",
37943796
"src/heap/cppgc-js/cpp-marking-state.h",
@@ -4609,10 +4611,6 @@ v8_header_set("v8_internal_headers") {
46094611
sources += [ "src/execution/pointer-authentication-dummy.h" ]
46104612
}
46114613

4612-
if (v8_enable_conservative_stack_scanning) {
4613-
sources += [ "src/heap/conservative-stack-visitor.h" ]
4614-
}
4615-
46164614
if (v8_enable_wasm_gdb_remote_debugging) {
46174615
sources += [
46184616
"src/debug/wasm/gdb-server/gdb-remote-util.h",
@@ -5538,6 +5536,7 @@ v8_source_set("v8_base_without_compiler") {
55385536
"src/heap/collection-barrier.cc",
55395537
"src/heap/combined-heap.cc",
55405538
"src/heap/concurrent-marking.cc",
5539+
"src/heap/conservative-stack-visitor.cc",
55415540
"src/heap/cppgc-js/cpp-heap.cc",
55425541
"src/heap/cppgc-js/cpp-snapshot.cc",
55435542
"src/heap/cppgc-js/cross-heap-remembered-set.cc",
@@ -5999,10 +5998,6 @@ v8_source_set("v8_base_without_compiler") {
59995998
}
60005999
}
60016000

6002-
if (v8_enable_conservative_stack_scanning) {
6003-
sources += [ "src/heap/conservative-stack-visitor.cc" ]
6004-
}
6005-
60066001
if (v8_enable_wasm_gdb_remote_debugging) {
60076002
sources += [
60086003
"src/debug/wasm/gdb-server/gdb-remote-util.cc",

src/execution/frames.cc

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,9 @@
2626
#include "src/execution/vm-state-inl.h"
2727
#include "src/ic/ic-stats.h"
2828
#include "src/logging/counters.h"
29+
#include "src/objects/casting-inl.h"
2930
#include "src/objects/code.h"
31+
#include "src/objects/instance-type-checker.h"
3032
#include "src/objects/slots.h"
3133
#include "src/objects/smi.h"
3234
#include "src/objects/visitors.h"
@@ -88,10 +90,11 @@ class StackHandlerIterator {
8890
// For CWasmEntry frames, the handler was registered by the last C++
8991
// frame (Execution::CallWasm), so even though its address is already
9092
// beyond the limit, we know we always want to unwind one handler.
91-
if (frame->is_c_wasm_entry()) handler_ = handler_->next();
93+
if (frame->is_c_wasm_entry()) {
94+
handler_ = handler_->next();
9295
#if V8_ENABLE_DRUMBRAKE
9396
// Do the same for GenericWasmToJsInterpreterWrapper frames.
94-
else if (v8_flags.wasm_jitless && frame->is_wasm_to_js()) {
97+
} else if (v8_flags.wasm_jitless && frame->is_wasm_to_js()) {
9598
handler_ = handler_->next();
9699
#ifdef USE_SIMULATOR
97100
// If we are running in the simulator, the handler_ address here will
@@ -100,8 +103,8 @@ class StackHandlerIterator {
100103
// any handler.
101104
limit_ = 0;
102105
#endif // USE_SIMULATOR
103-
}
104106
#endif // V8_ENABLE_DRUMBRAKE
107+
}
105108
#else
106109
// Make sure the handler has already been unwound to this frame.
107110
DCHECK_LE(frame->sp(), AddressOf(handler));
@@ -189,7 +192,8 @@ void StackFrameIterator::Advance() {
189192
// does not have a caller fp.
190193
auto parent = continuation()->parent();
191194
CHECK(!IsUndefined(parent));
192-
set_continuation(Cast<WasmContinuationObject>(parent));
195+
set_continuation(
196+
GCSafeCast<WasmContinuationObject>(parent, isolate_->heap()));
193197
wasm_stack_ = reinterpret_cast<wasm::StackMemory*>(continuation()->stack());
194198
CHECK_EQ(wasm_stack_->jmpbuf()->state, wasm::JumpBuffer::Inactive);
195199
StackSwitchFrame::GetStateForJumpBuffer(wasm_stack_->jmpbuf(), &state);
@@ -282,7 +286,8 @@ void StackFrameIterator::Reset(ThreadLocalTop* top) {
282286
#if V8_ENABLE_WEBASSEMBLY
283287
auto active_continuation = isolate_->root(RootIndex::kActiveContinuation);
284288
if (!IsUndefined(active_continuation, isolate_)) {
285-
auto continuation = Cast<WasmContinuationObject>(active_continuation);
289+
auto continuation = GCSafeCast<WasmContinuationObject>(active_continuation,
290+
isolate_->heap());
286291
if (!first_stack_only_) {
287292
set_continuation(continuation);
288293
}
@@ -839,7 +844,7 @@ void StackFrame::IteratePc(RootVisitor* v, Address* constant_pool_address,
839844
DCHECK(!isolate()->InFastCCall());
840845

841846
Tagged<InstructionStream> istream =
842-
UncheckedCast<InstructionStream>(visited_istream);
847+
GCSafeCast<InstructionStream>(visited_istream, isolate()->heap());
843848
const Address new_pc = istream->instruction_start() + pc_offset_from_start;
844849
// TODO(v8:10026): avoid replacing a signed pointer.
845850
PointerAuthentication::ReplacePC(pc_address(), new_pc, kSystemPointerSize);
@@ -1590,13 +1595,13 @@ void VisitSpillSlot(Isolate* isolate, RootVisitor* v,
15901595
? map_word.ToForwardingAddress(raw)
15911596
: raw;
15921597
bool is_self_forwarded =
1593-
forwarded->map_word(cage_base, kRelaxedLoad) ==
1594-
MapWord::FromForwardingAddress(forwarded, forwarded);
1598+
HeapLayout::IsSelfForwarded(forwarded, cage_base);
15951599
if (is_self_forwarded) {
15961600
// The object might be in a self-forwarding state if it's located
15971601
// in new large object space. GC will fix this at a later stage.
15981602
CHECK(
1599-
MemoryChunk::FromHeapObject(forwarded)->InNewLargeObjectSpace());
1603+
MemoryChunk::FromHeapObject(forwarded)->InNewLargeObjectSpace() ||
1604+
MemoryChunk::FromHeapObject(forwarded)->IsQuarantined());
16001605
} else {
16011606
Tagged<HeapObject> forwarded_map = forwarded->map(cage_base);
16021607
// The map might be forwarded as well.

src/flags/flag-definitions.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -475,6 +475,16 @@ DEFINE_BOOL_READONLY(direct_handle, V8_ENABLE_DIRECT_HANDLE_BOOL,
475475
// break the correctness of the GC.
476476
DEFINE_NEG_NEG_IMPLICATION(conservative_stack_scanning, direct_handle)
477477

478+
DEFINE_EXPERIMENTAL_FEATURE(scavenger_pinning_objects,
479+
"Objects reachable from the native stack during "
480+
"scavenge will be pinned and "
481+
"won't move.")
482+
DEFINE_IMPLICATION(scavenger_pinning_objects, separate_gc_phases)
483+
DEFINE_BOOL(stress_scavenger_pinning_objects, false,
484+
"Treat some precise refernces as conservative references to stress "
485+
"test object pinning in Scavenger")
486+
DEFINE_IMPLICATION(stress_scavenger_pinning_objects, scavenger_pinning_objects)
487+
478488
#ifdef V8_ENABLE_LOCAL_OFF_STACK_CHECK
479489
#define V8_ENABLE_LOCAL_OFF_STACK_CHECK_BOOL true
480490
#else

src/heap/conservative-stack-visitor.cc

Lines changed: 48 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,9 @@
44

55
#include "src/heap/conservative-stack-visitor.h"
66

7+
#include "src/common/globals.h"
78
#include "src/execution/isolate-inl.h"
9+
#include "src/heap/heap-layout.h"
810
#include "src/heap/marking-inl.h"
911
#include "src/heap/memory-chunk-metadata.h"
1012
#include "src/heap/memory-chunk.h"
@@ -17,10 +19,6 @@
1719
namespace v8 {
1820
namespace internal {
1921

20-
ConservativeStackVisitor::ConservativeStackVisitor(Isolate* isolate,
21-
RootVisitor* delegate)
22-
: ConservativeStackVisitor(isolate, delegate, delegate->collector()) {}
23-
2422
ConservativeStackVisitor::ConservativeStackVisitor(Isolate* isolate,
2523
RootVisitor* delegate,
2624
GarbageCollector collector)
@@ -40,12 +38,18 @@ ConservativeStackVisitor::ConservativeStackVisitor(Isolate* isolate,
4038
#ifdef V8_COMPRESS_POINTERS
4139
bool ConservativeStackVisitor::IsInterestingCage(
4240
PtrComprCageBase cage_base) const {
43-
if (cage_base == cage_base_) return true;
41+
if (cage_base == cage_base_) {
42+
return true;
43+
}
4444
#ifdef V8_EXTERNAL_CODE_SPACE
45-
if (cage_base == code_cage_base_) return true;
45+
if (cage_base == code_cage_base_) {
46+
return true;
47+
}
4648
#endif
4749
#ifdef V8_ENABLE_SANDBOX
48-
if (cage_base == trusted_cage_base_) return true;
50+
if (cage_base == trusted_cage_base_) {
51+
return true;
52+
}
4953
#endif
5054
return false;
5155
}
@@ -60,9 +64,35 @@ Address ConservativeStackVisitor::FindBasePtr(
6064
// heap. Bail out if it is not.
6165
const MemoryChunk* chunk =
6266
allocator_->LookupChunkContainingAddress(maybe_inner_ptr);
63-
if (chunk == nullptr) return kNullAddress;
67+
if (chunk == nullptr) {
68+
return kNullAddress;
69+
}
6470
const MemoryChunkMetadata* chunk_metadata = chunk->Metadata();
6571
DCHECK(chunk_metadata->Contains(maybe_inner_ptr));
72+
73+
// If it is not in the young generation and we're only interested in young
74+
// generation pointers, we must ignore it.
75+
if (Heap::IsYoungGenerationCollector(collector_)) {
76+
const bool is_old = v8_flags.sticky_mark_bits
77+
? chunk->IsFlagSet(MemoryChunk::CONTAINS_ONLY_OLD)
78+
: !chunk->InYoungGeneration();
79+
if (is_old) return kNullAddress;
80+
81+
// Scavenger already swapped the semi spaces, so all real objects should
82+
// be found in to space.
83+
if (collector_ == GarbageCollector::SCAVENGER ? chunk->IsToPage()
84+
: chunk->IsFromPage()) {
85+
return kNullAddress;
86+
}
87+
} else {
88+
// If it is in the young generation "from" semispace, it is not used and
89+
// we must ignore it, as its markbits may not be clean.
90+
if (chunk->IsFromPage()) {
91+
DCHECK(!v8_flags.sticky_mark_bits);
92+
return kNullAddress;
93+
}
94+
}
95+
6696
// If it is contained in a large page, we want to mark the only object on it.
6797
if (chunk->IsLargePage()) {
6898
// This could be simplified if we could guarantee that there are no free
@@ -71,25 +101,12 @@ Address ConservativeStackVisitor::FindBasePtr(
71101
static_cast<const LargePageMetadata*>(chunk_metadata)->GetObject());
72102
return IsFreeSpaceOrFiller(obj, cage_base) ? kNullAddress : obj.address();
73103
}
104+
74105
// Otherwise, we have a pointer inside a normal page.
75106
const PageMetadata* page = static_cast<const PageMetadata*>(chunk_metadata);
76-
// If it is not in the young generation and we're only interested in young
77-
// generation pointers, we must ignore it.
78-
if (v8_flags.sticky_mark_bits) {
79-
if (Heap::IsYoungGenerationCollector(collector_) &&
80-
chunk->IsFlagSet(MemoryChunk::CONTAINS_ONLY_OLD))
81-
return kNullAddress;
82-
} else {
83-
if (Heap::IsYoungGenerationCollector(collector_) &&
84-
!chunk->InYoungGeneration())
85-
return kNullAddress;
86-
87-
// If it is in the young generation "from" semispace, it is not used and we
88-
// must ignore it, as its markbits may not be clean.
89-
if (chunk->IsFromPage()) return kNullAddress;
90-
}
91-
92107
// Try to find the address of a previous valid object on this page.
108+
// TODO(379788114): Create a temporary bitmap for repeated visits to the same
109+
// page during CSS from Scavenger.
93110
Address base_ptr =
94111
MarkingBitmap::FindPreviousValidObject(page, maybe_inner_ptr);
95112
// Iterate through the objects in the page forwards, until we find the object
@@ -99,8 +116,9 @@ Address ConservativeStackVisitor::FindBasePtr(
99116
Tagged<HeapObject> obj(HeapObject::FromAddress(base_ptr));
100117
const int size = obj->Size(cage_base);
101118
DCHECK_LT(0, size);
102-
if (maybe_inner_ptr < base_ptr + size)
119+
if (maybe_inner_ptr < base_ptr + size) {
103120
return IsFreeSpaceOrFiller(obj, cage_base) ? kNullAddress : base_ptr;
121+
}
104122
base_ptr += size;
105123
DCHECK_LT(base_ptr, page->area_end());
106124
}
@@ -135,12 +153,11 @@ void ConservativeStackVisitor::VisitConservativelyIfPointer(Address address) {
135153
if (V8HeapCompressionScheme::GetPtrComprCageBaseAddress(address) ==
136154
cage_base_.address()) {
137155
VisitConservativelyIfPointer(address, cage_base_);
138-
}
139156
#ifdef V8_EXTERNAL_CODE_SPACE
140-
else if (code_address_region_.contains(address)) {
157+
} else if (code_address_region_.contains(address)) {
141158
VisitConservativelyIfPointer(address, code_cage_base_);
142-
}
143159
#endif // V8_EXTERNAL_CODE_SPACE
160+
}
144161
#else // !V8_COMPRESS_POINTERS
145162
VisitConservativelyIfPointer(address, cage_base_);
146163
#endif // V8_COMPRESS_POINTERS
@@ -156,7 +173,9 @@ void ConservativeStackVisitor::VisitConservativelyIfPointer(
156173
}
157174
// Proceed with inner-pointer resolution.
158175
Address base_ptr = FindBasePtr(address, cage_base);
159-
if (base_ptr == kNullAddress) return;
176+
if (base_ptr == kNullAddress) {
177+
return;
178+
}
160179
Tagged<HeapObject> obj = HeapObject::FromAddress(base_ptr);
161180
Tagged<Object> root = obj;
162181
DCHECK_NOT_NULL(delegate_);

src/heap/conservative-stack-visitor.h

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,8 @@ class RootVisitor;
1919
class V8_EXPORT_PRIVATE ConservativeStackVisitor
2020
: public ::heap::base::StackVisitor {
2121
public:
22-
ConservativeStackVisitor(Isolate* isolate, RootVisitor* delegate);
22+
ConservativeStackVisitor(Isolate* isolate, RootVisitor* delegate,
23+
GarbageCollector collector);
2324

2425
void VisitPointer(const void* pointer) final;
2526

@@ -39,9 +40,6 @@ class V8_EXPORT_PRIVATE ConservativeStackVisitor
3940
}
4041

4142
private:
42-
ConservativeStackVisitor(Isolate* isolate, RootVisitor* delegate,
43-
GarbageCollector collector);
44-
4543
void VisitConservativelyIfPointer(Address address);
4644
void VisitConservativelyIfPointer(Address address,
4745
PtrComprCageBase cage_base);

src/heap/gc-tracer.cc

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -859,6 +859,8 @@ void GCTracer::PrintNVP() const {
859859
"holes_size_after=%zu "
860860
"allocated=%zu "
861861
"promoted=%zu "
862+
"quarantined_size=%zu "
863+
"quarantined_pages=%zu "
862864
"new_space_survived=%zu "
863865
"nodes_died_in_new=%d "
864866
"nodes_copied_in_new=%d "
@@ -897,6 +899,8 @@ void GCTracer::PrintNVP() const {
897899
current_.start_object_size, current_.end_object_size,
898900
current_.start_holes_size, current_.end_holes_size,
899901
allocated_since_last_gc, heap_->promoted_objects_size(),
902+
heap_->semi_space_new_space()->QuarantinedSize(),
903+
heap_->semi_space_new_space()->QuarantinedPageCount(),
900904
heap_->new_space_surviving_object_size(),
901905
heap_->nodes_died_in_new_space_, heap_->nodes_copied_in_new_space_,
902906
heap_->nodes_promoted_, heap_->promotion_ratio_,

src/heap/heap-layout.cc

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
#include "src/heap/heap-layout-inl.h"
66
#include "src/heap/marking-inl.h"
77
#include "src/heap/memory-chunk.h"
8+
#include "src/objects/objects-inl.h"
89

910
namespace v8::internal {
1011

@@ -37,4 +38,16 @@ void HeapLayout::CheckYoungGenerationConsistency(const MemoryChunk* chunk) {
3738
#endif // DEBUG
3839
}
3940

41+
// static
42+
bool HeapLayout::IsSelfForwarded(Tagged<HeapObject> object) {
43+
return IsSelfForwarded(object, GetPtrComprCageBase(object));
44+
}
45+
46+
// static
47+
bool HeapLayout::IsSelfForwarded(Tagged<HeapObject> object,
48+
PtrComprCageBase cage_base) {
49+
return object->map_word(cage_base, kRelaxedLoad) ==
50+
MapWord::FromForwardingAddress(object, object);
51+
}
52+
4053
} // namespace v8::internal

src/heap/heap-layout.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,12 @@ class HeapLayout final : public AllStatic {
5454
// serialization.
5555
static V8_INLINE bool IsOwnedByAnyHeap(Tagged<HeapObject> object);
5656

57+
// Returns whether the map word of `object` is a self forwarding address.
58+
// This represents pinned objects and live large objects in Scavenger.
59+
static bool IsSelfForwarded(Tagged<HeapObject> object);
60+
static bool IsSelfForwarded(Tagged<HeapObject> object,
61+
PtrComprCageBase cage_base);
62+
5763
private:
5864
V8_EXPORT static bool InYoungGenerationForStickyMarkbits(
5965
const MemoryChunk* chunk, Tagged<HeapObject> object);

src/heap/heap-verifier.cc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -412,6 +412,7 @@ void HeapVerification::VerifyPage(const MemoryChunkMetadata* chunk_metadata) {
412412
CHECK(!current_chunk_.has_value());
413413
CHECK(!chunk->IsFlagSet(MemoryChunk::PAGE_NEW_OLD_PROMOTION));
414414
CHECK(!chunk->IsFlagSet(MemoryChunk::FROM_PAGE));
415+
CHECK(!chunk->IsQuarantined());
415416
if (chunk->InReadOnlySpace()) {
416417
CHECK_NULL(chunk_metadata->owner());
417418
} else {
@@ -629,7 +630,7 @@ class OldToNewSlotVerifyingVisitor : public SlotVerifyingVisitor {
629630
Tagged<Object> k = *key;
630631
if (!HeapLayout::InYoungGeneration(host) &&
631632
HeapLayout::InYoungGeneration(k)) {
632-
Tagged<EphemeronHashTable> table = Cast<EphemeronHashTable>(host);
633+
Tagged<EphemeronHashTable> table = i::Cast<EphemeronHashTable>(host);
633634
auto it = ephemeron_remembered_set_->find(table);
634635
CHECK(it != ephemeron_remembered_set_->end());
635636
int slot_index =

0 commit comments

Comments
 (0)