Skip to content

Commit 7dc4c18

Browse files
etiennep-chromiumV8 LUCI CQ
etiennep-chromium
authored and
V8 LUCI CQ
committed
[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}
1 parent 518473b commit 7dc4c18

16 files changed

Lines changed: 177 additions & 97 deletions

‎BUILD.bazel‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -671,6 +671,7 @@ filegroup(
671671
"include/v8-embedder-state-scope.h",
672672
"include/v8-exception.h",
673673
"include/v8-extension.h",
674+
"include/v8-external-memory-accounter.h",
674675
"include/v8-external.h",
675676
"include/v8-fast-api-calls.h",
676677
"include/v8-forward.h",

‎BUILD.gn‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,9 @@ declare_args() {
151151
# Sets -DV8_ENABLE_CHECKS.
152152
v8_enable_v8_checks = ""
153153

154+
# Sets -DV8_ENABLE_MEMORY_ACCOUNTING_CHECKS
155+
v8_enable_memory_accounting_checks = ""
156+
154157
# Sets -DV8_TRACE_UNOPTIMIZED.
155158
v8_enable_trace_unoptimized = ""
156159
v8_enable_trace_ignition = false
@@ -478,6 +481,10 @@ if (v8_enable_test_features == "") {
478481
if (v8_enable_v8_checks == "") {
479482
v8_enable_v8_checks = v8_enable_debugging_features
480483
}
484+
if (v8_enable_memory_accounting_checks == "") {
485+
v8_enable_memory_accounting_checks =
486+
v8_enable_debugging_features || v8_dcheck_always_on
487+
}
481488
if (v8_enable_heap_snapshot_verify == "") {
482489
v8_enable_heap_snapshot_verify =
483490
v8_enable_debugging_features || v8_dcheck_always_on
@@ -904,6 +911,7 @@ external_v8_defines = [
904911
"V8_ARRAY_BUFFER_VIEW_INTERNAL_FIELD_COUNT=${v8_array_buffer_view_internal_field_count}",
905912
"V8_PROMISE_INTERNAL_FIELD_COUNT=${v8_promise_internal_field_count}",
906913
"V8_ENABLE_CHECKS",
914+
"V8_ENABLE_MEMORY_ACCOUNTING_CHECKS",
907915
"V8_COMPRESS_POINTERS",
908916
"V8_COMPRESS_POINTERS_IN_SHARED_CAGE",
909917
"V8_31BIT_SMIS_ON_64BIT_ARCH",
@@ -936,6 +944,9 @@ enabled_external_v8_defines = [
936944
if (v8_enable_v8_checks) {
937945
enabled_external_v8_defines += [ "V8_ENABLE_CHECKS" ]
938946
}
947+
if (v8_enable_memory_accounting_checks) {
948+
enabled_external_v8_defines += [ "V8_ENABLE_MEMORY_ACCOUNTING_CHECKS" ]
949+
}
939950
if (v8_enable_pointer_compression) {
940951
enabled_external_v8_defines += [ "V8_COMPRESS_POINTERS" ]
941952
if (v8_enable_pointer_compression_shared_cage) {
@@ -2910,6 +2921,7 @@ generated_file("v8_generate_features_json") {
29102921
v8_enable_sandbox = v8_enable_sandbox
29112922
v8_enable_short_builtin_calls = v8_enable_short_builtin_calls
29122923
v8_enable_v8_checks = v8_enable_v8_checks
2924+
v8_enable_memory_accounting_checks = v8_enable_memory_accounting_checks
29132925
v8_enable_webassembly = v8_enable_webassembly
29142926
v8_enable_zone_compression = v8_enable_zone_compression
29152927
v8_imminent_deprecation_warnings = v8_imminent_deprecation_warnings
@@ -3222,6 +3234,7 @@ v8_header_set("v8_headers") {
32223234
"include/v8-embedder-state-scope.h",
32233235
"include/v8-exception.h",
32243236
"include/v8-extension.h",
3237+
"include/v8-external-memory-accounter.h",
32253238
"include/v8-external.h",
32263239
"include/v8-fast-api-calls.h",
32273240
"include/v8-forward.h",

‎include/v8-external-memory-accounter.h‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
// Copyright 2024 the V8 project authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
#ifndef INCLUDE_EXTERNAL_MEMORY_ACCOUNTER_H_
6+
#define INCLUDE_EXTERNAL_MEMORY_ACCOUNTER_H_
7+
8+
#include <stdint.h>
9+
10+
#include "v8-isolate.h"
11+
12+
namespace v8 {
13+
14+
/**
15+
* This class is used to give V8 an indication of the amount of externally
16+
* allocated memory that is kept alive by JavaScript objects. V8 uses this to
17+
* decide when to perform garbage collections. Registering externally allocated
18+
* memory will trigger garbage collections more often than it would otherwise in
19+
* an attempt to garbage collect the JavaScript objects that keep the externally
20+
* allocated memory alive. Instances of ExternalMemoryAccounter check that the
21+
* reported external memory is back to 0 on destruction.
22+
*/
23+
class V8_EXPORT ExternalMemoryAccounter {
24+
public:
25+
/**
26+
* Returns the amount of external memory registered for `isolate`.
27+
*/
28+
static int64_t GetTotalAmountOfExternalAllocatedMemoryForTesting(
29+
const Isolate* isolate);
30+
31+
ExternalMemoryAccounter() = default;
32+
~ExternalMemoryAccounter();
33+
ExternalMemoryAccounter(ExternalMemoryAccounter&&);
34+
ExternalMemoryAccounter& operator=(ExternalMemoryAccounter&&);
35+
ExternalMemoryAccounter(const ExternalMemoryAccounter&) = delete;
36+
ExternalMemoryAccounter& operator=(const ExternalMemoryAccounter&) = delete;
37+
38+
/**
39+
* Reports an increase of `size` bytes of external memory.
40+
*/
41+
void Increase(Isolate* isolate, size_t size);
42+
/**
43+
* Reports an update of `delta` bytes of external memory.
44+
*/
45+
void Update(Isolate* isolate, int64_t delta);
46+
/**
47+
* Reports an decrease of `size` bytes of external memory.
48+
*/
49+
void Decrease(Isolate* isolate, size_t size);
50+
51+
private:
52+
#ifdef V8_ENABLE_MEMORY_ACCOUNTING_CHECKS
53+
size_t amount_of_external_memory_ = 0;
54+
v8::Isolate* isolate_ = nullptr;
55+
#endif
56+
};
57+
58+
} // namespace v8
59+
60+
#endif // INCLUDE_EXTERNAL_MEMORY_ACCOUNTER_H_

‎include/v8-isolate.h‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -889,18 +889,13 @@ class V8_EXPORT Isolate {
889889
size_t frames_limit, SampleInfo* sample_info);
890890

891891
/**
892-
* Adjusts the amount of registered external memory. Used to give V8 an
893-
* indication of the amount of externally allocated memory that is kept alive
894-
* by JavaScript objects. V8 uses this to decide when to perform global
895-
* garbage collections. Registering externally allocated memory will trigger
896-
* global garbage collections more often than it would otherwise in an attempt
897-
* to garbage collect the JavaScript objects that keep the externally
898-
* allocated memory alive.
892+
* Adjusts the amount of registered external memory.
899893
*
900894
* \param change_in_bytes the change in externally allocated memory that is
901895
* kept alive by JavaScript objects.
902896
* \returns the adjusted value.
903897
*/
898+
V8_DEPRECATE_SOON("Use ExternalMemoryAccounter instead.")
904899
int64_t AdjustAmountOfExternalAllocatedMemory(int64_t change_in_bytes);
905900

906901
/**
@@ -1755,9 +1750,11 @@ class V8_EXPORT Isolate {
17551750
private:
17561751
template <class K, class V, class Traits>
17571752
friend class PersistentValueMapBase;
1753+
friend class ExternalMemoryAccounter;
17581754

17591755
internal::ValueHelper::InternalRepresentationType GetDataFromSnapshotOnce(
17601756
size_t index);
1757+
int64_t AdjustAmountOfExternalAllocatedMemoryImpl(int64_t change_in_bytes);
17611758
void HandleExternalMemoryInterrupt();
17621759
};
17631760

‎src/api/api.cc‎

Lines changed: 51 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
#include "include/v8-date.h"
2020
#include "include/v8-embedder-state-scope.h"
2121
#include "include/v8-extension.h"
22+
#include "include/v8-external-memory-accounter.h"
2223
#include "include/v8-fast-api-calls.h"
2324
#include "include/v8-function.h"
2425
#include "include/v8-json.h"
@@ -9728,6 +9729,29 @@ void BigInt::ToWordsArray(int* sign_bit, int* word_count,
97289729
*word_count = base::checked_cast<int>(unsigned_word_count);
97299730
}
97309731

9732+
int64_t Isolate::AdjustAmountOfExternalAllocatedMemoryImpl(
9733+
int64_t change_in_bytes) {
9734+
// Try to check for unreasonably large or small values from the embedder.
9735+
static constexpr int64_t kMaxReasonableBytes = int64_t(1) << 60;
9736+
static constexpr int64_t kMinReasonableBytes = -kMaxReasonableBytes;
9737+
static_assert(kMaxReasonableBytes >= i::JSArrayBuffer::kMaxByteLength);
9738+
CHECK(kMinReasonableBytes <= change_in_bytes &&
9739+
change_in_bytes < kMaxReasonableBytes);
9740+
9741+
i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(this);
9742+
const uint64_t amount =
9743+
i_isolate->heap()->UpdateExternalMemory(change_in_bytes);
9744+
9745+
if (change_in_bytes <= 0) {
9746+
return amount;
9747+
}
9748+
9749+
if (amount > i_isolate->heap()->external_memory_limit_for_interrupt()) {
9750+
HandleExternalMemoryInterrupt();
9751+
}
9752+
return amount;
9753+
}
9754+
97319755
void Isolate::HandleExternalMemoryInterrupt() {
97329756
i::Heap* heap = reinterpret_cast<i::Isolate*>(this)->heap();
97339757
if (heap->gc_state() != i::Heap::NOT_IN_GC) return;
@@ -10437,25 +10461,7 @@ void Isolate::GetStackSample(const RegisterState& state, void** frames,
1043710461

1043810462
int64_t Isolate::AdjustAmountOfExternalAllocatedMemory(
1043910463
int64_t change_in_bytes) {
10440-
// Try to check for unreasonably large or small values from the embedder.
10441-
static constexpr int64_t kMaxReasonableBytes = int64_t(1) << 60;
10442-
static constexpr int64_t kMinReasonableBytes = -kMaxReasonableBytes;
10443-
static_assert(kMaxReasonableBytes >= i::JSArrayBuffer::kMaxByteLength);
10444-
CHECK(kMinReasonableBytes <= change_in_bytes &&
10445-
change_in_bytes < kMaxReasonableBytes);
10446-
10447-
i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(this);
10448-
const uint64_t amount =
10449-
i_isolate->heap()->UpdateExternalMemory(change_in_bytes);
10450-
10451-
if (change_in_bytes <= 0) {
10452-
return amount;
10453-
}
10454-
10455-
if (amount > i_isolate->heap()->external_memory_limit_for_interrupt()) {
10456-
HandleExternalMemoryInterrupt();
10457-
}
10458-
return amount;
10464+
return AdjustAmountOfExternalAllocatedMemoryImpl(change_in_bytes);
1045910465
}
1046010466

1046110467
void Isolate::SetEventLogger(LogEventCallback that) {
@@ -12336,24 +12342,26 @@ bool V8_EXPORT ValidateCallbackInfo(const PropertyCallbackInfo<void>& info) {
1233612342
return ValidatePropertyCallbackInfo(info);
1233712343
}
1233812344

12339-
ExternalMemoryAccounterBase::~ExternalMemoryAccounterBase() {
12340-
#ifdef DEBUG
12345+
} // namespace internal
12346+
12347+
ExternalMemoryAccounter::~ExternalMemoryAccounter() {
12348+
#ifdef V8_ENABLE_MEMORY_ACCOUNTING_CHECKS
1234112349
DCHECK_EQ(amount_of_external_memory_, 0U);
1234212350
#endif
1234312351
}
1234412352

12345-
ExternalMemoryAccounterBase::ExternalMemoryAccounterBase(
12346-
ExternalMemoryAccounterBase&& other) V8_NOEXCEPT {
12347-
#if DEBUG
12353+
ExternalMemoryAccounter::ExternalMemoryAccounter(
12354+
ExternalMemoryAccounter&& other) {
12355+
#if V8_ENABLE_MEMORY_ACCOUNTING_CHECKS
1234812356
amount_of_external_memory_ =
1234912357
std::exchange(other.amount_of_external_memory_, 0U);
1235012358
isolate_ = std::exchange(other.isolate_, nullptr);
1235112359
#endif
1235212360
}
1235312361

12354-
ExternalMemoryAccounterBase& ExternalMemoryAccounterBase::operator=(
12355-
ExternalMemoryAccounterBase&& other) V8_NOEXCEPT {
12356-
#if DEBUG
12362+
ExternalMemoryAccounter& ExternalMemoryAccounter::operator=(
12363+
ExternalMemoryAccounter&& other) {
12364+
#if V8_ENABLE_MEMORY_ACCOUNTING_CHECKS
1235712365
if (this == &other) {
1235812366
return *this;
1235912367
}
@@ -12365,33 +12373,32 @@ ExternalMemoryAccounterBase& ExternalMemoryAccounterBase::operator=(
1236512373
return *this;
1236612374
}
1236712375

12368-
void ExternalMemoryAccounterBase::Increase(Isolate* isolate, size_t size) {
12369-
#ifdef DEBUG
12376+
void ExternalMemoryAccounter::Increase(Isolate* isolate, size_t size) {
12377+
#ifdef V8_ENABLE_MEMORY_ACCOUNTING_CHECKS
1237012378
DCHECK(isolate == isolate_ || isolate_ == nullptr);
1237112379
isolate_ = isolate;
1237212380
amount_of_external_memory_ += size;
1237312381
#endif
12374-
reinterpret_cast<v8::Isolate*>(isolate)
12375-
->AdjustAmountOfExternalAllocatedMemory(static_cast<int64_t>(size));
12382+
isolate->AdjustAmountOfExternalAllocatedMemoryImpl(
12383+
static_cast<int64_t>(size));
1237612384
}
1237712385

12378-
void ExternalMemoryAccounterBase::Update(Isolate* isolate, int64_t delta) {
12379-
#ifdef DEBUG
12386+
void ExternalMemoryAccounter::Update(Isolate* isolate, int64_t delta) {
12387+
#ifdef V8_ENABLE_MEMORY_ACCOUNTING_CHECKS
1238012388
DCHECK(isolate == isolate_ || isolate_ == nullptr);
1238112389
DCHECK_GE(static_cast<int64_t>(amount_of_external_memory_), -delta);
1238212390
isolate_ = isolate;
1238312391
amount_of_external_memory_ += delta;
1238412392
#endif
12385-
reinterpret_cast<v8::Isolate*>(isolate)
12386-
->AdjustAmountOfExternalAllocatedMemory(delta);
12393+
isolate->AdjustAmountOfExternalAllocatedMemoryImpl(delta);
1238712394
}
1238812395

12389-
void ExternalMemoryAccounterBase::Decrease(Isolate* isolate, size_t size) {
12390-
DisallowGarbageCollection no_gc;
12396+
void ExternalMemoryAccounter::Decrease(Isolate* isolate, size_t size) {
12397+
internal::DisallowGarbageCollection no_gc;
1239112398
if (size == 0) {
1239212399
return;
1239312400
}
12394-
#ifdef DEBUG
12401+
#ifdef V8_ENABLE_MEMORY_ACCOUNTING_CHECKS
1239512402
DCHECK_EQ(isolate, isolate_);
1239612403
DCHECK_GE(amount_of_external_memory_, size);
1239712404
amount_of_external_memory_ -= size;
@@ -12400,7 +12407,12 @@ void ExternalMemoryAccounterBase::Decrease(Isolate* isolate, size_t size) {
1240012407
i_isolate->heap()->UpdateExternalMemory(-static_cast<int64_t>(size));
1240112408
}
1240212409

12403-
} // namespace internal
12410+
int64_t
12411+
ExternalMemoryAccounter::GetTotalAmountOfExternalAllocatedMemoryForTesting(
12412+
const Isolate* isolate) {
12413+
const i::Isolate* i_isolate = reinterpret_cast<const i::Isolate*>(isolate);
12414+
return i_isolate->heap()->external_memory();
12415+
}
1240412416

1240512417
template <>
1240612418
bool V8_EXPORT V8_WARN_UNUSED_RESULT

‎src/api/api.h‎

Lines changed: 0 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -549,31 +549,6 @@ bool ValidateCallbackInfo(const PropertyCallbackInfo<T>& info);
549549

550550
DECLARE_CONTEXTUAL_VARIABLE_WITH_DEFAULT(StackAllocatedCheck, const bool, true);
551551

552-
// TODO(crbug.com/42203776): This should move to the API and be integrated into
553-
// `AdjustAmountOfExternalAllocatedMemory()` to make sure there are no
554-
// unbalanced bytes floating around.
555-
class V8_EXPORT_PRIVATE ExternalMemoryAccounterBase {
556-
public:
557-
ExternalMemoryAccounterBase() = default;
558-
~ExternalMemoryAccounterBase();
559-
ExternalMemoryAccounterBase(ExternalMemoryAccounterBase&&) V8_NOEXCEPT;
560-
ExternalMemoryAccounterBase& operator=(ExternalMemoryAccounterBase&&)
561-
V8_NOEXCEPT;
562-
ExternalMemoryAccounterBase(const ExternalMemoryAccounterBase&) = delete;
563-
ExternalMemoryAccounterBase& operator=(const ExternalMemoryAccounterBase&) =
564-
delete;
565-
566-
void Increase(Isolate* isolate, size_t size);
567-
void Update(Isolate* isolate, int64_t delta);
568-
void Decrease(Isolate* isolate, size_t size);
569-
570-
private:
571-
#ifdef DEBUG
572-
size_t amount_of_external_memory_ = 0;
573-
Isolate* isolate_ = nullptr;
574-
#endif
575-
};
576-
577552
} // namespace internal
578553
} // namespace v8
579554

‎src/execution/isolate.cc‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3671,7 +3671,8 @@ void Isolate::ReleaseSharedPtrs() {
36713671
ManagedPtrDestructor* n = nullptr;
36723672
managed_ptr_destructors_head_ = nullptr;
36733673
for (; l != nullptr; l = n) {
3674-
l->external_memory_accounter_.Decrease(this, l->estimated_size_);
3674+
l->external_memory_accounter_.Decrease(
3675+
reinterpret_cast<v8::Isolate*>(this), l->estimated_size_);
36753676
l->destructor_(l->shared_ptr_ptr_);
36763677
n = l->next_;
36773678
delete l;

‎src/heap/array-buffer-sweeper.cc‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -422,14 +422,16 @@ void ArrayBufferSweeper::IncrementExternalMemoryCounters(size_t bytes) {
422422
if (bytes == 0) return;
423423
heap_->IncrementExternalBackingStoreBytes(
424424
ExternalBackingStoreType::kArrayBuffer, bytes);
425-
external_memory_accounter_.Increase(heap_->isolate(), bytes);
425+
external_memory_accounter_.Increase(
426+
reinterpret_cast<v8::Isolate*>(heap_->isolate()), bytes);
426427
}
427428

428429
void ArrayBufferSweeper::DecrementExternalMemoryCounters(size_t bytes) {
429430
if (bytes == 0) return;
430431
heap_->DecrementExternalBackingStoreBytes(
431432
ExternalBackingStoreType::kArrayBuffer, bytes);
432-
external_memory_accounter_.Decrease(heap_->isolate(), bytes);
433+
external_memory_accounter_.Decrease(
434+
reinterpret_cast<v8::Isolate*>(heap_->isolate()), bytes);
433435
}
434436

435437
void ArrayBufferSweeper::FinalizeAndDelete(ArrayBufferExtension* extension) {

‎src/heap/array-buffer-sweeper.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
#include <memory>
99

10+
#include "include/v8-external-memory-accounter.h"
1011
#include "include/v8config.h"
1112
#include "src/api/api.h"
1213
#include "src/base/logging.h"
@@ -115,7 +116,7 @@ class ArrayBufferSweeper final {
115116
// finishes.
116117
int64_t young_bytes_adjustment_while_sweeping_{0};
117118
int64_t old_bytes_adjustment_while_sweeping_{0};
118-
V8_NO_UNIQUE_ADDRESS ExternalMemoryAccounterBase external_memory_accounter_;
119+
V8_NO_UNIQUE_ADDRESS ExternalMemoryAccounter external_memory_accounter_;
119120
};
120121

121122
} // namespace internal

0 commit comments

Comments
 (0)