Revert "Reland "[heap] Added per-page array buffer accouting (external memory).""

This reverts commit 9072bef0.

Reason for revert: Speculative revert because of https://chromium-review.googlesource.com/c/chromium/src/+/1118280

Original change's description:
> Reland "[heap] Added per-page array buffer accouting (external memory)."
> 
> This is a reland of d4792e8f
> 
> Original change's description:
> > [heap] Added per-page array buffer accouting (external memory).
> > 
> > Bug: chromium:845409
> > Change-Id: Ibc568cdc501edf5d84d9c6379aff58be069369af
> > Reviewed-on: https://chromium-review.googlesource.com/1114602
> > Commit-Queue: Rodrigo Bruno <rfbpb@google.com>
> > Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#54028}
> 
> Bug: chromium:845409
> Change-Id: I6b11d7f66313bcbcc31be9217c1b780cf3eaee99
> Reviewed-on: https://chromium-review.googlesource.com/1116638
> Commit-Queue: Rodrigo Bruno <rfbpb@google.com>
> Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#54066}

TBR=ulan@chromium.org,hpayer@chromium.org,mlippautz@chromium.org,rfbpb@google.com

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: chromium:845409
Change-Id: I818e24d236d93a4645d1532b666056f89156eb86
Reviewed-on: https://chromium-review.googlesource.com/1119825Reviewed-by: Michael Hablich <hablich@chromium.org>
Commit-Queue: Michael Hablich <hablich@chromium.org>
Cr-Commit-Position: refs/heads/master@{#54100}

src/heap/array-buffer-tracker-inl.h

@@ -75,8 +75,8 @@ void LocalArrayBufferTracker::Free(Callback should_free) {
     }
   }
   if (freed_memory > 0) {
-    page_->DecrementExternalBackingStoreBytes(
-        ExternalBackingStoreType::kArrayBuffer, freed_memory);
+    // Update the Space with any freed backing-store bytes.
+    space()->DecrementExternalBackingStoreBytes(freed_memory);
 
     // TODO(wez): Remove backing-store from external memory accounting.
     page_->heap()->update_external_memory_concurrently_freed(
@@ -98,8 +98,8 @@ void ArrayBufferTracker::FreeDead(Page* page, MarkingState* marking_state) {
 }
 
 void LocalArrayBufferTracker::Add(JSArrayBuffer* buffer, size_t length) {
-  page_->IncrementExternalBackingStoreBytes(
-      ExternalBackingStoreType::kArrayBuffer, length);
+  // Track the backing-store usage against the owning Space.
+  space()->IncrementExternalBackingStoreBytes(length);
 
   auto ret = array_buffers_.insert({buffer, {buffer->backing_store(), length}});
   USE(ret);
@@ -109,8 +109,8 @@ void LocalArrayBufferTracker::Add(JSArrayBuffer* buffer, size_t length) {
 }
 
 void LocalArrayBufferTracker::Remove(JSArrayBuffer* buffer, size_t length) {
-  page_->DecrementExternalBackingStoreBytes(
-      ExternalBackingStoreType::kArrayBuffer, length);
+  // Remove the backing-store accounting from the owning Space.
+  space()->DecrementExternalBackingStoreBytes(length);
 
   TrackingData::iterator it = array_buffers_.find(buffer);
   // Check that we indeed find a key to remove.

src/heap/array-buffer-tracker.cc

@@ -30,7 +30,6 @@ void LocalArrayBufferTracker::Process(Callback callback) {
   for (TrackingData::iterator it = array_buffers_.begin();
        it != array_buffers_.end(); ++it) {
     old_buffer = reinterpret_cast<JSArrayBuffer*>(it->first);
-    Page* old_page = Page::FromAddress(old_buffer->address());
     const CallbackResult result = callback(old_buffer, &new_buffer);
     if (result == kKeepEntry) {
       kept_array_buffers.insert(*it);
@@ -46,14 +45,9 @@ void LocalArrayBufferTracker::Process(Callback callback) {
         }
         DCHECK_NOT_NULL(tracker);
         const size_t size = NumberToSize(new_buffer->byte_length());
-        // We should decrement before adding to avoid potential overflows in
-        // the external memory counters.
-        old_page->DecrementExternalBackingStoreBytes(
-            ExternalBackingStoreType::kArrayBuffer, it->second.length);
         tracker->Add(new_buffer, size);
       }
       moved_memory += it->second.length;
-
     } else if (result == kRemoveEntry) {
       freed_memory += it->second.length;
       // We pass backing_store() and stored length to the collector for freeing
@@ -62,14 +56,14 @@ void LocalArrayBufferTracker::Process(Callback callback) {
       backing_stores_to_free.emplace_back(
           it->second.backing_store, it->second.length, it->second.backing_store,
           old_buffer->allocation_mode(), old_buffer->is_wasm_memory());
-      old_page->DecrementExternalBackingStoreBytes(
-          ExternalBackingStoreType::kArrayBuffer, it->second.length);
-
     } else {
       UNREACHABLE();
     }
   }
   if (moved_memory || freed_memory) {
+    // Update the Space with any moved or freed backing-store bytes.
+    space()->DecrementExternalBackingStoreBytes(freed_memory + moved_memory);
+
     // TODO(wez): Remove backing-store from external memory accounting.
     page_->heap()->update_external_memory_concurrently_freed(
         static_cast<intptr_t>(freed_memory));

src/heap/spaces.cc

@@ -631,6 +631,7 @@ MemoryChunk* MemoryChunk::Initialize(Heap* heap, Address base, size_t size,
   chunk->young_generation_bitmap_ = nullptr;
   chunk->local_tracker_ = nullptr;
 
+  chunk->external_backing_store_bytes_[ExternalBackingStoreType::kOther] = 0;
   chunk->external_backing_store_bytes_[ExternalBackingStoreType::kArrayBuffer] =
       0;
   chunk->external_backing_store_bytes_
@@ -1399,19 +1400,6 @@ void MemoryChunk::ReleaseYoungGenerationBitmap() {
   young_generation_bitmap_ = nullptr;
 }
 
-void MemoryChunk::IncrementExternalBackingStoreBytes(
-    ExternalBackingStoreType type, size_t amount) {
-  external_backing_store_bytes_[type] += amount;
-  owner()->IncrementExternalBackingStoreBytes(type, amount);
-}
-
-void MemoryChunk::DecrementExternalBackingStoreBytes(
-    ExternalBackingStoreType type, size_t amount) {
-  DCHECK_GE(external_backing_store_bytes_[type], amount);
-  external_backing_store_bytes_[type] -= amount;
-  owner()->DecrementExternalBackingStoreBytes(type, amount);
-}
-
 // -----------------------------------------------------------------------------
 // PagedSpace implementation
 
@@ -1512,6 +1500,9 @@ void PagedSpace::RefillFreeList() {
 void PagedSpace::MergeCompactionSpace(CompactionSpace* other) {
   base::LockGuard<base::Mutex> guard(mutex());
 
+  IncrementExternalBackingStoreBytes(other->ExternalBackingStoreBytes());
+  other->DecrementExternalBackingStoreBytes(other->ExternalBackingStoreBytes());
+
   DCHECK(identity() == other->identity());
   // Unmerged fields:
   //   area_size_
@@ -1601,10 +1592,6 @@ size_t PagedSpace::AddPage(Page* page) {
   AccountCommitted(page->size());
   IncreaseCapacity(page->area_size());
   IncreaseAllocatedBytes(page->allocated_bytes(), page);
-  for (size_t i = 0; i < ExternalBackingStoreType::kNumTypes; i++) {
-    ExternalBackingStoreType t = static_cast<ExternalBackingStoreType>(i);
-    IncrementExternalBackingStoreBytes(t, page->ExternalBackingStoreBytes(t));
-  }
   return RelinkFreeListCategories(page);
 }
 
@@ -1615,10 +1602,6 @@ void PagedSpace::RemovePage(Page* page) {
   DecreaseAllocatedBytes(page->allocated_bytes(), page);
   DecreaseCapacity(page->area_size());
   AccountUncommitted(page->size());
-  for (size_t i = 0; i < ExternalBackingStoreType::kNumTypes; i++) {
-    ExternalBackingStoreType t = static_cast<ExternalBackingStoreType>(i);
-    DecrementExternalBackingStoreBytes(t, page->ExternalBackingStoreBytes(t));
-  }
 }
 
 size_t PagedSpace::ShrinkPageToHighWaterMark(Page* page) {
@@ -2571,10 +2554,6 @@ void SemiSpace::RemovePage(Page* page) {
     }
   }
   memory_chunk_list_.Remove(page);
-  for (size_t i = 0; i < ExternalBackingStoreType::kNumTypes; i++) {
-    ExternalBackingStoreType t = static_cast<ExternalBackingStoreType>(i);
-    DecrementExternalBackingStoreBytes(t, page->ExternalBackingStoreBytes(t));
-  }
 }
 
 void SemiSpace::PrependPage(Page* page) {
@@ -2583,10 +2562,6 @@ void SemiSpace::PrependPage(Page* page) {
   page->set_owner(this);
   memory_chunk_list_.PushFront(page);
   pages_used_++;
-  for (size_t i = 0; i < ExternalBackingStoreType::kNumTypes; i++) {
-    ExternalBackingStoreType t = static_cast<ExternalBackingStoreType>(i);
-    IncrementExternalBackingStoreBytes(t, page->ExternalBackingStoreBytes(t));
-  }
 }
 
 void SemiSpace::Swap(SemiSpace* from, SemiSpace* to) {

src/heap/spaces.h

@@ -143,6 +143,7 @@ enum FreeMode { kLinkCategory, kDoNotLinkCategory };
 enum class SpaceAccountingMode { kSpaceAccounted, kSpaceUnaccounted };
 
 enum ExternalBackingStoreType {
+  kOther,
   kArrayBuffer,
   kExternalString,
   kNumTypes
@@ -537,12 +538,13 @@ class MemoryChunk {
     }
   }
 
-  void IncrementExternalBackingStoreBytes(ExternalBackingStoreType type,
-                                          size_t amount);
-  void DecrementExternalBackingStoreBytes(ExternalBackingStoreType type,
-                                          size_t amount);
-  size_t ExternalBackingStoreBytes(ExternalBackingStoreType type) {
-    return external_backing_store_bytes_[type];
+  void IncrementExternalBackingStoreBytes(size_t amount,
+                                          ExternalBackingStoreType type) {
+    external_backing_store_bytes_[type] += amount;
+  }
+  void DecrementExternalBackingStoreBytes(size_t amount,
+                                          ExternalBackingStoreType type) {
+    external_backing_store_bytes_[type] -= amount;
   }
 
   inline uint32_t AddressToMarkbitIndex(Address addr) const {
@@ -911,6 +913,7 @@ class Space : public Malloced {
         max_committed_(0) {
     external_backing_store_bytes_ =
         new std::atomic<size_t>[ExternalBackingStoreType::kNumTypes];
+    external_backing_store_bytes_[ExternalBackingStoreType::kOther] = 0;
     external_backing_store_bytes_[ExternalBackingStoreType::kArrayBuffer] = 0;
     external_backing_store_bytes_[ExternalBackingStoreType::kExternalString] =
         0;
@@ -957,7 +960,7 @@ class Space : public Malloced {
 
   // Returns amount of off-heap memory in-use by objects in this Space.
   virtual size_t ExternalBackingStoreBytes(
-      ExternalBackingStoreType type) const {
+      ExternalBackingStoreType type = ExternalBackingStoreType::kOther) const {
     return external_backing_store_bytes_[type];
   }
 
@@ -990,13 +993,14 @@ class Space : public Malloced {
     committed_ -= bytes;
   }
 
-  void IncrementExternalBackingStoreBytes(ExternalBackingStoreType type,
-                                          size_t amount) {
+  void IncrementExternalBackingStoreBytes(
+      size_t amount,
+      ExternalBackingStoreType type = ExternalBackingStoreType::kOther) {
     external_backing_store_bytes_[type] += amount;
   }
-  void DecrementExternalBackingStoreBytes(ExternalBackingStoreType type,
-                                          size_t amount) {
-    DCHECK_GE(external_backing_store_bytes_[type], amount);
+  void DecrementExternalBackingStoreBytes(
+      size_t amount,
+      ExternalBackingStoreType type = ExternalBackingStoreType::kOther) {
     external_backing_store_bytes_[type] -= amount;
   }
 
@@ -2622,7 +2626,8 @@ class NewSpace : public SpaceWithLinearArea {
   }
 
   size_t ExternalBackingStoreBytes(
-      ExternalBackingStoreType type) const override {
+      ExternalBackingStoreType type =
+          ExternalBackingStoreType::kOther) const override {
     DCHECK_EQ(0, from_space_.ExternalBackingStoreBytes(type));
     return to_space_.ExternalBackingStoreBytes(type);
   }

test/cctest/heap/test-array-buffer-tracker.cc

@@ -343,17 +343,16 @@ TEST(ArrayBuffer_ExternalBackingStoreSizeIncreases) {
   LocalContext env;
   v8::Isolate* isolate = env->GetIsolate();
   Heap* heap = reinterpret_cast<Isolate*>(isolate)->heap();
-  ExternalBackingStoreType type = ExternalBackingStoreType::kArrayBuffer;
 
   const size_t backing_store_before =
-      heap->new_space()->ExternalBackingStoreBytes(type);
+      heap->new_space()->ExternalBackingStoreBytes();
   {
     const size_t kArraybufferSize = 117;
     v8::HandleScope handle_scope(isolate);
     Local<v8::ArrayBuffer> ab = v8::ArrayBuffer::New(isolate, kArraybufferSize);
     USE(ab);
     const size_t backing_store_after =
-        heap->new_space()->ExternalBackingStoreBytes(type);
+        heap->new_space()->ExternalBackingStoreBytes();
     CHECK_EQ(kArraybufferSize, backing_store_after - backing_store_before);
   }
 }
@@ -363,10 +362,9 @@ TEST(ArrayBuffer_ExternalBackingStoreSizeDecreases) {
   LocalContext env;
   v8::Isolate* isolate = env->GetIsolate();
   Heap* heap = reinterpret_cast<Isolate*>(isolate)->heap();
-  ExternalBackingStoreType type = ExternalBackingStoreType::kArrayBuffer;
 
   const size_t backing_store_before =
-      heap->new_space()->ExternalBackingStoreBytes(type);
+      heap->new_space()->ExternalBackingStoreBytes();
   {
     const size_t kArraybufferSize = 117;
     v8::HandleScope handle_scope(isolate);
@@ -375,7 +373,7 @@ TEST(ArrayBuffer_ExternalBackingStoreSizeDecreases) {
   }
   heap::GcAndSweep(heap, OLD_SPACE);
   const size_t backing_store_after =
-      heap->new_space()->ExternalBackingStoreBytes(type);
+      heap->new_space()->ExternalBackingStoreBytes();
   CHECK_EQ(0, backing_store_after - backing_store_before);
 }
 
@@ -388,10 +386,9 @@ TEST(ArrayBuffer_ExternalBackingStoreSizeIncreasesMarkCompact) {
   v8::Isolate* isolate = env->GetIsolate();
   Heap* heap = reinterpret_cast<Isolate*>(isolate)->heap();
   heap::AbandonCurrentlyFreeMemory(heap->old_space());
-  ExternalBackingStoreType type = ExternalBackingStoreType::kArrayBuffer;
 
   const size_t backing_store_before =
-      heap->old_space()->ExternalBackingStoreBytes(type);
+      heap->old_space()->ExternalBackingStoreBytes();
 
   const size_t kArraybufferSize = 117;
   {
@@ -410,13 +407,13 @@ TEST(ArrayBuffer_ExternalBackingStoreSizeIncreasesMarkCompact) {
     CcTest::CollectAllGarbage();
 
     const size_t backing_store_after =
-        heap->old_space()->ExternalBackingStoreBytes(type);
+        heap->old_space()->ExternalBackingStoreBytes();
     CHECK_EQ(kArraybufferSize, backing_store_after - backing_store_before);
   }
 
   heap::GcAndSweep(heap, OLD_SPACE);
   const size_t backing_store_after =
-      heap->old_space()->ExternalBackingStoreBytes(type);
+      heap->old_space()->ExternalBackingStoreBytes();
   CHECK_EQ(0, backing_store_after - backing_store_before);
 }