From a5b3c3df9b22f441ce61b70c3a2fcf652f9e3f9e Mon Sep 17 00:00:00 2001 From: Omer Katz Date: Thu, 8 Sep 2022 11:26:28 +0200 Subject: [PATCH] [heap] Minor MinorMC fixes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix broken DCHECK: When using MinorMC, new space is a paged space and only uses the TO_PAGE page flag. New large object space however still uses both TO_PAGE and FROM_PAGE page flags. With MinorMC it still possible to find reference to FROM_PAGEs, but those pages have to be large pages. Fix broken test: MinorMC may only free empty pages when shrinking. Therefore, shrink may actually not change the space capacity at all (e.g. when all pages have live objects on them). More specifically, the capacity is not guaranteed to be half the previous capacity. Bug: v8:12612 Change-Id: Ib0edcafd758828f821f82bc8c796c205f162809c Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3879493 Reviewed-by: Dominik Inführ Commit-Queue: Omer Katz Auto-Submit: Omer Katz Cr-Commit-Position: refs/heads/main@{#83061} --- src/heap/mark-compact.cc | 3 ++- test/unittests/heap/heap-unittest.cc | 8 +++++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/heap/mark-compact.cc b/src/heap/mark-compact.cc index 4aa8aa37ec..dcf5b14bc6 100644 --- a/src/heap/mark-compact.cc +++ b/src/heap/mark-compact.cc @@ -4946,7 +4946,8 @@ class RememberedSetUpdatingItem : public UpdatingItem { return REMOVE_SLOT; } if (Heap::InFromPage(heap_object)) { - DCHECK(!v8_flags.minor_mc); + DCHECK_IMPLIES(v8_flags.minor_mc, + Page::FromHeapObject(heap_object)->IsLargePage()); MapWord map_word = heap_object.map_word(kRelaxedLoad); if (map_word.IsForwardingAddress()) { HeapObjectReference::Update(THeapObjectSlot(slot), diff --git a/test/unittests/heap/heap-unittest.cc b/test/unittests/heap/heap-unittest.cc index 0b48641910..7db368c292 100644 --- a/test/unittests/heap/heap-unittest.cc +++ b/test/unittests/heap/heap-unittest.cc @@ -228,7 +228,13 @@ TEST_F(HeapTest, GrowAndShrinkNewSpace) { old_capacity = new_space->TotalCapacity(); new_space->Shrink(); new_capacity = new_space->TotalCapacity(); - CHECK_EQ(old_capacity, 2 * new_capacity); + if (v8_flags.minor_mc) { + // Shrinking may not be able to remove any pages if all contain live + // objects. + CHECK_GE(old_capacity, new_capacity); + } else { + CHECK_EQ(old_capacity, 2 * new_capacity); + } // Consecutive shrinking should not affect space capacity. old_capacity = new_space->TotalCapacity();