[heap] Refactor addition and removal of pages in PagedSpace.

Bug: chromium:694255
Change-Id: I7cd5b713f4a1d64dc53d99b65c924cae6e39f193
Reviewed-on: https://chromium-review.googlesource.com/621009
Commit-Queue: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#47439}

src/heap/spaces-inl.h

@@ -161,9 +161,9 @@ void PagedSpace::UnlinkFreeListCategories(Page* page) {
   });
 }
 
-intptr_t PagedSpace::RelinkFreeListCategories(Page* page) {
+size_t PagedSpace::RelinkFreeListCategories(Page* page) {
   DCHECK_EQ(this, page->owner());
-  intptr_t added = 0;
+  size_t added = 0;
   page->ForAllFreeListCategories([&added](FreeListCategory* category) {
     added += category->available();
     category->Relink();

src/heap/spaces.cc

@@ -1414,7 +1414,7 @@ void PagedSpace::RefillFreeList() {
     return;
   }
   MarkCompactCollector* collector = heap()->mark_compact_collector();
-  intptr_t added = 0;
+  size_t added = 0;
   {
     Page* p = nullptr;
     while ((p = collector->sweeper().GetSweptPageSafe(this)) != nullptr) {
@@ -1426,17 +1426,14 @@ void PagedSpace::RefillFreeList() {
         PagedSpace* owner = reinterpret_cast<PagedSpace*>(p->owner());
         base::LockGuard<base::Mutex> guard(owner->mutex());
         owner->RefineAllocatedBytesAfterSweeping(p);
-        p->Unlink();
-        owner->AccountRemovedPage(p);
-        p->set_owner(this);
-        p->InsertAfter(anchor_.prev_page());
-        AccountAddedPage(p);
+        owner->RemovePage(p);
+        added += AddPage(p);
       } else {
         base::LockGuard<base::Mutex> guard(mutex());
         DCHECK_EQ(this, p->owner());
         RefineAllocatedBytesAfterSweeping(p);
+        added += RelinkFreeListCategories(p);
       }
-      added += RelinkFreeListCategories(p);
       added += p->wasted_memory();
       if (is_local() && (added > kCompactionMemoryWanted)) break;
     }
@@ -1461,13 +1458,8 @@ void PagedSpace::MergeCompactionSpace(CompactionSpace* other) {
   for (auto it = other->begin(); it != other->end();) {
     Page* p = *(it++);
     // Relinking requires the category to be unlinked.
-    other->UnlinkFreeListCategories(p);
-    p->Unlink();
-    other->AccountRemovedPage(p);
-    p->set_owner(this);
-    p->InsertAfter(anchor_.prev_page());
-    this->AccountAddedPage(p);
-    RelinkFreeListCategories(p);
+    other->RemovePage(p);
+    AddPage(p);
     DCHECK_EQ(p->AvailableInFreeList(),
               p->AvailableInFreeListFromAllocatedBytes());
   }
@@ -1506,20 +1498,6 @@ void PagedSpace::RefineAllocatedBytesAfterSweeping(Page* page) {
   marking_state->SetLiveBytes(page, 0);
 }
 
-void PagedSpace::AccountAddedPage(Page* page) {
-  CHECK(page->SweepingDone());
-  AccountCommitted(page->size());
-  IncreaseCapacity(page->area_size());
-  IncreaseAllocatedBytes(page->allocated_bytes(), page);
-}
-
-void PagedSpace::AccountRemovedPage(Page* page) {
-  CHECK(page->SweepingDone());
-  DecreaseAllocatedBytes(page->allocated_bytes(), page);
-  DecreaseCapacity(page->area_size());
-  AccountUncommitted(page->size());
-}
-
 Page* PagedSpace::RemovePageSafe(int size_in_bytes) {
   base::LockGuard<base::Mutex> guard(mutex());
   // Check for pages that still contain free list entries. Bail out for smaller
@@ -1538,17 +1516,27 @@ Page* PagedSpace::RemovePageSafe(int size_in_bytes) {
   if (!page && static_cast<int>(kTiniest) >= minimum_category)
     page = free_list()->GetPageForCategoryType(kTiniest);
   if (!page) return nullptr;
-  page->Unlink();
-  AccountRemovedPage(page);
-  UnlinkFreeListCategories(page);
+  RemovePage(page);
   return page;
 }
 
-void PagedSpace::AddPage(Page* page) {
+size_t PagedSpace::AddPage(Page* page) {
+  CHECK(page->SweepingDone());
   page->set_owner(this);
   page->InsertAfter(anchor()->prev_page());
-  AccountAddedPage(page);
-  RelinkFreeListCategories(page);
+  AccountCommitted(page->size());
+  IncreaseCapacity(page->area_size());
+  IncreaseAllocatedBytes(page->allocated_bytes(), page);
+  return RelinkFreeListCategories(page);
+}
+
+void PagedSpace::RemovePage(Page* page) {
+  CHECK(page->SweepingDone());
+  page->Unlink();
+  UnlinkFreeListCategories(page);
+  DecreaseAllocatedBytes(page->allocated_bytes(), page);
+  DecreaseCapacity(page->area_size());
+  AccountUncommitted(page->size());
 }
 
 size_t PagedSpace::ShrinkPageToHighWaterMark(Page* page) {

src/heap/spaces.h

@@ -1952,7 +1952,7 @@ class V8_EXPORT_PRIVATE PagedSpace : NON_EXPORTED_BASE(public Space) {
  public:
   typedef PageIterator iterator;
 
-  static const intptr_t kCompactionMemoryWanted = 500 * KB;
+  static const size_t kCompactionMemoryWanted = 500 * KB;
 
   // Creates a space with an id.
   PagedSpace(Heap* heap, AllocationSpace id, Executability executable);
@@ -2101,8 +2101,6 @@ class V8_EXPORT_PRIVATE PagedSpace : NON_EXPORTED_BASE(public Space) {
     accounting_stats_.IncreaseCapacity(bytes);
   }
 
-  void AccountAddedPage(Page* page);
-  void AccountRemovedPage(Page* page);
   void RefineAllocatedBytesAfterSweeping(Page* page);
 
   // The dummy page that anchors the linked list of pages.
@@ -2110,7 +2108,10 @@ class V8_EXPORT_PRIVATE PagedSpace : NON_EXPORTED_BASE(public Space) {
 
   Page* InitializePage(MemoryChunk* chunk, Executability executable);
   void ReleasePage(Page* page);
-  void AddPage(Page* page);
+  // Adds the page to this space and returns the number of bytes added to the
+  // free list of the space.
+  size_t AddPage(Page* page);
+  void RemovePage(Page* page);
   // Remove a page if it has at least |size_in_bytes| bytes available that can
   // be used for allocation.
   Page* RemovePageSafe(int size_in_bytes);
@@ -2164,7 +2165,7 @@ class V8_EXPORT_PRIVATE PagedSpace : NON_EXPORTED_BASE(public Space) {
   base::Mutex* mutex() { return &space_mutex_; }
 
   inline void UnlinkFreeListCategories(Page* page);
-  inline intptr_t RelinkFreeListCategories(Page* page);
+  inline size_t RelinkFreeListCategories(Page* page);
 
   iterator begin() { return iterator(anchor_.next_page()); }
   iterator end() { return iterator(&anchor_); }