Revert "Reland "[heap] Introduce backing store counters on Heap""

This reverts commit 9d5ab51f.

Reason for revert: Blocks important revert. Also rolling is down
since 2 days... so please wait with relanding until rolling is
confirmed to be up again.
https://chromium-review.googlesource.com/c/v8/v8/+/1219695

Original change's description:
> Reland "[heap] Introduce backing store counters on Heap"
> 
> - Update those counters from space
> - Add fast path for move memory
> 
> Fix:
> - Introduce proper variants for checked arithmetic on atomics.
> 
> This reverts commit 0c62f5b3.
> 
> Bug: chromium:845409
> Change-Id: I98ad1a7c470c7e7b805e087b4ec74e3451344712
> Reviewed-on: https://chromium-review.googlesource.com/1219647
> Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
> Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#55815}

TBR=ulan@chromium.org,mlippautz@chromium.org

Change-Id: Ifb7c6e31f7f105bd11d1497a1061f73d42c05c31
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: chromium:845409
Reviewed-on: https://chromium-review.googlesource.com/1221186Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Commit-Queue: Michael Achenbach <machenbach@chromium.org>
Cr-Commit-Position: refs/heads/master@{#55816}

src/base/atomic-utils.h

@@ -377,22 +377,6 @@ class AtomicElement {
   T value_;
 };
 
-template <typename T,
-          typename = typename std::enable_if<std::is_unsigned<T>::value>::type>
-inline void CheckedIncrement(std::atomic<T>* number, T amount) {
-  const T old = number->fetch_add(amount);
-  DCHECK_GE(old + amount, old);
-  USE(old);
-}
-
-template <typename T,
-          typename = typename std::enable_if<std::is_unsigned<T>::value>::type>
-inline void CheckedDecrement(std::atomic<T>* number, T amount) {
-  const T old = number->fetch_sub(amount);
-  DCHECK_GE(old, amount);
-  USE(old);
-}
-
 }  // namespace base
 }  // namespace v8
 

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

@@ -100,11 +100,6 @@ void LocalArrayBufferTracker::Add(JSArrayBuffer* buffer, size_t length) {
   page_->IncrementExternalBackingStoreBytes(
       ExternalBackingStoreType::kArrayBuffer, length);
 
-  AddInternal(buffer, length);
-}
-
-void LocalArrayBufferTracker::AddInternal(JSArrayBuffer* buffer,
-                                          size_t length) {
   auto ret = array_buffers_.insert(
       {buffer,
        {buffer->backing_store(), length, buffer->backing_store(),

src/heap/array-buffer-tracker.cc

@@ -26,10 +26,11 @@ void LocalArrayBufferTracker::Process(Callback callback) {
   JSArrayBuffer* new_buffer = nullptr;
   JSArrayBuffer* old_buffer = nullptr;
   size_t freed_memory = 0;
+  size_t moved_memory = 0;
   for (TrackingData::iterator it = array_buffers_.begin();
        it != array_buffers_.end(); ++it) {
     old_buffer = it->first;
-    DCHECK_EQ(page_, Page::FromAddress(old_buffer->address()));
+    Page* old_page = Page::FromAddress(old_buffer->address());
     const CallbackResult result = callback(old_buffer, &new_buffer);
     if (result == kKeepEntry) {
       kept_array_buffers.insert(*it);
@@ -48,25 +49,26 @@ void LocalArrayBufferTracker::Process(Callback callback) {
         // We should decrement before adding to avoid potential overflows in
         // the external memory counters.
         DCHECK_EQ(it->first->is_wasm_memory(), it->second.is_wasm_memory);
-        tracker->AddInternal(new_buffer, length);
-        MemoryChunk::MoveExternalBackingStoreBytes(
-            ExternalBackingStoreType::kArrayBuffer,
-            static_cast<MemoryChunk*>(page_),
-            static_cast<MemoryChunk*>(target_page), length);
+        old_page->DecrementExternalBackingStoreBytes(
+            ExternalBackingStoreType::kArrayBuffer, length);
+        tracker->Add(new_buffer, length);
       }
+      moved_memory += it->second.length;
+
     } else if (result == kRemoveEntry) {
-      freed_memory += it->second.length;
+      const size_t length = it->second.length;
+      freed_memory += length;
       // We pass backing_store() and stored length to the collector for freeing
       // the backing store. Wasm allocations will go through their own tracker
       // based on the backing store.
       backing_stores_to_free.push_back(it->second);
+      old_page->DecrementExternalBackingStoreBytes(
+          ExternalBackingStoreType::kArrayBuffer, length);
     } else {
       UNREACHABLE();
     }
   }
-  if (freed_memory) {
-    page_->DecrementExternalBackingStoreBytes(
-        ExternalBackingStoreType::kArrayBuffer, freed_memory);
+  if (moved_memory || freed_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/array-buffer-tracker.h

@@ -113,10 +113,6 @@ class LocalArrayBufferTracker {
   typedef std::unordered_map<JSArrayBuffer*, JSArrayBuffer::Allocation, Hasher>
       TrackingData;
 
-  // Internal version of add that does not update counters. Requires separate
-  // logic for updating external memory counters.
-  inline void AddInternal(JSArrayBuffer* buffer, size_t length);
-
   inline Space* space();
 
   Page* page_;

src/heap/heap-inl.h

@@ -13,7 +13,6 @@
 #include "src/heap/heap-write-barrier.h"
 #include "src/heap/heap.h"
 
-#include "src/base/atomic-utils.h"
 #include "src/base/platform/platform.h"
 #include "src/counters-inl.h"
 #include "src/feedback-vector.h"
@@ -582,19 +581,6 @@ int Heap::MaxNumberToStringCacheSize() const {
   // of entries.
   return static_cast<int>(number_string_cache_size * 2);
 }
-
-void Heap::IncrementExternalBackingStoreBytes(ExternalBackingStoreType type,
-                                              size_t amount) {
-  base::CheckedIncrement(&backing_store_bytes_, amount);
-  // TODO(mlippautz): Implement interrupt for global memory allocations that can
-  // trigger garbage collections.
-}
-
-void Heap::DecrementExternalBackingStoreBytes(ExternalBackingStoreType type,
-                                              size_t amount) {
-  base::CheckedDecrement(&backing_store_bytes_, amount);
-}
-
 AlwaysAllocateScope::AlwaysAllocateScope(Isolate* isolate)
     : heap_(isolate->heap()) {
   heap_->always_allocate_scope_count_++;

src/heap/heap.cc

@@ -155,7 +155,6 @@ Heap::Heap()
       // Will be 4 * reserved_semispace_size_ to ensure that young
       // generation can be aligned to its size.
       maximum_committed_(0),
-      backing_store_bytes_(0),
       survived_since_last_expansion_(0),
       survived_last_scavenge_(0),
       always_allocate_scope_count_(0),
@@ -478,8 +477,6 @@ void Heap::PrintShortHeapStatistics() {
                CommittedMemoryOfHeapAndUnmapper() / KB);
   PrintIsolate(isolate_, "External memory reported: %6" PRId64 " KB\n",
                external_memory_ / KB);
-  PrintIsolate(isolate_, "Backing store memory: %6" PRIuS " KB\n",
-               backing_store_bytes_ / KB);
   PrintIsolate(isolate_, "External memory global %zu KB\n",
                external_memory_callback_() / KB);
   PrintIsolate(isolate_, "Total time spent in GC  : %.1f ms\n",
@@ -2292,6 +2289,15 @@ bool Heap::ExternalStringTable::Contains(HeapObject* obj) {
   return false;
 }
 
+void Heap::ProcessMovedExternalString(Page* old_page, Page* new_page,
+                                      ExternalString* string) {
+  size_t size = string->ExternalPayloadSize();
+  new_page->IncrementExternalBackingStoreBytes(
+      ExternalBackingStoreType::kExternalString, size);
+  old_page->DecrementExternalBackingStoreBytes(
+      ExternalBackingStoreType::kExternalString, size);
+}
+
 String* Heap::UpdateNewSpaceReferenceInExternalStringTableEntry(Heap* heap,
                                                                 Object** p) {
   MapWord first_word = HeapObject::cast(*p)->map_word();
@@ -2319,11 +2325,9 @@ String* Heap::UpdateNewSpaceReferenceInExternalStringTableEntry(Heap* heap,
     // Filtering Thin strings out of the external string table.
     return nullptr;
   } else if (new_string->IsExternalString()) {
-    MemoryChunk::MoveExternalBackingStoreBytes(
-        ExternalBackingStoreType::kExternalString,
+    heap->ProcessMovedExternalString(
         Page::FromAddress(reinterpret_cast<Address>(*p)),
-        Page::FromHeapObject(new_string),
-        ExternalString::cast(new_string)->ExternalPayloadSize());
+        Page::FromHeapObject(new_string), ExternalString::cast(new_string));
     return new_string;
   }
 

src/heap/heap.h

@@ -207,8 +207,6 @@ enum class ClearRecordedSlots { kYes, kNo };
 
 enum class ClearFreedMemoryMode { kClearFreedMemory, kDontClearFreedMemory };
 
-enum ExternalBackingStoreType { kArrayBuffer, kExternalString, kNumTypes };
-
 enum class FixedArrayVisitationMode { kRegular, kIncremental };
 
 enum class TraceRetainingPathMode { kEnabled, kDisabled };
@@ -692,7 +690,8 @@ class Heap {
     external_memory_concurrently_freed_ = 0;
   }
 
-  size_t backing_store_bytes() const { return backing_store_bytes_; }
+  void ProcessMovedExternalString(Page* old_page, Page* new_page,
+                                  ExternalString* string);
 
   void CompactWeakArrayLists(PretenureFlag pretenure);
 
@@ -1844,12 +1843,6 @@ class Heap {
   void CheckIneffectiveMarkCompact(size_t old_generation_size,
                                    double mutator_utilization);
 
-  inline void IncrementExternalBackingStoreBytes(ExternalBackingStoreType type,
-                                                 size_t amount);
-
-  inline void DecrementExternalBackingStoreBytes(ExternalBackingStoreType type,
-                                                 size_t amount);
-
   // ===========================================================================
   // Growing strategy. =========================================================
   // ===========================================================================
@@ -2018,9 +2011,6 @@ class Heap {
   bool old_generation_size_configured_;
   size_t maximum_committed_;
 
-  // Backing store bytes (array buffers and external strings).
-  std::atomic<size_t> backing_store_bytes_;
-
   // For keeping track of how much data has survived
   // scavenge since last new space expansion.
   size_t survived_since_last_expansion_;
@@ -2296,7 +2286,6 @@ class Heap {
   friend class Page;
   friend class PagedSpace;
   friend class Scavenger;
-  friend class Space;
   friend class StoreBuffer;
   friend class Sweeper;
   friend class heap::TestMemoryAllocatorScope;

src/heap/mark-compact.cc

@@ -2274,11 +2274,9 @@ static String* UpdateReferenceInExternalStringTableEntry(Heap* heap,
     String* new_string = String::cast(map_word.ToForwardingAddress());
 
     if (new_string->IsExternalString()) {
-      MemoryChunk::MoveExternalBackingStoreBytes(
-          ExternalBackingStoreType::kExternalString,
+      heap->ProcessMovedExternalString(
           Page::FromAddress(reinterpret_cast<Address>(*p)),
-          Page::FromHeapObject(new_string),
-          ExternalString::cast(new_string)->ExternalPayloadSize());
+          Page::FromHeapObject(new_string), ExternalString::cast(new_string));
     }
     return new_string;
   }

src/heap/spaces-inl.h

@@ -5,7 +5,6 @@
 #ifndef V8_HEAP_SPACES_INL_H_
 #define V8_HEAP_SPACES_INL_H_
 
-#include "src/base/atomic-utils.h"
 #include "src/base/bounded-page-allocator.h"
 #include "src/base/v8-fallthrough.h"
 #include "src/heap/incremental-marking.h"
@@ -94,27 +93,6 @@ HeapObject* HeapObjectIterator::FromCurrentPage() {
   return nullptr;
 }
 
-void Space::IncrementExternalBackingStoreBytes(ExternalBackingStoreType type,
-                                               size_t amount) {
-  base::CheckedIncrement(&external_backing_store_bytes_[type], amount);
-  heap()->IncrementExternalBackingStoreBytes(type, amount);
-}
-
-void Space::DecrementExternalBackingStoreBytes(ExternalBackingStoreType type,
-                                               size_t amount) {
-  base::CheckedDecrement(&external_backing_store_bytes_[type], amount);
-  heap()->DecrementExternalBackingStoreBytes(type, amount);
-}
-
-void Space::MoveExternalBackingStoreBytes(ExternalBackingStoreType type,
-                                          Space* from, Space* to,
-                                          size_t amount) {
-  if (from == to) return;
-
-  base::CheckedDecrement(&(from->external_backing_store_bytes_[type]), amount);
-  base::CheckedIncrement(&(to->external_backing_store_bytes_[type]), amount);
-}
-
 // -----------------------------------------------------------------------------
 // SemiSpace
 
@@ -212,28 +190,6 @@ MemoryChunk* MemoryChunk::FromAnyPointerAddress(Heap* heap, Address addr) {
   return chunk;
 }
 
-void MemoryChunk::IncrementExternalBackingStoreBytes(
-    ExternalBackingStoreType type, size_t amount) {
-  base::CheckedIncrement(&external_backing_store_bytes_[type], amount);
-  owner()->IncrementExternalBackingStoreBytes(type, amount);
-}
-
-void MemoryChunk::DecrementExternalBackingStoreBytes(
-    ExternalBackingStoreType type, size_t amount) {
-  base::CheckedDecrement(&external_backing_store_bytes_[type], amount);
-  owner()->DecrementExternalBackingStoreBytes(type, amount);
-}
-
-void MemoryChunk::MoveExternalBackingStoreBytes(ExternalBackingStoreType type,
-                                                MemoryChunk* from,
-                                                MemoryChunk* to,
-                                                size_t amount) {
-  base::CheckedDecrement(&(from->external_backing_store_bytes_[type]), amount);
-  base::CheckedIncrement(&(to->external_backing_store_bytes_[type]), amount);
-  Space::MoveExternalBackingStoreBytes(type, from->owner(), to->owner(),
-                                       amount);
-}
-
 void Page::MarkNeverAllocateForTesting() {
   DCHECK(this->owner()->identity() != NEW_SPACE);
   DCHECK(!IsFlagSet(NEVER_ALLOCATE_ON_PAGE));

src/heap/spaces.cc

@@ -1308,6 +1308,19 @@ 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
 

src/heap/spaces.h

@@ -143,6 +143,12 @@ enum FreeMode { kLinkCategory, kDoNotLinkCategory };
 
 enum class SpaceAccountingMode { kSpaceAccounted, kSpaceUnaccounted };
 
+enum ExternalBackingStoreType {
+  kArrayBuffer,
+  kExternalString,
+  kNumTypes
+};
+
 enum RememberedSetType {
   OLD_TO_NEW,
   OLD_TO_OLD,
@@ -373,7 +379,7 @@ class MemoryChunk {
       kPointerSize  // std::atomic<ConcurrentSweepingState> concurrent_sweeping_
       + kPointerSize  // base::Mutex* page_protection_change_mutex_
       + kPointerSize  // unitptr_t write_unprotect_counter_
-      + kSizetSize * ExternalBackingStoreType::kNumTypes
+      + kSizetSize * kNumTypes
       // std::atomic<size_t> external_backing_store_bytes_
       + kSizetSize        // size_t allocated_bytes_
       + kSizetSize        // size_t wasted_memory_
@@ -439,10 +445,6 @@ class MemoryChunk {
         !chunk->high_water_mark_.compare_exchange_weak(old_mark, new_mark));
   }
 
-  static inline void MoveExternalBackingStoreBytes(
-      ExternalBackingStoreType type, MemoryChunk* from, MemoryChunk* to,
-      size_t amount);
-
   Address address() const {
     return reinterpret_cast<Address>(const_cast<MemoryChunk*>(this));
   }
@@ -549,12 +551,10 @@ class MemoryChunk {
     }
   }
 
-  inline void IncrementExternalBackingStoreBytes(ExternalBackingStoreType type,
-                                                 size_t amount);
-
-  inline void DecrementExternalBackingStoreBytes(ExternalBackingStoreType type,
-                                                 size_t amount);
-
+  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];
   }
@@ -945,9 +945,6 @@ class Space : public Malloced {
         0;
   }
 
-  static inline void MoveExternalBackingStoreBytes(
-      ExternalBackingStoreType type, Space* from, Space* to, size_t amount);
-
   virtual ~Space() {
     delete[] external_backing_store_bytes_;
     external_backing_store_bytes_ = nullptr;
@@ -987,6 +984,12 @@ class Space : public Malloced {
   // (e.g. see LargeObjectSpace).
   virtual size_t SizeOfObjects() { return Size(); }
 
+  // Returns amount of off-heap memory in-use by objects in this Space.
+  virtual size_t ExternalBackingStoreBytes(
+      ExternalBackingStoreType type) const {
+    return external_backing_store_bytes_[type];
+  }
+
   // Approximate amount of physical memory committed for this space.
   virtual size_t CommittedPhysicalMemory() = 0;
 
@@ -1016,16 +1019,14 @@ class Space : public Malloced {
     committed_ -= bytes;
   }
 
-  inline void IncrementExternalBackingStoreBytes(ExternalBackingStoreType type,
-                                                 size_t amount);
-
-  inline void DecrementExternalBackingStoreBytes(ExternalBackingStoreType type,
-                                                 size_t amount);
-
-  // Returns amount of off-heap memory in-use by objects in this Space.
-  virtual size_t ExternalBackingStoreBytes(
-      ExternalBackingStoreType type) const {
-    return external_backing_store_bytes_[type];
+  void IncrementExternalBackingStoreBytes(ExternalBackingStoreType type,
+                                          size_t amount) {
+    external_backing_store_bytes_[type] += amount;
+  }
+  void DecrementExternalBackingStoreBytes(ExternalBackingStoreType type,
+                                          size_t amount) {
+    DCHECK_GE(external_backing_store_bytes_[type], amount);
+    external_backing_store_bytes_[type] -= amount;
   }
 
   V8_EXPORT_PRIVATE void* GetRandomMmapAddr();