[heap] Fix allocated_object_size for RO_SPACE

After https://chromium-review.googlesource.com/c/v8/v8/+/2250254,
allocated_object_size in RO_SPACE is incorrect. This changes it to use
the accounting_stats_ value. This also fixes the Capacity() which was
previously uninitialized. Both are tested in new ReadOnlySpace allocation
tests in test-spaces.cc.

Couple of cleanups:
* area_size_ becomes const since its value is fixed after construction.
* Deletes incorrect comment in base-space.h

Bug: v8:10454
Change-Id: I9bbbc1ef2548722eee9dae1bb8d67448eccf8955
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2259937
Commit-Queue: Dan Elphick <delphick@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#68489}

src/heap/base-space.h

@@ -62,8 +62,6 @@ class V8_EXPORT_PRIVATE BaseSpace : public Malloced {
   BaseSpace(Heap* heap, AllocationSpace id)
       : heap_(heap), id_(id), committed_(0), max_committed_(0) {}
 
-  // Even though this has no virtual functions, this ensures that pointers are
-  // stable through casting.
   virtual ~BaseSpace() = default;
 
  protected:

src/heap/read-only-spaces.cc

@@ -28,6 +28,7 @@ ReadOnlySpace::ReadOnlySpace(Heap* heap)
       top_(kNullAddress),
       limit_(kNullAddress),
       is_string_padding_cleared_(heap->isolate()->initialized_from_snapshot()),
+      capacity_(0),
       area_size_(MemoryChunkLayout::AllocatableMemoryInMemoryChunk(RO_SPACE)) {}
 
 ReadOnlySpace::~ReadOnlySpace() {
@@ -362,6 +363,7 @@ void ReadOnlySpace::EnsureSpaceForAllocation(int size_in_bytes) {
 
   BasicMemoryChunk* chunk =
       heap()->memory_allocator()->AllocateReadOnlyPage(AreaSize(), this);
+  capacity_ += AreaSize();
 
   accounting_stats_.IncreaseCapacity(chunk->area_size());
   AccountCommitted(chunk->size());
@@ -491,7 +493,6 @@ size_t ReadOnlyPage::ShrinkToHighWaterMark() {
 }
 
 void ReadOnlySpace::ShrinkPages() {
-  DCHECK(!heap()->deserialization_complete());
   BasicMemoryChunk::UpdateHighWaterMark(top_);
   heap()->CreateFillerObjectAt(top_, static_cast<int>(limit_ - top_),
                                ClearRecordedSlots::kNo);
@@ -499,6 +500,7 @@ void ReadOnlySpace::ShrinkPages() {
   for (ReadOnlyPage* chunk : pages_) {
     DCHECK(chunk->IsFlagSet(Page::NEVER_EVACUATE));
     size_t unused = chunk->ShrinkToHighWaterMark();
+    capacity_ -= unused;
     accounting_stats_.DecreaseCapacity(static_cast<intptr_t>(unused));
     AccountUncommitted(unused);
   }

src/heap/read-only-spaces.h

@@ -72,14 +72,14 @@ class ReadOnlyArtifacts {
 // Read Only space for all Immortal Immovable and Immutable objects
 class ReadOnlySpace : public BaseSpace {
  public:
-  explicit ReadOnlySpace(Heap* heap);
+  V8_EXPORT_PRIVATE explicit ReadOnlySpace(Heap* heap);
 
   // Detach the pages and them to artifacts for using in creating a
   // SharedReadOnlySpace.
   void DetachPagesAndAddToArtifacts(
       std::shared_ptr<ReadOnlyArtifacts> artifacts);
 
-  ~ReadOnlySpace() override;
+  V8_EXPORT_PRIVATE ~ReadOnlySpace() override;
 
   bool IsDetached() const { return heap_ == nullptr; }
 
@@ -99,14 +99,14 @@ class ReadOnlySpace : public BaseSpace {
   // Seal the space by marking it read-only, optionally detaching it
   // from the heap and forgetting it for memory bookkeeping purposes (e.g.
   // prevent space's memory from registering as leaked).
-  void Seal(SealMode ro_mode);
+  V8_EXPORT_PRIVATE void Seal(SealMode ro_mode);
 
   // During boot the free_space_map is created, and afterwards we may need
   // to write it into the free space nodes that were already created.
   void RepairFreeSpacesAfterDeserialization();
 
-  size_t Size() override { return area_size_; }
-  size_t CommittedPhysicalMemory() override;
+  size_t Size() override { return accounting_stats_.Size(); }
+  V8_EXPORT_PRIVATE size_t CommittedPhysicalMemory() override;
 
   const std::vector<ReadOnlyPage*>& pages() const { return pages_; }
   Address top() const { return top_; }
@@ -114,7 +114,7 @@ class ReadOnlySpace : public BaseSpace {
   size_t Capacity() const { return capacity_; }
 
   bool ContainsSlow(Address addr);
-  void ShrinkPages();
+  V8_EXPORT_PRIVATE void ShrinkPages();
 #ifdef VERIFY_HEAP
   void Verify(Isolate* isolate);
 #ifdef DEBUG
@@ -164,7 +164,7 @@ class ReadOnlySpace : public BaseSpace {
   bool is_string_padding_cleared_;
 
   size_t capacity_;
-  size_t area_size_;
+  const size_t area_size_;
 };
 
 class SharedReadOnlySpace : public ReadOnlySpace {

test/cctest/heap/test-spaces.cc

@@ -29,7 +29,9 @@
 
 #include "include/v8-platform.h"
 #include "src/base/bounded-page-allocator.h"
+#include "src/base/macros.h"
 #include "src/base/platform/platform.h"
+#include "src/common/globals.h"
 #include "src/heap/factory.h"
 #include "src/heap/large-spaces.h"
 #include "src/heap/memory-allocator.h"
@@ -771,7 +773,6 @@ class FailingPageAllocator : public v8::PageAllocator {
     return false;
   }
 };
-
 }  // namespace
 
 TEST(NoMemoryForNewPage) {
@@ -791,6 +792,92 @@ TEST(NoMemoryForNewPage) {
   CHECK_NULL(page);
 }
 
+TEST(ReadOnlySpaceMetrics_OnePage) {
+  Isolate* isolate = CcTest::i_isolate();
+  Heap* heap = isolate->heap();
+
+  // Create a read-only space and allocate some memory, shrink the pages and
+  // check the allocated object size is as expected.
+
+  ReadOnlySpace faked_space(heap);
+
+  // Initially no memory.
+  CHECK_EQ(faked_space.Size(), 0);
+  CHECK_EQ(faked_space.Capacity(), 0);
+  CHECK_EQ(faked_space.CommittedMemory(), 0);
+  CHECK_EQ(faked_space.CommittedPhysicalMemory(), 0);
+
+  faked_space.AllocateRaw(16, kWordAligned);
+
+  faked_space.ShrinkPages();
+  faked_space.Seal(ReadOnlySpace::SealMode::kDoNotDetachFromHeap);
+
+  MemoryAllocator* allocator = heap->memory_allocator();
+
+  // Allocated objects size.
+  CHECK_EQ(faked_space.Size(), 16);
+
+  // Capacity will be one OS page minus the page header.
+  CHECK_EQ(faked_space.Capacity(),
+           allocator->GetCommitPageSize() -
+               MemoryChunkLayout::ObjectStartOffsetInDataPage());
+
+  // Amount of OS allocated memory.
+  CHECK_EQ(faked_space.CommittedMemory(), allocator->GetCommitPageSize());
+  CHECK_EQ(faked_space.CommittedPhysicalMemory(),
+           allocator->GetCommitPageSize());
+}
+
+TEST(ReadOnlySpaceMetrics_TwoPages) {
+  Isolate* isolate = CcTest::i_isolate();
+  Heap* heap = isolate->heap();
+
+  // Create a read-only space and allocate some memory, shrink the pages and
+  // check the allocated object size is as expected.
+
+  ReadOnlySpace faked_space(heap);
+
+  // Initially no memory.
+  CHECK_EQ(faked_space.Size(), 0);
+  CHECK_EQ(faked_space.Capacity(), 0);
+  CHECK_EQ(faked_space.CommittedMemory(), 0);
+  CHECK_EQ(faked_space.CommittedPhysicalMemory(), 0);
+
+  MemoryAllocator* allocator = heap->memory_allocator();
+
+  // Allocate an object that's too big to have more than one on a page.
+  size_t object_size =
+      MemoryChunkLayout::AllocatableMemoryInMemoryChunk(RO_SPACE) / 2 + 16;
+  CHECK_GT(object_size * 2,
+           MemoryChunkLayout::AllocatableMemoryInMemoryChunk(RO_SPACE));
+  faked_space.AllocateRaw(object_size, kWordAligned);
+
+  // Then allocate another so it expands the space to two pages.
+  faked_space.AllocateRaw(object_size, kWordAligned);
+
+  faked_space.ShrinkPages();
+  faked_space.Seal(ReadOnlySpace::SealMode::kDoNotDetachFromHeap);
+
+  // Allocated objects size.
+  CHECK_EQ(faked_space.Size(), object_size * 2);
+
+  // Amount of OS allocated memory.
+  size_t committed_memory_per_page =
+      RoundUp(MemoryChunkLayout::ObjectStartOffsetInDataPage() + object_size,
+              allocator->GetCommitPageSize());
+  CHECK_EQ(faked_space.CommittedMemory(), 2 * committed_memory_per_page);
+  CHECK_EQ(faked_space.CommittedPhysicalMemory(),
+           2 * committed_memory_per_page);
+
+  // Capacity will be the space up to the amount of committed memory minus the
+  // page headers.
+  size_t capacity_per_page =
+      RoundUp(MemoryChunkLayout::ObjectStartOffsetInDataPage() + object_size,
+              allocator->GetCommitPageSize()) -
+      MemoryChunkLayout::ObjectStartOffsetInDataPage();
+  CHECK_EQ(faked_space.Capacity(), 2 * capacity_per_page);
+}
+
 }  // namespace heap
 }  // namespace internal
 }  // namespace v8