Skip to content

Commit b3054f7

Browse files
o-V8 LUCI CQ
o-
authored and
V8 LUCI CQ
committed
[refactoring][heap] Remove build configs without shared r/o heap
Remove any remaining build config that supports more than one read-only heap per cage (or in total for non-compressed builds). There is no feature and/or build config left that relies on multiple r/o heaps. Au contraire, several things can be simplified if we can assume a unique r/o heap. Change-Id: I0e0414d93a0a5ecac4f7cbd03dcc57cefbb7c48f Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6038531 Reviewed-by: Igor Sheludko <ishell@chromium.org> Commit-Queue: Olivier Flückiger <olivf@chromium.org> Reviewed-by: Dominik Inführ <dinfuehr@chromium.org> Cr-Commit-Position: refs/heads/main@{#97371}
1 parent 70d2436 commit b3054f7

19 files changed

Lines changed: 74 additions & 183 deletions

‎BUILD.bazel‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -541,14 +541,6 @@ v8_config(
541541
"V8_COMPRESS_POINTERS_IN_MULTIPLE_CAGES",
542542
],
543543
"//conditions:default": [],
544-
}) + select({
545-
# Shared RO heap is unconfigurable in bazel. However, we
546-
# still have to make sure that the flag is disabled when
547-
# v8_enable_pointer_compression_shared_cage is set to false.
548-
":is_v8_enable_pointer_compression_shared_cage": [
549-
"V8_SHARED_RO_HEAP",
550-
],
551-
"//conditions:default": [],
552544
}) + select({
553545
":is_v8_enable_short_builtin_calls": [
554546
"V8_SHORT_BUILTIN_CALLS",

‎BUILD.gn‎

Lines changed: 5 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -259,10 +259,6 @@ declare_args() {
259259
# specific hook).
260260
v8_check_header_includes = false
261261

262-
# Enable sharing read-only space across isolates.
263-
# Sets -DV8_SHARED_RO_HEAP.
264-
v8_enable_shared_ro_heap = ""
265-
266262
# Enable lazy source positions by default.
267263
v8_enable_lazy_source_positions = true
268264

@@ -597,9 +593,6 @@ if (v8_enable_short_builtin_calls &&
597593
# guaranteed to be close enough to embedded builtins.
598594
v8_enable_short_builtin_calls = false
599595
}
600-
if (v8_enable_shared_ro_heap == "") {
601-
v8_enable_shared_ro_heap = true
602-
}
603596

604597
if (v8_enable_sandbox == "") {
605598
# TODO(saelo, v8:11880) remove dependency on v8_enable_external_code_space
@@ -622,15 +615,13 @@ if (v8_enable_static_roots == "") {
622615
# without external code space allocate read only roots at a further
623616
# location relative to the cage base.
624617
v8_enable_static_roots =
625-
v8_enable_pointer_compression && v8_enable_shared_ro_heap &&
626-
v8_enable_external_code_space && v8_enable_webassembly &&
627-
v8_enable_i18n_support
618+
v8_enable_pointer_compression && v8_enable_external_code_space &&
619+
v8_enable_webassembly && v8_enable_i18n_support
628620
}
629621

630622
assert(!v8_enable_static_roots ||
631-
(v8_enable_pointer_compression && v8_enable_shared_ro_heap &&
632-
v8_enable_external_code_space && v8_enable_webassembly &&
633-
v8_enable_i18n_support),
623+
(v8_enable_pointer_compression && v8_enable_external_code_space &&
624+
v8_enable_webassembly && v8_enable_i18n_support),
634625
"Trying to enable static roots in a configuration that is not supported")
635626

636627
assert(
@@ -1215,9 +1206,6 @@ config("features") {
12151206
if (v8_use_siphash) {
12161207
defines += [ "V8_USE_SIPHASH" ]
12171208
}
1218-
if (v8_enable_shared_ro_heap) {
1219-
defines += [ "V8_SHARED_RO_HEAP" ]
1220-
}
12211209
if (v8_win64_unwinding_info) {
12221210
defines += [ "V8_WIN64_UNWINDING_INFO" ]
12231211
}
@@ -2811,7 +2799,7 @@ action("v8_dump_build_config") {
28112799
mips_use_msa_var = mips_use_msa
28122800
}
28132801

2814-
js_shared_memory = v8_enable_shared_ro_heap && !v8_disable_write_barriers
2802+
js_shared_memory = !v8_disable_write_barriers
28152803
simd_mips = mips_arch_variant_var == "r6" && mips_use_msa
28162804
simulator_run = target_cpu != v8_target_cpu
28172805
use_sanitizer = is_asan || is_cfi || is_msan || is_tsan || is_ubsan
@@ -2870,7 +2858,6 @@ action("v8_dump_build_config") {
28702858
"pointer_compression_shared_cage=$v8_enable_pointer_compression_shared_cage",
28712859
"runtime_call_stats=$v8_enable_runtime_call_stats",
28722860
"sandbox=$v8_enable_sandbox",
2873-
"shared_ro_heap=$v8_enable_shared_ro_heap",
28742861
"simd_mips=$simd_mips",
28752862
"simulator_run=$simulator_run",
28762863
"single_generation=$v8_enable_single_generation",
@@ -2921,7 +2908,6 @@ generated_file("v8_generate_features_json") {
29212908
v8_enable_pointer_compression_shared_cage =
29222909
v8_enable_pointer_compression_shared_cage
29232910
v8_enable_sandbox = v8_enable_sandbox
2924-
v8_enable_shared_ro_heap = v8_enable_shared_ro_heap
29252911
v8_enable_short_builtin_calls = v8_enable_short_builtin_calls
29262912
v8_enable_v8_checks = v8_enable_v8_checks
29272913
v8_enable_webassembly = v8_enable_webassembly

‎src/api/api.cc‎

Lines changed: 6 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -10284,14 +10284,6 @@ void Isolate::GetHeapStatistics(HeapStatistics* heap_statistics) {
1028410284

1028510285
heap_statistics->total_available_size_ = heap->Available();
1028610286

10287-
if (!i::ReadOnlyHeap::IsReadOnlySpaceShared()) {
10288-
i::ReadOnlySpace* ro_space = heap->read_only_space();
10289-
heap_statistics->used_heap_size_ += ro_space->Size();
10290-
heap_statistics->total_physical_size_ +=
10291-
ro_space->CommittedPhysicalMemory();
10292-
heap_statistics->total_heap_size_ += ro_space->CommittedMemory();
10293-
}
10294-
1029510287
// TODO(dinfuehr): Right now used <= committed physical does not hold. Fix
1029610288
// this and add DCHECK.
1029710289
DCHECK_LE(heap_statistics->used_heap_size_,
@@ -10346,19 +10338,12 @@ bool Isolate::GetHeapSpaceStatistics(HeapSpaceStatistics* space_statistics,
1034610338
space_statistics->space_name_ = i::ToString(allocation_space);
1034710339

1034810340
if (allocation_space == i::RO_SPACE) {
10349-
if (i::ReadOnlyHeap::IsReadOnlySpaceShared()) {
10350-
// RO_SPACE memory is accounted for elsewhere when ReadOnlyHeap is shared.
10351-
space_statistics->space_size_ = 0;
10352-
space_statistics->space_used_size_ = 0;
10353-
space_statistics->space_available_size_ = 0;
10354-
space_statistics->physical_space_size_ = 0;
10355-
} else {
10356-
i::ReadOnlySpace* space = heap->read_only_space();
10357-
space_statistics->space_size_ = space->CommittedMemory();
10358-
space_statistics->space_used_size_ = space->Size();
10359-
space_statistics->space_available_size_ = 0;
10360-
space_statistics->physical_space_size_ = space->CommittedPhysicalMemory();
10361-
}
10341+
// RO_SPACE memory is shared across all isolates and accounted for
10342+
// elsewhere.
10343+
space_statistics->space_size_ = 0;
10344+
space_statistics->space_used_size_ = 0;
10345+
space_statistics->space_available_size_ = 0;
10346+
space_statistics->physical_space_size_ = 0;
1036210347
} else {
1036310348
i::Space* space = heap->space(static_cast<int>(index));
1036410349
space_statistics->space_size_ = space ? space->CommittedMemory() : 0;

‎src/codegen/code-stub-assembler.cc‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18573,9 +18573,6 @@ void CodeStubAssembler::SharedValueBarrier(
1857318573

1857418574
// Fast path: Smis are trivially shared.
1857518575
GotoIf(TaggedIsSmi(value), &done);
18576-
// Fast path: Shared memory features imply shared RO space, so RO objects are
18577-
// trivially shared.
18578-
CSA_DCHECK(this, BoolConstant(ReadOnlyHeap::IsReadOnlySpaceShared()));
1857918576
TNode<IntPtrT> page_flags = LoadMemoryChunkFlags(CAST(value));
1858018577
GotoIf(WordNotEqual(
1858118578
WordAnd(page_flags, IntPtrConstant(MemoryChunk::READ_ONLY_HEAP)),

‎src/common/globals.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ namespace internal {
116116
#define COMPRESS_POINTERS_IN_MULTIPLE_CAGES_BOOL false
117117
#endif
118118

119-
#if defined(V8_SHARED_RO_HEAP) && !defined(V8_DISABLE_WRITE_BARRIERS)
119+
#ifndef V8_DISABLE_WRITE_BARRIERS
120120
#define V8_CAN_CREATE_SHARED_HEAP_BOOL true
121121
#else
122122
#define V8_CAN_CREATE_SHARED_HEAP_BOOL false

‎src/execution/isolate.cc‎

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4039,12 +4039,8 @@ void Isolate::Delete(Isolate* isolate) {
40394039

40404040
void Isolate::SetUpFromReadOnlyArtifacts(ReadOnlyArtifacts* artifacts,
40414041
ReadOnlyHeap* ro_heap) {
4042-
if (ReadOnlyHeap::IsReadOnlySpaceShared()) {
4043-
DCHECK_NOT_NULL(artifacts);
4044-
InitializeNextUniqueSfiId(artifacts->initial_next_unique_sfi_id());
4045-
} else {
4046-
DCHECK_NULL(artifacts);
4047-
}
4042+
DCHECK_NOT_NULL(artifacts);
4043+
InitializeNextUniqueSfiId(artifacts->initial_next_unique_sfi_id());
40484044
DCHECK_NOT_NULL(ro_heap);
40494045
DCHECK_IMPLIES(read_only_heap_ != nullptr, read_only_heap_ == ro_heap);
40504046
read_only_heap_ = ro_heap;
@@ -5218,9 +5214,6 @@ VirtualMemoryCage* Isolate::GetPtrComprCodeCageForTesting() {
52185214

52195215
void Isolate::VerifyStaticRoots() {
52205216
#if V8_STATIC_ROOTS_BOOL
5221-
static_assert(ReadOnlyHeap::IsReadOnlySpaceShared(),
5222-
"Static read only roots are only supported when there is one "
5223-
"shared read only space per cage");
52245217
#define STATIC_ROOTS_FAILED_MSG \
52255218
"Read-only heap layout changed. Run `tools/dev/gen-static-roots.py` to " \
52265219
"update static-roots.h."

‎src/execution/isolate.h‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -763,7 +763,7 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory {
763763

764764
ReadOnlyArtifacts* read_only_artifacts() const {
765765
ReadOnlyArtifacts* artifacts = isolate_group()->read_only_artifacts();
766-
DCHECK_IMPLIES(ReadOnlyHeap::IsReadOnlySpaceShared(), artifacts != nullptr);
766+
DCHECK_NOT_NULL(artifacts);
767767
return artifacts;
768768
}
769769

@@ -1271,8 +1271,7 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory {
12711271
// only possible if we have just one sole read only heap. In case we extend
12721272
// support to other build configurations we need a table of dispatch entries
12731273
// per isolate. See https://crrev.com/c/5783686 on how to do that.
1274-
static constexpr bool kBuiltinDispatchHandlesAreStatic =
1275-
ReadOnlyHeap::IsReadOnlySpaceShared();
1274+
static constexpr bool kBuiltinDispatchHandlesAreStatic = true;
12761275

12771276
static V8_INLINE JSDispatchHandle
12781277
builtin_dispatch_handle(JSBuiltinDispatchHandleRoot::Idx idx) {

‎src/flags/flag-definitions.h‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -385,12 +385,6 @@ DEFINE_BOOL(icu_timezone_data, true, "get information about timezones from ICU")
385385
#define V8_LAZY_SOURCE_POSITIONS_BOOL false
386386
#endif
387387

388-
#ifdef V8_SHARED_RO_HEAP
389-
#define V8_SHARED_RO_HEAP_BOOL true
390-
#else
391-
#define V8_SHARED_RO_HEAP_BOOL false
392-
#endif
393-
394388
DEFINE_BOOL(stress_snapshot, false,
395389
"disables sharing of the read-only heap for testing")
396390
// Incremental marking is incompatible with the stress_snapshot mode;

‎src/heap/heap-layout-inl.h‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,11 +68,9 @@ bool HeapLayout::InWritableSharedSpace(Tagged<HeapObject> object) {
6868

6969
// static
7070
bool HeapLayout::InAnySharedSpace(Tagged<HeapObject> object) {
71-
#ifdef V8_SHARED_RO_HEAP
7271
if (HeapLayout::InReadOnlySpace(object)) {
73-
return V8_SHARED_RO_HEAP_BOOL;
72+
return true;
7473
}
75-
#endif // V8_SHARED_RO_HEAP
7674
return HeapLayout::InWritableSharedSpace(object);
7775
}
7876

‎src/heap/heap-verifier.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -412,7 +412,7 @@ void HeapVerification::VerifyPage(const MemoryChunkMetadata* chunk_metadata) {
412412
CHECK(!current_chunk_.has_value());
413413
CHECK(!chunk->IsFlagSet(MemoryChunk::PAGE_NEW_OLD_PROMOTION));
414414
CHECK(!chunk->IsFlagSet(MemoryChunk::FROM_PAGE));
415-
if (V8_SHARED_RO_HEAP_BOOL && chunk->InReadOnlySpace()) {
415+
if (chunk->InReadOnlySpace()) {
416416
CHECK_NULL(chunk_metadata->owner());
417417
} else {
418418
CHECK_EQ(chunk_metadata->heap(), heap());

0 commit comments

Comments
 (0)