Revert "[heap] Use PagedNewSpace when MinorMC is enabled"

This reverts commit 924be6956f.

Reason for revert: speculative revert for TSAN failures:
https://ci.chromium.org/ui/p/v8/builders/ci/V8%20Linux64%20TSAN%20-%20stress-incremental-marking/8726/overview

Original change's description:
> [heap] Use PagedNewSpace when MinorMC is enabled
>
> This CL also introduces/updates DCHECKs that some methods are never
> reached with MinorMC (they may still be reached by full GC when MinorMC
> is disabled).
>
> Bug: v8:12612
> Change-Id: I8afb8c964bc5c44225a92d0f8d9ac5a4c0ecef75
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3823130
> Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
> Commit-Queue: Omer Katz <omerkatz@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#82439}

Bug: v8:12612
Change-Id: I540f38fea17fbacffbd120dd050626d7d1ec32f3
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3827039
Auto-Submit: Adam Klein <adamk@chromium.org>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#82446}
This commit is contained in:
Adam Klein 2022-08-12 20:50:18 +00:00 committed by V8 LUCI CQ
parent eaa1c536aa
commit 6587dec02d
10 changed files with 36 additions and 106 deletions

View File

@ -63,7 +63,6 @@ T ForwardingAddress(T heap_obj) {
if (map_word.IsForwardingAddress()) {
return T::cast(map_word.ToForwardingAddress());
} else if (Heap::InFromPage(heap_obj)) {
DCHECK(!FLAG_minor_mc);
return T();
} else {
return heap_obj;
@ -411,9 +410,8 @@ void Heap::UpdateAllocationSite(Map map, HeapObject object,
DCHECK_NE(pretenuring_feedback, &global_pretenuring_feedback_);
#ifdef DEBUG
BasicMemoryChunk* chunk = BasicMemoryChunk::FromHeapObject(object);
DCHECK_IMPLIES(
chunk->IsToPage(),
FLAG_minor_mc || chunk->IsFlagSet(MemoryChunk::PAGE_NEW_NEW_PROMOTION));
DCHECK_IMPLIES(chunk->IsToPage(),
chunk->IsFlagSet(MemoryChunk::PAGE_NEW_NEW_PROMOTION));
DCHECK_IMPLIES(!chunk->InYoungGeneration(),
chunk->IsFlagSet(MemoryChunk::PAGE_NEW_OLD_PROMOTION));
#endif

View File

@ -930,9 +930,6 @@ void Heap::PrintRetainingPath(HeapObject target, RetainingPathOption option) {
void UpdateRetainersMapAfterScavenge(
std::unordered_map<HeapObject, HeapObject, Object::Hasher>* map) {
// This is only used for Scavenger.
DCHECK(!FLAG_minor_mc);
std::unordered_map<HeapObject, HeapObject, Object::Hasher> updated_map;
for (auto pair : *map) {
@ -960,7 +957,7 @@ void UpdateRetainersMapAfterScavenge(
void Heap::UpdateRetainersAfterScavenge() {
if (!incremental_marking()->IsMarking()) return;
// This is only used for Scavenger.
// This isn't supported for Minor MC.
DCHECK(!FLAG_minor_mc);
UpdateRetainersMapAfterScavenge(&retainer_);
@ -2601,6 +2598,11 @@ void Heap::MinorMarkCompact() {
CHECK_EQ(NOT_IN_GC, gc_state());
DCHECK(new_space());
if (FLAG_trace_incremental_marking && !incremental_marking()->IsStopped()) {
isolate()->PrintWithTimestamp(
"[IncrementalMarking] MinorMarkCompact during marking.\n");
}
PauseAllocationObserversScope pause_observers(this);
SetGCState(MINOR_MARK_COMPACT);
@ -2773,9 +2775,6 @@ void Heap::UpdateExternalString(String string, size_t old_payload,
String Heap::UpdateYoungReferenceInExternalStringTableEntry(Heap* heap,
FullObjectSlot p) {
// This is only used for Scavenger.
DCHECK(!FLAG_minor_mc);
PtrComprCageBase cage_base(heap->isolate());
HeapObject obj = HeapObject::cast(*p);
MapWord first_word = obj.map_word(cage_base, kRelaxedLoad);
@ -4782,11 +4781,6 @@ void Heap::VerifyCommittedPhysicalMemory() {
space = spaces.Next()) {
space->VerifyCommittedPhysicalMemory();
}
if (FLAG_minor_mc && new_space()) {
PagedNewSpace::From(new_space())
->paged_space()
->VerifyCommittedPhysicalMemory();
}
}
#endif // DEBUG
@ -5763,15 +5757,9 @@ void Heap::SetUpSpaces(LinearAllocationArea& new_allocation_info,
DCHECK_NOT_NULL(read_only_space_);
const bool has_young_gen = !FLAG_single_generation && !IsShared();
if (has_young_gen) {
if (FLAG_minor_mc) {
space_[NEW_SPACE] = new_space_ =
new PagedNewSpace(this, initial_semispace_size_, max_semi_space_size_,
new_allocation_info);
} else {
space_[NEW_SPACE] = new_space_ =
new SemiSpaceNewSpace(this, initial_semispace_size_,
max_semi_space_size_, new_allocation_info);
}
space_[NEW_SPACE] = new_space_ =
new SemiSpaceNewSpace(this, initial_semispace_size_,
max_semi_space_size_, new_allocation_info);
space_[NEW_LO_SPACE] = new_lo_space_ =
new NewLargeObjectSpace(this, NewSpaceCapacity());
}
@ -5930,7 +5918,6 @@ void Heap::NotifyOldGenerationExpansion(AllocationSpace space,
MemoryChunk* chunk) {
// Pages created during bootstrapping may contain immortal immovable objects.
if (!deserialization_complete()) {
DCHECK_NE(NEW_SPACE, chunk->owner()->identity());
chunk->MarkNeverEvacuate();
}
if (space == CODE_SPACE || space == CODE_LO_SPACE) {

View File

@ -378,7 +378,6 @@ void IncrementalMarking::UpdateMarkingWorklistAfterYoungGenGC() {
DCHECK(obj.IsHeapObject());
// Only pointers to from space have to be updated.
if (Heap::InFromPage(obj)) {
DCHECK(!FLAG_minor_mc);
MapWord map_word = obj.map_word(cage_base, kRelaxedLoad);
if (!map_word.IsForwardingAddress()) {
// There may be objects on the marking deque that do not exist
@ -402,11 +401,6 @@ void IncrementalMarking::UpdateMarkingWorklistAfterYoungGenGC() {
// new space.
DCHECK(Heap::IsLargeObject(obj) || Page::FromHeapObject(obj)->IsFlagSet(
Page::PAGE_NEW_NEW_PROMOTION));
DCHECK_IMPLIES(FLAG_minor_mc, !Page::FromHeapObject(obj)->IsFlagSet(
Page::PAGE_NEW_NEW_PROMOTION));
DCHECK_IMPLIES(
FLAG_minor_mc,
!obj.map_word(cage_base, kRelaxedLoad).IsForwardingAddress());
if (minor_marking_state->IsWhite(obj)) {
return false;
}

View File

@ -137,7 +137,7 @@ class MarkingVerifier : public ObjectVisitorWithCageBases, public RootVisitor {
void VerifyRoots();
void VerifyMarkingOnPage(const Page* page, Address start, Address end);
void VerifyMarking(NewSpace* new_space);
void VerifyMarking(PagedSpaceBase* paged_space);
void VerifyMarking(PagedSpace* paged_space);
void VerifyMarking(LargeObjectSpace* lo_space);
Heap* heap_;
@ -177,10 +177,6 @@ void MarkingVerifier::VerifyMarkingOnPage(const Page* page, Address start,
void MarkingVerifier::VerifyMarking(NewSpace* space) {
if (!space) return;
if (FLAG_minor_mc) {
VerifyMarking(PagedNewSpace::From(space)->paged_space());
return;
}
Address end = space->top();
// The bottom position is at the start of its page. Allows us to use
// page->area_start() as start of range on all pages.
@ -196,7 +192,7 @@ void MarkingVerifier::VerifyMarking(NewSpace* space) {
}
}
void MarkingVerifier::VerifyMarking(PagedSpaceBase* space) {
void MarkingVerifier::VerifyMarking(PagedSpace* space) {
for (Page* p : *space) {
VerifyMarkingOnPage(p, p->area_start(), p->area_end());
}
@ -348,7 +344,7 @@ class EvacuationVerifier : public ObjectVisitorWithCageBases,
void VerifyRoots();
void VerifyEvacuationOnPage(Address start, Address end);
void VerifyEvacuation(NewSpace* new_space);
void VerifyEvacuation(PagedSpaceBase* paged_space);
void VerifyEvacuation(PagedSpace* paged_space);
Heap* heap_;
};
@ -371,10 +367,6 @@ void EvacuationVerifier::VerifyEvacuationOnPage(Address start, Address end) {
void EvacuationVerifier::VerifyEvacuation(NewSpace* space) {
if (!space) return;
if (FLAG_minor_mc) {
VerifyEvacuation(PagedNewSpace::From(space)->paged_space());
return;
}
PageRange range(space->first_allocatable_address(), space->top());
for (auto it = range.begin(); it != range.end();) {
Page* page = *(it++);
@ -385,7 +377,7 @@ void EvacuationVerifier::VerifyEvacuation(NewSpace* space) {
}
}
void EvacuationVerifier::VerifyEvacuation(PagedSpaceBase* space) {
void EvacuationVerifier::VerifyEvacuation(PagedSpace* space) {
for (Page* p : *space) {
if (p->IsEvacuationCandidate()) continue;
if (p->Contains(space->top())) {
@ -653,7 +645,7 @@ void MarkCompactCollector::VerifyMarkbitsAreDirty(ReadOnlySpace* space) {
}
}
void MarkCompactCollector::VerifyMarkbitsAreClean(PagedSpaceBase* space) {
void MarkCompactCollector::VerifyMarkbitsAreClean(PagedSpace* space) {
for (Page* p : *space) {
CHECK(non_atomic_marking_state()->bitmap(p)->IsClean());
CHECK_EQ(0, non_atomic_marking_state()->live_bytes(p));
@ -662,10 +654,6 @@ void MarkCompactCollector::VerifyMarkbitsAreClean(PagedSpaceBase* space) {
void MarkCompactCollector::VerifyMarkbitsAreClean(NewSpace* space) {
if (!space) return;
if (FLAG_minor_mc) {
VerifyMarkbitsAreClean(PagedNewSpace::From(space)->paged_space());
return;
}
for (Page* p : PageRange(space->first_allocatable_address(), space->top())) {
CHECK(non_atomic_marking_state()->bitmap(p)->IsClean());
CHECK_EQ(0, non_atomic_marking_state()->live_bytes(p));
@ -1036,12 +1024,9 @@ void MarkCompactCollector::Prepare() {
heap()->new_lo_space()->ResetPendingObject();
}
NewSpace* new_space = heap()->new_space();
if (new_space) {
if (FLAG_minor_mc) {
PagedNewSpace::From(new_space)->paged_space()->PrepareForMarkCompact();
}
DCHECK_EQ(new_space->top(), new_space->original_top_acquire());
if (heap()->new_space()) {
DCHECK_EQ(heap()->new_space()->top(),
heap()->new_space()->original_top_acquire());
}
}
@ -1073,10 +1058,6 @@ void MarkCompactCollector::VerifyMarking() {
heap()->old_space()->VerifyLiveBytes();
if (heap()->map_space()) heap()->map_space()->VerifyLiveBytes();
heap()->code_space()->VerifyLiveBytes();
if (FLAG_minor_mc && heap()->new_space())
PagedNewSpace::From(heap()->new_space())
->paged_space()
->VerifyLiveBytes();
}
#endif
}
@ -1645,10 +1626,9 @@ class RecordMigratedSlotVisitor : public ObjectVisitorWithCageBases {
if (value->IsStrongOrWeak()) {
BasicMemoryChunk* p = BasicMemoryChunk::FromAddress(value.ptr());
if (p->InYoungGeneration()) {
DCHECK_IMPLIES(p->IsToPage(),
FLAG_minor_mc ||
p->IsFlagSet(Page::PAGE_NEW_NEW_PROMOTION) ||
p->IsLargePage());
DCHECK_IMPLIES(
p->IsToPage(),
p->IsFlagSet(Page::PAGE_NEW_NEW_PROMOTION) || p->IsLargePage());
MemoryChunk* chunk = MemoryChunk::FromHeapObject(host);
DCHECK(chunk->SweepingDone());
@ -1797,8 +1777,7 @@ class EvacuateVisitorBase : public HeapObjectVisitor {
AllocationResult allocation;
if (ShouldPromoteIntoSharedHeap(map)) {
DCHECK_EQ(target_space, OLD_SPACE);
// TODO(v8:12612): Implement promotion from new space to shared heap.
DCHECK_IMPLIES(!FLAG_minor_mc, Heap::InYoungGeneration(object));
DCHECK(Heap::InYoungGeneration(object));
DCHECK_NOT_NULL(shared_old_allocator_);
allocation = shared_old_allocator_->AllocateRaw(size, alignment,
AllocationOrigin::kGC);
@ -1901,9 +1880,6 @@ class EvacuateNewSpaceVisitor final : public EvacuateVisitorBase {
if (heap_->new_space()->ShouldBePromoted(object.address()) &&
TryEvacuateObject(OLD_SPACE, object, size, &target_object)) {
// Full GCs use AlwaysPromoteYoung::kYes above and MinorMC should never
// move objects.
DCHECK(!FLAG_minor_mc);
promoted_size_ += size;
return true;
}
@ -1923,7 +1899,7 @@ class EvacuateNewSpaceVisitor final : public EvacuateVisitorBase {
private:
inline bool TryEvacuateWithoutCopy(HeapObject object) {
DCHECK(!is_incremental_marking_);
if (is_incremental_marking_) return false;
Map map = object.map();
@ -1990,7 +1966,6 @@ class EvacuateNewSpacePageVisitor final : public HeapObjectVisitor {
static void Move(Page* page) {
switch (mode) {
case NEW_TO_NEW:
DCHECK(!FLAG_minor_mc);
page->heap()->new_space()->PromotePageInNewSpace(page);
break;
case NEW_TO_OLD: {
@ -3584,7 +3559,7 @@ static inline void UpdateSlot(PtrComprCageBase cage_base, TSlot slot,
"expected here");
MapWord map_word = heap_obj.map_word(cage_base, kRelaxedLoad);
if (map_word.IsForwardingAddress()) {
DCHECK_IMPLIES((!FLAG_minor_mc && !Heap::InFromPage(heap_obj)),
DCHECK_IMPLIES(!Heap::InFromPage(heap_obj),
MarkCompactCollector::IsOnEvacuationCandidate(heap_obj) ||
Page::FromHeapObject(heap_obj)->IsFlagSet(
Page::COMPACTION_WAS_ABORTED));
@ -5576,10 +5551,9 @@ class YoungGenerationRecordMigratedSlotVisitor final
if (value->IsStrongOrWeak()) {
BasicMemoryChunk* p = BasicMemoryChunk::FromAddress(value.ptr());
if (p->InYoungGeneration()) {
DCHECK_IMPLIES(p->IsToPage(),
FLAG_minor_mc ||
p->IsFlagSet(Page::PAGE_NEW_NEW_PROMOTION) ||
p->IsLargePage());
DCHECK_IMPLIES(
p->IsToPage(),
p->IsFlagSet(Page::PAGE_NEW_NEW_PROMOTION) || p->IsLargePage());
MemoryChunk* chunk = MemoryChunk::FromHeapObject(host);
DCHECK(chunk->SweepingDone());
RememberedSet<OLD_TO_NEW>::Insert<AccessMode::NON_ATOMIC>(chunk, slot);

View File

@ -504,7 +504,7 @@ class MarkCompactCollector final : public CollectorBase {
#ifdef VERIFY_HEAP
void VerifyMarkbitsAreClean();
void VerifyMarkbitsAreDirty(ReadOnlySpace* space);
void VerifyMarkbitsAreClean(PagedSpaceBase* space);
void VerifyMarkbitsAreClean(PagedSpace* space);
void VerifyMarkbitsAreClean(NewSpace* space);
void VerifyMarkbitsAreClean(LargeObjectSpace* space);
#endif

View File

@ -909,9 +909,10 @@ PagedSpaceForNewSpace::PagedSpaceForNewSpace(
target_capacity_(initial_capacity_) {
DCHECK_LE(initial_capacity_, max_capacity_);
if (!PreallocatePages()) {
V8::FatalProcessOutOfMemory(heap->isolate(), "New space setup");
}
// Adding entries to the free list requires having a map for free space. Not
// preallocating pages yet because the map may not be available yet when the
// space is initialized. `EnsureCurrentCapacity()` should be called after maps
// are allocated to preallocate pages.
}
Page* PagedSpaceForNewSpace::InitializePage(MemoryChunk* chunk) {
@ -996,9 +997,10 @@ bool PagedSpaceForNewSpace::AddFreshPage() {
return EnsureCurrentCapacity();
}
bool PagedSpaceForNewSpace::PreallocatePages() {
bool PagedSpaceForNewSpace::EnsureCurrentCapacity() {
// Verify that the free space map is already initialized. Otherwise, new free
// list entries will be invalid.
DCHECK_NE(0, heap()->isolate()->root(RootIndex::kFreeSpaceMap).ptr());
while (current_capacity_ < target_capacity_) {
if (!TryExpandImpl()) return false;
}
@ -1006,13 +1008,6 @@ bool PagedSpaceForNewSpace::PreallocatePages() {
return true;
}
bool PagedSpaceForNewSpace::EnsureCurrentCapacity() {
// Verify that the free space map is already initialized. Otherwise, new free
// list entries will be invalid.
DCHECK_NE(0, heap()->isolate()->root(RootIndex::kFreeSpaceMap).ptr());
return PreallocatePages();
}
void PagedSpaceForNewSpace::FreeLinearAllocationArea() {
size_t remaining_allocation_area_size = limit() - top();
DCHECK_GE(allocated_linear_areas_, remaining_allocation_area_size);

View File

@ -350,7 +350,6 @@ class NewSpace : NON_EXPORTED_BASE(public SpaceWithLinearArea) {
class V8_EXPORT_PRIVATE SemiSpaceNewSpace final : public NewSpace {
public:
static SemiSpaceNewSpace* From(NewSpace* space) {
DCHECK(!FLAG_minor_mc);
return static_cast<SemiSpaceNewSpace*>(space);
}
@ -629,8 +628,6 @@ class V8_EXPORT_PRIVATE PagedSpaceForNewSpace final : public PagedSpaceBase {
#endif // V8_ENABLE_INNER_POINTER_RESOLUTION_OSB
private:
bool PreallocatePages();
const size_t initial_capacity_;
const size_t max_capacity_;
size_t target_capacity_ = 0;
@ -645,7 +642,6 @@ class V8_EXPORT_PRIVATE PagedSpaceForNewSpace final : public PagedSpaceBase {
class V8_EXPORT_PRIVATE PagedNewSpace final : public NewSpace {
public:
static PagedNewSpace* From(NewSpace* space) {
DCHECK(FLAG_minor_mc);
return static_cast<PagedNewSpace*>(space);
}

View File

@ -184,7 +184,6 @@ void PagedSpaceBase::MergeCompactionSpace(CompactionSpace* other) {
base::MutexGuard guard(mutex());
DCHECK_NE(NEW_SPACE, identity());
DCHECK_NE(NEW_SPACE, other->identity());
DCHECK(identity() == other->identity());
// Unmerged fields:
@ -326,9 +325,6 @@ void PagedSpaceBase::RemovePage(Page* page) {
DCHECK_IMPLIES(identity() == NEW_SPACE, page->IsFlagSet(Page::TO_PAGE));
memory_chunk_list_.Remove(page);
UnlinkFreeListCategories(page);
if (identity() == NEW_SPACE) {
page->ReleaseFreeListCategories();
}
DecreaseAllocatedBytes(page->allocated_bytes(), page);
DecreaseCapacity(page->area_size());
AccountUncommitted(page->size());
@ -363,7 +359,6 @@ void PagedSpaceBase::ResetFreeList() {
free_list_->EvictFreeListItems(page);
}
DCHECK(free_list_->IsEmpty());
DCHECK_EQ(0, free_list_->Available());
}
void PagedSpaceBase::ShrinkImmortalImmovablePages() {
@ -509,7 +504,7 @@ void PagedSpaceBase::FreeLinearAllocationArea() {
AdvanceAllocationObservers();
if (identity() != NEW_SPACE && current_top != current_limit &&
if (current_top != current_limit &&
heap()->incremental_marking()->black_allocation()) {
Page::FromAddress(current_top)
->DestroyBlackArea(current_top, current_limit);

View File

@ -8,7 +8,6 @@
#include "src/execution/protectors.h"
#include "src/heap/factory.h"
#include "src/heap/heap-inl.h"
#include "src/heap/new-spaces.h"
#include "src/ic/handler-configuration.h"
#include "src/init/heap-symbols.h"
#include "src/init/setup-isolate.h"
@ -75,12 +74,6 @@ bool SetupIsolateDelegate::SetupHeapInternal(Heap* heap) {
bool Heap::CreateHeapObjects() {
// Create initial maps.
if (!CreateInitialMaps()) return false;
if (FLAG_minor_mc && new_space()) {
PagedNewSpace::From(new_space())
->paged_space()
->free_list()
->RepairLists(this);
}
CreateApiObjects();
// Create initial objects

View File

@ -930,8 +930,6 @@ void StringForwardingTable::Block::UpdateAfterEvacuation(Isolate* isolate) {
void StringForwardingTable::Block::UpdateAfterEvacuation(Isolate* isolate,
int up_to_index) {
// This is only used for Scavenger.
DCHECK(!FLAG_minor_mc);
DCHECK(FLAG_always_use_string_forwarding_table);
for (int index = 0; index < up_to_index; ++index) {
Object original = Get(isolate, IndexOfOriginalString(index));