[gc] Expose ExternalMemoryAccounter publicly

To replace AdjustAmountOfExternalAllocatedMemory.

Bug: 42203776
Change-Id: Ib7bde62ce953832c5631a57a39000e849b9a6b89
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6015340
Reviewed-by: Michael Lippautz <[email protected]>
Commit-Queue: Etienne Pierre-Doray <[email protected]>
Cr-Commit-Position: refs/heads/main@{#97427}

Author: etiennep-chromium

BUILD.bazel

@@ -671,6 +671,7 @@ filegroup(
         "include/v8-embedder-state-scope.h",
         "include/v8-exception.h",
         "include/v8-extension.h",
+        "include/v8-external-memory-accounter.h",
         "include/v8-external.h",
         "include/v8-fast-api-calls.h",
         "include/v8-forward.h",

BUILD.gn

@@ -151,6 +151,9 @@ declare_args() {
   # Sets -DV8_ENABLE_CHECKS.
   v8_enable_v8_checks = ""
 
+  # Sets -DV8_ENABLE_MEMORY_ACCOUNTING_CHECKS
+  v8_enable_memory_accounting_checks = ""
+
   # Sets -DV8_TRACE_UNOPTIMIZED.
   v8_enable_trace_unoptimized = ""
   v8_enable_trace_ignition = false
@@ -478,6 +481,10 @@ if (v8_enable_test_features == "") {
 if (v8_enable_v8_checks == "") {
   v8_enable_v8_checks = v8_enable_debugging_features
 }
+if (v8_enable_memory_accounting_checks == "") {
+  v8_enable_memory_accounting_checks =
+      v8_enable_debugging_features || v8_dcheck_always_on
+}
 if (v8_enable_heap_snapshot_verify == "") {
   v8_enable_heap_snapshot_verify =
       v8_enable_debugging_features || v8_dcheck_always_on
@@ -904,6 +911,7 @@ external_v8_defines = [
   "V8_ARRAY_BUFFER_VIEW_INTERNAL_FIELD_COUNT=${v8_array_buffer_view_internal_field_count}",
   "V8_PROMISE_INTERNAL_FIELD_COUNT=${v8_promise_internal_field_count}",
   "V8_ENABLE_CHECKS",
+  "V8_ENABLE_MEMORY_ACCOUNTING_CHECKS",
   "V8_COMPRESS_POINTERS",
   "V8_COMPRESS_POINTERS_IN_SHARED_CAGE",
   "V8_31BIT_SMIS_ON_64BIT_ARCH",
@@ -936,6 +944,9 @@ enabled_external_v8_defines = [
 if (v8_enable_v8_checks) {
   enabled_external_v8_defines += [ "V8_ENABLE_CHECKS" ]
 }
+if (v8_enable_memory_accounting_checks) {
+  enabled_external_v8_defines += [ "V8_ENABLE_MEMORY_ACCOUNTING_CHECKS" ]
+}
 if (v8_enable_pointer_compression) {
   enabled_external_v8_defines += [ "V8_COMPRESS_POINTERS" ]
   if (v8_enable_pointer_compression_shared_cage) {
@@ -2910,6 +2921,7 @@ generated_file("v8_generate_features_json") {
     v8_enable_sandbox = v8_enable_sandbox
     v8_enable_short_builtin_calls = v8_enable_short_builtin_calls
     v8_enable_v8_checks = v8_enable_v8_checks
+    v8_enable_memory_accounting_checks = v8_enable_memory_accounting_checks
     v8_enable_webassembly = v8_enable_webassembly
     v8_enable_zone_compression = v8_enable_zone_compression
     v8_imminent_deprecation_warnings = v8_imminent_deprecation_warnings
@@ -3222,6 +3234,7 @@ v8_header_set("v8_headers") {
     "include/v8-embedder-state-scope.h",
     "include/v8-exception.h",
     "include/v8-extension.h",
+    "include/v8-external-memory-accounter.h",
     "include/v8-external.h",
     "include/v8-fast-api-calls.h",
     "include/v8-forward.h",

include/v8-external-memory-accounter.h

@@ -0,0 +1,60 @@
+// Copyright 2024 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef INCLUDE_EXTERNAL_MEMORY_ACCOUNTER_H_
+#define INCLUDE_EXTERNAL_MEMORY_ACCOUNTER_H_
+
+#include <stdint.h>
+
+#include "v8-isolate.h"
+
+namespace v8 {
+
+/**
+ * This class is used to give V8 an indication of the amount of externally
+ * allocated memory that is kept alive by JavaScript objects. V8 uses this to
+ * decide when to perform garbage collections. Registering externally allocated
+ * memory will trigger garbage collections more often than it would otherwise in
+ * an attempt to garbage collect the JavaScript objects that keep the externally
+ * allocated memory alive. Instances of ExternalMemoryAccounter check that the
+ * reported external memory is back to 0 on destruction.
+ */
+class V8_EXPORT ExternalMemoryAccounter {
+ public:
+  /**
+   * Returns the amount of external memory registered for `isolate`.
+   */
+  static int64_t GetTotalAmountOfExternalAllocatedMemoryForTesting(
+      const Isolate* isolate);
+
+  ExternalMemoryAccounter() = default;
+  ~ExternalMemoryAccounter();
+  ExternalMemoryAccounter(ExternalMemoryAccounter&&);
+  ExternalMemoryAccounter& operator=(ExternalMemoryAccounter&&);
+  ExternalMemoryAccounter(const ExternalMemoryAccounter&) = delete;
+  ExternalMemoryAccounter& operator=(const ExternalMemoryAccounter&) = delete;
+
+  /**
+   * Reports an increase of `size` bytes of external memory.
+   */
+  void Increase(Isolate* isolate, size_t size);
+  /**
+   * Reports an update of `delta` bytes of external memory.
+   */
+  void Update(Isolate* isolate, int64_t delta);
+  /**
+   * Reports an decrease of `size` bytes of external memory.
+   */
+  void Decrease(Isolate* isolate, size_t size);
+
+ private:
+#ifdef V8_ENABLE_MEMORY_ACCOUNTING_CHECKS
+  size_t amount_of_external_memory_ = 0;
+  v8::Isolate* isolate_ = nullptr;
+#endif
+};
+
+}  // namespace v8
+
+#endif  // INCLUDE_EXTERNAL_MEMORY_ACCOUNTER_H_

include/v8-isolate.h

@@ -889,18 +889,13 @@ class V8_EXPORT Isolate {
                       size_t frames_limit, SampleInfo* sample_info);
 
   /**
-   * Adjusts the amount of registered external memory. Used to give V8 an
-   * indication of the amount of externally allocated memory that is kept alive
-   * by JavaScript objects. V8 uses this to decide when to perform global
-   * garbage collections. Registering externally allocated memory will trigger
-   * global garbage collections more often than it would otherwise in an attempt
-   * to garbage collect the JavaScript objects that keep the externally
-   * allocated memory alive.
+   * Adjusts the amount of registered external memory.
    *
    * \param change_in_bytes the change in externally allocated memory that is
    *   kept alive by JavaScript objects.
    * \returns the adjusted value.
    */
+  V8_DEPRECATE_SOON("Use ExternalMemoryAccounter instead.")
   int64_t AdjustAmountOfExternalAllocatedMemory(int64_t change_in_bytes);
 
   /**
@@ -1755,9 +1750,11 @@ class V8_EXPORT Isolate {
  private:
   template <class K, class V, class Traits>
   friend class PersistentValueMapBase;
+  friend class ExternalMemoryAccounter;
 
   internal::ValueHelper::InternalRepresentationType GetDataFromSnapshotOnce(
       size_t index);
+  int64_t AdjustAmountOfExternalAllocatedMemoryImpl(int64_t change_in_bytes);
   void HandleExternalMemoryInterrupt();
 };
 

src/api/api.cc

@@ -19,6 +19,7 @@
 #include "include/v8-date.h"
 #include "include/v8-embedder-state-scope.h"
 #include "include/v8-extension.h"
+#include "include/v8-external-memory-accounter.h"
 #include "include/v8-fast-api-calls.h"
 #include "include/v8-function.h"
 #include "include/v8-json.h"
@@ -9728,6 +9729,29 @@ void BigInt::ToWordsArray(int* sign_bit, int* word_count,
   *word_count = base::checked_cast<int>(unsigned_word_count);
 }
 
+int64_t Isolate::AdjustAmountOfExternalAllocatedMemoryImpl(
+    int64_t change_in_bytes) {
+  // Try to check for unreasonably large or small values from the embedder.
+  static constexpr int64_t kMaxReasonableBytes = int64_t(1) << 60;
+  static constexpr int64_t kMinReasonableBytes = -kMaxReasonableBytes;
+  static_assert(kMaxReasonableBytes >= i::JSArrayBuffer::kMaxByteLength);
+  CHECK(kMinReasonableBytes <= change_in_bytes &&
+        change_in_bytes < kMaxReasonableBytes);
+
+  i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(this);
+  const uint64_t amount =
+      i_isolate->heap()->UpdateExternalMemory(change_in_bytes);
+
+  if (change_in_bytes <= 0) {
+    return amount;
+  }
+
+  if (amount > i_isolate->heap()->external_memory_limit_for_interrupt()) {
+    HandleExternalMemoryInterrupt();
+  }
+  return amount;
+}
+
 void Isolate::HandleExternalMemoryInterrupt() {
   i::Heap* heap = reinterpret_cast<i::Isolate*>(this)->heap();
   if (heap->gc_state() != i::Heap::NOT_IN_GC) return;
@@ -10437,25 +10461,7 @@ void Isolate::GetStackSample(const RegisterState& state, void** frames,
 
 int64_t Isolate::AdjustAmountOfExternalAllocatedMemory(
     int64_t change_in_bytes) {
-  // Try to check for unreasonably large or small values from the embedder.
-  static constexpr int64_t kMaxReasonableBytes = int64_t(1) << 60;
-  static constexpr int64_t kMinReasonableBytes = -kMaxReasonableBytes;
-  static_assert(kMaxReasonableBytes >= i::JSArrayBuffer::kMaxByteLength);
-  CHECK(kMinReasonableBytes <= change_in_bytes &&
-        change_in_bytes < kMaxReasonableBytes);
-
-  i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(this);
-  const uint64_t amount =
-      i_isolate->heap()->UpdateExternalMemory(change_in_bytes);
-
-  if (change_in_bytes <= 0) {
-    return amount;
-  }
-
-  if (amount > i_isolate->heap()->external_memory_limit_for_interrupt()) {
-    HandleExternalMemoryInterrupt();
-  }
-  return amount;
+  return AdjustAmountOfExternalAllocatedMemoryImpl(change_in_bytes);
 }
 
 void Isolate::SetEventLogger(LogEventCallback that) {
@@ -12336,24 +12342,26 @@ bool V8_EXPORT ValidateCallbackInfo(const PropertyCallbackInfo<void>& info) {
   return ValidatePropertyCallbackInfo(info);
 }
 
-ExternalMemoryAccounterBase::~ExternalMemoryAccounterBase() {
-#ifdef DEBUG
+}  // namespace internal
+
+ExternalMemoryAccounter::~ExternalMemoryAccounter() {
+#ifdef V8_ENABLE_MEMORY_ACCOUNTING_CHECKS
   DCHECK_EQ(amount_of_external_memory_, 0U);
 #endif
 }
 
-ExternalMemoryAccounterBase::ExternalMemoryAccounterBase(
-    ExternalMemoryAccounterBase&& other) V8_NOEXCEPT {
-#if DEBUG
+ExternalMemoryAccounter::ExternalMemoryAccounter(
+    ExternalMemoryAccounter&& other) {
+#if V8_ENABLE_MEMORY_ACCOUNTING_CHECKS
   amount_of_external_memory_ =
       std::exchange(other.amount_of_external_memory_, 0U);
   isolate_ = std::exchange(other.isolate_, nullptr);
 #endif
 }
 
-ExternalMemoryAccounterBase& ExternalMemoryAccounterBase::operator=(
-    ExternalMemoryAccounterBase&& other) V8_NOEXCEPT {
-#if DEBUG
+ExternalMemoryAccounter& ExternalMemoryAccounter::operator=(
+    ExternalMemoryAccounter&& other) {
+#if V8_ENABLE_MEMORY_ACCOUNTING_CHECKS
   if (this == &other) {
     return *this;
   }
@@ -12365,33 +12373,32 @@ ExternalMemoryAccounterBase& ExternalMemoryAccounterBase::operator=(
   return *this;
 }
 
-void ExternalMemoryAccounterBase::Increase(Isolate* isolate, size_t size) {
-#ifdef DEBUG
+void ExternalMemoryAccounter::Increase(Isolate* isolate, size_t size) {
+#ifdef V8_ENABLE_MEMORY_ACCOUNTING_CHECKS
   DCHECK(isolate == isolate_ || isolate_ == nullptr);
   isolate_ = isolate;
   amount_of_external_memory_ += size;
 #endif
-  reinterpret_cast<v8::Isolate*>(isolate)
-      ->AdjustAmountOfExternalAllocatedMemory(static_cast<int64_t>(size));
+  isolate->AdjustAmountOfExternalAllocatedMemoryImpl(
+      static_cast<int64_t>(size));
 }
 
-void ExternalMemoryAccounterBase::Update(Isolate* isolate, int64_t delta) {
-#ifdef DEBUG
+void ExternalMemoryAccounter::Update(Isolate* isolate, int64_t delta) {
+#ifdef V8_ENABLE_MEMORY_ACCOUNTING_CHECKS
   DCHECK(isolate == isolate_ || isolate_ == nullptr);
   DCHECK_GE(static_cast<int64_t>(amount_of_external_memory_), -delta);
   isolate_ = isolate;
   amount_of_external_memory_ += delta;
 #endif
-  reinterpret_cast<v8::Isolate*>(isolate)
-      ->AdjustAmountOfExternalAllocatedMemory(delta);
+  isolate->AdjustAmountOfExternalAllocatedMemoryImpl(delta);
 }
 
-void ExternalMemoryAccounterBase::Decrease(Isolate* isolate, size_t size) {
-  DisallowGarbageCollection no_gc;
+void ExternalMemoryAccounter::Decrease(Isolate* isolate, size_t size) {
+  internal::DisallowGarbageCollection no_gc;
   if (size == 0) {
     return;
   }
-#ifdef DEBUG
+#ifdef V8_ENABLE_MEMORY_ACCOUNTING_CHECKS
   DCHECK_EQ(isolate, isolate_);
   DCHECK_GE(amount_of_external_memory_, size);
   amount_of_external_memory_ -= size;
@@ -12400,7 +12407,12 @@ void ExternalMemoryAccounterBase::Decrease(Isolate* isolate, size_t size) {
   i_isolate->heap()->UpdateExternalMemory(-static_cast<int64_t>(size));
 }
 
-}  // namespace internal
+int64_t
+ExternalMemoryAccounter::GetTotalAmountOfExternalAllocatedMemoryForTesting(
+    const Isolate* isolate) {
+  const i::Isolate* i_isolate = reinterpret_cast<const i::Isolate*>(isolate);
+  return i_isolate->heap()->external_memory();
+}
 
 template <>
 bool V8_EXPORT V8_WARN_UNUSED_RESULT

src/api/api.h

@@ -549,31 +549,6 @@ bool ValidateCallbackInfo(const PropertyCallbackInfo<T>& info);
 
 DECLARE_CONTEXTUAL_VARIABLE_WITH_DEFAULT(StackAllocatedCheck, const bool, true);
 
-// TODO(crbug.com/42203776): This should move to the API and be integrated into
-// `AdjustAmountOfExternalAllocatedMemory()` to make sure there are no
-// unbalanced bytes floating around.
-class V8_EXPORT_PRIVATE ExternalMemoryAccounterBase {
- public:
-  ExternalMemoryAccounterBase() = default;
-  ~ExternalMemoryAccounterBase();
-  ExternalMemoryAccounterBase(ExternalMemoryAccounterBase&&) V8_NOEXCEPT;
-  ExternalMemoryAccounterBase& operator=(ExternalMemoryAccounterBase&&)
-      V8_NOEXCEPT;
-  ExternalMemoryAccounterBase(const ExternalMemoryAccounterBase&) = delete;
-  ExternalMemoryAccounterBase& operator=(const ExternalMemoryAccounterBase&) =
-      delete;
-
-  void Increase(Isolate* isolate, size_t size);
-  void Update(Isolate* isolate, int64_t delta);
-  void Decrease(Isolate* isolate, size_t size);
-
- private:
-#ifdef DEBUG
-  size_t amount_of_external_memory_ = 0;
-  Isolate* isolate_ = nullptr;
-#endif
-};
-
 }  // namespace internal
 }  // namespace v8
 

src/execution/isolate.cc

@@ -3671,7 +3671,8 @@ void Isolate::ReleaseSharedPtrs() {
     ManagedPtrDestructor* n = nullptr;
     managed_ptr_destructors_head_ = nullptr;
     for (; l != nullptr; l = n) {
-      l->external_memory_accounter_.Decrease(this, l->estimated_size_);
+      l->external_memory_accounter_.Decrease(
+          reinterpret_cast<v8::Isolate*>(this), l->estimated_size_);
       l->destructor_(l->shared_ptr_ptr_);
       n = l->next_;
       delete l;

src/heap/array-buffer-sweeper.cc

@@ -422,14 +422,16 @@ void ArrayBufferSweeper::IncrementExternalMemoryCounters(size_t bytes) {
   if (bytes == 0) return;
   heap_->IncrementExternalBackingStoreBytes(
       ExternalBackingStoreType::kArrayBuffer, bytes);
-  external_memory_accounter_.Increase(heap_->isolate(), bytes);
+  external_memory_accounter_.Increase(
+      reinterpret_cast<v8::Isolate*>(heap_->isolate()), bytes);
 }
 
 void ArrayBufferSweeper::DecrementExternalMemoryCounters(size_t bytes) {
   if (bytes == 0) return;
   heap_->DecrementExternalBackingStoreBytes(
       ExternalBackingStoreType::kArrayBuffer, bytes);
-  external_memory_accounter_.Decrease(heap_->isolate(), bytes);
+  external_memory_accounter_.Decrease(
+      reinterpret_cast<v8::Isolate*>(heap_->isolate()), bytes);
 }
 
 void ArrayBufferSweeper::FinalizeAndDelete(ArrayBufferExtension* extension) {

src/heap/array-buffer-sweeper.h

@@ -7,6 +7,7 @@
 
 #include <memory>
 
+#include "include/v8-external-memory-accounter.h"
 #include "include/v8config.h"
 #include "src/api/api.h"
 #include "src/base/logging.h"
@@ -115,7 +116,7 @@ class ArrayBufferSweeper final {
   // finishes.
   int64_t young_bytes_adjustment_while_sweeping_{0};
   int64_t old_bytes_adjustment_while_sweeping_{0};
-  V8_NO_UNIQUE_ADDRESS ExternalMemoryAccounterBase external_memory_accounter_;
+  V8_NO_UNIQUE_ADDRESS ExternalMemoryAccounter external_memory_accounter_;
 };
 
 }  // namespace internal