Skip to content

Commit

Permalink
Revert "cppgc: Get AgeTableTest compiling when young generation is en…
Browse files Browse the repository at this point in the history
…abled"

This reverts commit d13ce73.

Reason for revert: blocks V8 roll due to blink_heap_unittests failures:
https://crrev.com/c/4562338

Original change's description:
> cppgc: Get AgeTableTest compiling when young generation is enabled
>
> age-table-unittest.cc is gated behind cppgc_enable_young_generation in
> test/unittests/BUILD.gn. Root BUILD.gn implemented a dependency where
> cppgc_enable_young_generation was set to true when the caged heap is
> enabled, but that dependency was not propagating to the tests. This CL
> moves the caged heap flag and the dependency to v8.gni so that it's
> consistent throughout the source tree.
>
> That change exposed a compile error in age-table-unittest.cc due to the
> non-stack-allocated test fixture having a member of stack-allocated type
> subtle::DisallowGarbageCollectionScope. The fix is for each test to
> declare its own DisallowGarbageCollectionScope instead.
>
> Bug: chromium:1029379, chromium:1434388
> Change-Id: If3d4f8f124585f4c74637c6cf8073cdbe6a6b5a9
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4548686
> Commit-Queue: Kevin Babbitt <kbabbitt@microsoft.com>
> Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#87808}

Bug: chromium:1029379, chromium:1434388
Change-Id: Ie64885187bdd09c193eb0b12ffbffc1b0cbd8992
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4562958
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Adam Klein <adamk@chromium.org>
Cr-Commit-Position: refs/heads/main@{#87844}
  • Loading branch information
ajklein authored and V8 LUCI CQ committed May 24, 2023
1 parent ce53c9c commit d097bd3
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 19 deletions.
8 changes: 8 additions & 0 deletions BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,11 @@ declare_args() {
v8_current_cpu == "arm64" &&
(target_is_simulator || arm_control_flow_integrity != "none")

# Enable heap reservation of size 4GB. Only possible for 64bit archs.
cppgc_enable_caged_heap =
v8_current_cpu == "x64" || v8_current_cpu == "arm64" ||
v8_current_cpu == "loong64" || v8_current_cpu == "riscv64"

# Enables additional heap verification phases and checks.
cppgc_enable_verify_heap = ""

Expand Down Expand Up @@ -889,6 +894,9 @@ if (cppgc_enable_object_names) {
if (cppgc_enable_caged_heap) {
enabled_external_cppgc_defines += [ "CPPGC_CAGED_HEAP" ]

# Always enable young generation compile time flag if caged heap is enabled.
cppgc_enable_young_generation = true

# Pointer compression regresses binary size on Fuchsia by about 300K.
# However, the change improves Oilpan memory by 15-20% (2-4% of PMF),
# which is beneficial for memory-impoverished platforms.
Expand Down
10 changes: 0 additions & 10 deletions gni/v8.gni
Original file line number Diff line number Diff line change
Expand Up @@ -101,11 +101,6 @@ declare_args() {
# Enable object names in cppgc for debug purposes.
cppgc_enable_object_names = false

# Enable heap reservation of size 4GB. Only possible for 64bit archs.
cppgc_enable_caged_heap =
v8_current_cpu == "x64" || v8_current_cpu == "arm64" ||
v8_current_cpu == "loong64" || v8_current_cpu == "riscv64"

# Enable young generation in cppgc.
cppgc_enable_young_generation = false

Expand Down Expand Up @@ -197,11 +192,6 @@ v8_path_prefix = get_path_info("../", "abspath")

v8_inspector_js_protocol = v8_path_prefix + "/include/js_protocol.pdl"

if (cppgc_enable_caged_heap) {
# Always enable young generation compile time flag if caged heap is enabled.
cppgc_enable_young_generation = true
}

###############################################################################
# Templates
#
Expand Down
13 changes: 4 additions & 9 deletions test/unittests/heap/cppgc/age-table-unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@ class AgeTableTest : public testing::TestWithHeap {
using AdjacentCardsPolicy = AgeTable::AdjacentCardsPolicy;
static constexpr auto kCardSizeInBytes = AgeTable::kCardSizeInBytes;

AgeTableTest() : age_table_(CagedHeapLocalData::Get().age_table) {}
AgeTableTest()
: disallow_gc_(GetHeapHandle()),
age_table_(CagedHeapLocalData::Get().age_table) {}

~AgeTableTest() override { age_table_.ResetForTesting(); }

Expand Down Expand Up @@ -74,14 +76,14 @@ class AgeTableTest : public testing::TestWithHeap {
}

private:
subtle::DisallowGarbageCollectionScope disallow_gc_;
std::vector<std::unique_ptr<BasePage, void (*)(BasePage*)>> allocated_pages_;
AgeTable& age_table_;
};

} // namespace

TEST_F(AgeTableTest, SetAgeForNormalPage) {
subtle::DisallowGarbageCollectionScope disallow_gc(*Heap::From(GetHeap()));
auto* page = AllocateNormalPage();
// By default, everything is old.
AssertAgeForAddressRange(page->PayloadStart(), page->PayloadEnd(), Age::kOld);
Expand All @@ -94,7 +96,6 @@ TEST_F(AgeTableTest, SetAgeForNormalPage) {
}

TEST_F(AgeTableTest, SetAgeForLargePage) {
subtle::DisallowGarbageCollectionScope disallow_gc(*Heap::From(GetHeap()));
auto* page = AllocateLargePage();
// By default, everything is old.
AssertAgeForAddressRange(page->PayloadStart(), page->PayloadEnd(), Age::kOld);
Expand All @@ -107,7 +108,6 @@ TEST_F(AgeTableTest, SetAgeForLargePage) {
}

TEST_F(AgeTableTest, SetAgeForSingleCardWithUnalignedAddresses) {
subtle::DisallowGarbageCollectionScope disallow_gc(*Heap::From(GetHeap()));
auto* page = AllocateNormalPage();
Address object_begin = reinterpret_cast<Address>(
RoundUp(reinterpret_cast<uintptr_t>(page->PayloadStart()),
Expand All @@ -128,7 +128,6 @@ TEST_F(AgeTableTest, SetAgeForSingleCardWithUnalignedAddresses) {
}

TEST_F(AgeTableTest, SetAgeForSingleCardWithAlignedAddresses) {
subtle::DisallowGarbageCollectionScope disallow_gc(*Heap::From(GetHeap()));
auto* page = AllocateNormalPage();
Address object_begin = reinterpret_cast<Address>(RoundUp(
reinterpret_cast<uintptr_t>(page->PayloadStart()), kCardSizeInBytes));
Expand All @@ -145,7 +144,6 @@ TEST_F(AgeTableTest, SetAgeForSingleCardWithAlignedAddresses) {
}

TEST_F(AgeTableTest, SetAgeForSingleCardWithAlignedBeginButUnalignedEnd) {
subtle::DisallowGarbageCollectionScope disallow_gc(*Heap::From(GetHeap()));
auto* page = AllocateNormalPage();
Address object_begin = reinterpret_cast<Address>(RoundUp(
reinterpret_cast<uintptr_t>(page->PayloadStart()), kCardSizeInBytes));
Expand All @@ -162,7 +160,6 @@ TEST_F(AgeTableTest, SetAgeForSingleCardWithAlignedBeginButUnalignedEnd) {
}

TEST_F(AgeTableTest, SetAgeForMultipleCardsWithUnalignedAddresses) {
subtle::DisallowGarbageCollectionScope disallow_gc(*Heap::From(GetHeap()));
static constexpr size_t kNumberOfCards = 4;
auto* page = AllocateNormalPage();
Address object_begin = reinterpret_cast<Address>(
Expand All @@ -182,7 +179,6 @@ TEST_F(AgeTableTest, SetAgeForMultipleCardsWithUnalignedAddresses) {
}

TEST_F(AgeTableTest, SetAgeForMultipleCardsConsiderAdjacentCards) {
subtle::DisallowGarbageCollectionScope disallow_gc(*Heap::From(GetHeap()));
static constexpr size_t kNumberOfCards = 4;
auto* page = AllocateNormalPage();
Address object_begin = reinterpret_cast<Address>(
Expand All @@ -204,7 +200,6 @@ TEST_F(AgeTableTest, SetAgeForMultipleCardsConsiderAdjacentCards) {
}

TEST_F(AgeTableTest, MarkAllCardsAsYoung) {
subtle::DisallowGarbageCollectionScope disallow_gc(*Heap::From(GetHeap()));
uint8_t* heap_start = reinterpret_cast<uint8_t*>(CagedHeapBase::GetBase());
void* heap_end = heap_start + kCagedHeapReservationSize - 1;
AssertAgeForAddressRange(heap_start, heap_end, Age::kOld);
Expand Down

0 comments on commit d097bd3

Please sign in to comment.