cppgc: Avoid fragmentation in NormalPageMemoryRegion

NormalPageMemoryRegion is a span of 10 pages, all of which must belong
to the same space. This requirement imposes a fragmentation issue for virtual space, which is not ideal for the current 2GB cage
configuration.

The CL fixes this by mixing pages of different spaces inside the same
NormalPageMemoryRegion. With cage it's actually not necessary anymore
to have NormalPageMemoryRegion, but we keep it to allow the code to be
uniform for cage/non-cage configurations.

There is no type confusion across spaces, since pages (even empty) are
never shared between spaces. In addition, the shared cage puts an
additional memory constraint on the GC. So, there is no security benefit
in having NormalPageMemoryRegion assigned to a single space.

Savings in reserved address space:
cnn:2021: 14%
facebook_infinite_scroll:2018: 23%

Bug: chromium:1325007, chromium:1352649
Change-Id: I7b49032d581dd56feb8633734a1f37803e9526c6
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3840749Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Anton Bikineev <bikineev@chromium.org>
Cr-Commit-Position: refs/heads/main@{#82584}

src/heap/cppgc/heap-page.cc

@@ -134,7 +134,7 @@ BasePage::BasePage(HeapBase& heap, BaseSpace& space, PageType type)
 // static
 NormalPage* NormalPage::Create(PageBackend& page_backend,
                                NormalPageSpace& space) {
-  void* memory = page_backend.AllocateNormalPageMemory(space.index());
+  void* memory = page_backend.AllocateNormalPageMemory();
   auto* normal_page = new (memory) NormalPage(*space.raw_heap()->heap(), space);
   normal_page->SynchronizedStore();
   normal_page->heap().stats_collector()->NotifyAllocatedMemory(kPageSize);

src/heap/cppgc/page-memory.cc

@@ -171,18 +171,15 @@ NormalPageMemoryPool::NormalPageMemoryPool() = default;
 
 NormalPageMemoryPool::~NormalPageMemoryPool() = default;
 
-void NormalPageMemoryPool::Add(size_t bucket, NormalPageMemoryRegion* pmr,
+void NormalPageMemoryPool::Add(NormalPageMemoryRegion* pmr,
                                Address writeable_base) {
-  DCHECK_LT(bucket, kNumPoolBuckets);
-  pool_[bucket].push_back(std::make_pair(pmr, writeable_base));
+  pool_.push_back(std::make_pair(pmr, writeable_base));
 }
 
-std::pair<NormalPageMemoryRegion*, Address> NormalPageMemoryPool::Take(
-    size_t bucket) {
-  DCHECK_LT(bucket, kNumPoolBuckets);
-  if (pool_[bucket].empty()) return {nullptr, nullptr};
-  std::pair<NormalPageMemoryRegion*, Address> pair = pool_[bucket].back();
-  pool_[bucket].pop_back();
+std::pair<NormalPageMemoryRegion*, Address> NormalPageMemoryPool::Take() {
+  if (pool_.empty()) return {nullptr, nullptr};
+  std::pair<NormalPageMemoryRegion*, Address> pair = pool_.back();
+  pool_.pop_back();
   return pair;
 }
 
@@ -195,19 +192,19 @@ PageBackend::PageBackend(PageAllocator& normal_page_allocator,
 
 PageBackend::~PageBackend() = default;
 
-Address PageBackend::AllocateNormalPageMemory(size_t bucket) {
+Address PageBackend::AllocateNormalPageMemory() {
   v8::base::MutexGuard guard(&mutex_);
-  std::pair<NormalPageMemoryRegion*, Address> result = page_pool_.Take(bucket);
+  std::pair<NormalPageMemoryRegion*, Address> result = page_pool_.Take();
   if (!result.first) {
     auto pmr = std::make_unique<NormalPageMemoryRegion>(normal_page_allocator_,
                                                         oom_handler_);
     for (size_t i = 0; i < NormalPageMemoryRegion::kNumPageRegions; ++i) {
-      page_pool_.Add(bucket, pmr.get(),
+      page_pool_.Add(pmr.get(),
                      pmr->GetPageMemory(i).writeable_region().base());
     }
     page_memory_region_tree_.Add(pmr.get());
     normal_page_memory_regions_.push_back(std::move(pmr));
-    result = page_pool_.Take(bucket);
+    result = page_pool_.Take();
     DCHECK(result.first);
   }
   result.first->Allocate(result.second);
@@ -219,7 +216,7 @@ void PageBackend::FreeNormalPageMemory(size_t bucket, Address writeable_base) {
   auto* pmr = static_cast<NormalPageMemoryRegion*>(
       page_memory_region_tree_.Lookup(writeable_base));
   pmr->Free(writeable_base);
-  page_pool_.Add(bucket, pmr, writeable_base);
+  page_pool_.Add(pmr, writeable_base);
 }
 
 Address PageBackend::AllocateLargePageMemory(size_t size) {

src/heap/cppgc/page-memory.h

@@ -178,18 +178,16 @@ class V8_EXPORT_PRIVATE PageMemoryRegionTree final {
 // capabilities.
 class V8_EXPORT_PRIVATE NormalPageMemoryPool final {
  public:
-  static constexpr size_t kNumPoolBuckets = 16;
-
   using Result = std::pair<NormalPageMemoryRegion*, Address>;
 
   NormalPageMemoryPool();
   ~NormalPageMemoryPool();
 
-  void Add(size_t, NormalPageMemoryRegion*, Address);
-  Result Take(size_t);
+  void Add(NormalPageMemoryRegion*, Address);
+  Result Take();
 
  private:
-  std::vector<Result> pool_[kNumPoolBuckets];
+  std::vector<Result> pool_;
 };
 
 // A backend that is used for allocating and freeing normal and large pages.
@@ -205,7 +203,7 @@ class V8_EXPORT_PRIVATE PageBackend final {
   // Allocates a normal page from the backend.
   //
   // Returns the writeable base of the region.
-  Address AllocateNormalPageMemory(size_t);
+  Address AllocateNormalPageMemory();
 
   // Returns normal page memory back to the backend. Expects the
   // |writeable_base| returned by |AllocateNormalMemory()|.

test/unittests/heap/cppgc/page-memory-unittest.cc

@@ -233,37 +233,19 @@ TEST(PageMemoryRegionTreeTest, AddLookupRemoveMultiple) {
 TEST(NormalPageMemoryPool, ConstructorEmpty) {
   v8::base::PageAllocator allocator;
   NormalPageMemoryPool pool;
-  constexpr size_t kBucket = 0;
-  EXPECT_EQ(NormalPageMemoryPool::Result(nullptr, nullptr), pool.Take(kBucket));
-}
-
-TEST(NormalPageMemoryPool, AddTakeSameBucket) {
-  v8::base::PageAllocator allocator;
-  FatalOutOfMemoryHandler oom_handler;
-  auto pmr = std::make_unique<NormalPageMemoryRegion>(allocator, oom_handler);
-  const PageMemory pm = pmr->GetPageMemory(0);
-  NormalPageMemoryPool pool;
-  constexpr size_t kBucket = 0;
-  pool.Add(kBucket, pmr.get(), pm.writeable_region().base());
-  EXPECT_EQ(
-      NormalPageMemoryPool::Result(pmr.get(), pm.writeable_region().base()),
-      pool.Take(kBucket));
+  EXPECT_EQ(NormalPageMemoryPool::Result(nullptr, nullptr), pool.Take());
 }
 
-TEST(NormalPageMemoryPool, AddTakeNotFoundDifferentBucket) {
+TEST(NormalPageMemoryPool, AddTake) {
   v8::base::PageAllocator allocator;
   FatalOutOfMemoryHandler oom_handler;
   auto pmr = std::make_unique<NormalPageMemoryRegion>(allocator, oom_handler);
   const PageMemory pm = pmr->GetPageMemory(0);
   NormalPageMemoryPool pool;
-  constexpr size_t kFirstBucket = 0;
-  constexpr size_t kSecondBucket = 1;
-  pool.Add(kFirstBucket, pmr.get(), pm.writeable_region().base());
-  EXPECT_EQ(NormalPageMemoryPool::Result(nullptr, nullptr),
-            pool.Take(kSecondBucket));
+  pool.Add(pmr.get(), pm.writeable_region().base());
   EXPECT_EQ(
       NormalPageMemoryPool::Result(pmr.get(), pm.writeable_region().base()),
-      pool.Take(kFirstBucket));
+      pool.Take());
 }
 
 TEST(PageBackendTest, AllocateNormalUsesPool) {
@@ -271,10 +253,10 @@ TEST(PageBackendTest, AllocateNormalUsesPool) {
   FatalOutOfMemoryHandler oom_handler;
   PageBackend backend(allocator, allocator, oom_handler);
   constexpr size_t kBucket = 0;
-  Address writeable_base1 = backend.AllocateNormalPageMemory(kBucket);
+  Address writeable_base1 = backend.AllocateNormalPageMemory();
   EXPECT_NE(nullptr, writeable_base1);
   backend.FreeNormalPageMemory(kBucket, writeable_base1);
-  Address writeable_base2 = backend.AllocateNormalPageMemory(kBucket);
+  Address writeable_base2 = backend.AllocateNormalPageMemory();
   EXPECT_NE(nullptr, writeable_base2);
   EXPECT_EQ(writeable_base1, writeable_base2);
 }
@@ -296,8 +278,7 @@ TEST(PageBackendTest, LookupNormal) {
   v8::base::PageAllocator allocator;
   FatalOutOfMemoryHandler oom_handler;
   PageBackend backend(allocator, allocator, oom_handler);
-  constexpr size_t kBucket = 0;
-  Address writeable_base = backend.AllocateNormalPageMemory(kBucket);
+  Address writeable_base = backend.AllocateNormalPageMemory();
   if (kGuardPageSize) {
     EXPECT_EQ(nullptr, backend.Lookup(writeable_base - kGuardPageSize));
   }
@@ -333,8 +314,7 @@ TEST(PageBackendDeathTest, DestructingBackendDestroysPageMemory) {
   Address base;
   {
     PageBackend backend(allocator, allocator, oom_handler);
-    constexpr size_t kBucket = 0;
-    base = backend.AllocateNormalPageMemory(kBucket);
+    base = backend.AllocateNormalPageMemory();
   }
   EXPECT_DEATH_IF_SUPPORTED(access(base[0]), "");
 }