Skip to content

Commit 1c9f59c

Browse files
sygV8 LUCI CQ
syg
authored and
V8 LUCI CQ
committed
[builtins] Add machinery for multithreaded builtin generation
This is a reland of commit b022146 Changes since revert: - Turn off OMT builtin generation when --reorder-builtins - Make flag off-by-default, controlled by v8_enable_concurrent_mksnapshot GN arg Original change's description: > Reland^2 "[builtins] Optimize and generate builtins off-main-thread" > > This is a reland of commit 9125716 > > Changes since revert: > - Drain the output queue of the dispatcher when input queue is > unavailable or output queue is using too much memory > - Fix data race in AddBuiltin > - Disable when profiling > > Original change's description: > > Reland "[builtins] Optimize and generate builtins off-main-thread" > > > > This is a reland of commit b132bd4 > > > > Changes since revert: > > - Don't request InstallCode interrupt during concurrent builtin > > generation > > > > Original change's description: > > > [builtins] Optimize and generate builtins off-main-thread > > > > > > Bug: 342692713 > > > Change-Id: I7c397ba93bc3b717a00ff4cfafa6919ad571cd43 > > > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5594234 > > > Reviewed-by: Nico Hartmann <nicohartmann@chromium.org> > > > Commit-Queue: Shu-yu Guo <syg@chromium.org> > > > Cr-Commit-Position: refs/heads/main@{#94750} > > > > Bug: 342692713 > > Change-Id: I64c7e482901763aadacc5c6c4a35b99d1c03481f > > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5670430 > > Reviewed-by: Nico Hartmann <nicohartmann@chromium.org> > > Commit-Queue: Shu-yu Guo <syg@chromium.org> > > Cr-Commit-Position: refs/heads/main@{#94784} > > Bug: 342692713 > Change-Id: I99cbcfb4576b3a5a256eb36c3ac926cfd02cb6fc > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5671629 > Reviewed-by: Nico Hartmann <nicohartmann@chromium.org> > Commit-Queue: Shu-yu Guo <syg@chromium.org> > Cr-Commit-Position: refs/heads/main@{#95024} Bug: 342692713 Change-Id: If867249bfae57ed7c5661b3f91768128bd39a323 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5710271 Reviewed-by: Nico Hartmann <nicohartmann@chromium.org> Commit-Queue: Shu-yu Guo <syg@chromium.org> Reviewed-by: Igor Sheludko <ishell@chromium.org> Cr-Commit-Position: refs/heads/main@{#97356}
1 parent f54c8cd commit 1c9f59c

19 files changed

Lines changed: 800 additions & 182 deletions

‎BUILD.gn‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,9 @@ declare_args() {
103103
# Enable fast mksnapshot runs.
104104
v8_enable_fast_mksnapshot = false
105105

106+
# Enable using multiple threads to build builtins in mksnapshot.
107+
v8_enable_concurrent_mksnapshot = ""
108+
106109
# Optimize code for Torque executable, even during a debug build.
107110
v8_enable_fast_torque = ""
108111

@@ -529,6 +532,9 @@ if (v8_enable_pointer_compression_8gb == "") {
529532
if (v8_enable_fast_torque == "") {
530533
v8_enable_fast_torque = v8_enable_fast_mksnapshot
531534
}
535+
if (v8_enable_concurrent_mksnapshot == "") {
536+
v8_enable_concurrent_mksnapshot = v8_enable_fast_mksnapshot
537+
}
532538
if (v8_enable_zone_compression == "") {
533539
v8_enable_zone_compression = false
534540
}
@@ -2665,6 +2671,15 @@ template("run_mksnapshot") {
26652671
}
26662672
}
26672673

2674+
if (v8_enable_concurrent_mksnapshot) {
2675+
args += [
2676+
"--concurrent-builtin-generation",
2677+
2678+
# Use all the cores for concurrent builtin generation.
2679+
"--concurrent-turbofan-max-threads=0",
2680+
]
2681+
}
2682+
26682683
if (v8_enable_verify_heap) {
26692684
args += [ "--verify-heap" ]
26702685
}

‎src/DEPS‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,9 @@ specific_include_rules = {
126126
"+src/heap/factory-base.h",
127127
"+src/heap/local-factory.h",
128128
],
129+
"setup-builtins-internal\.cc": [
130+
"+src/compiler/pipeline.h",
131+
],
129132
"snapshot\.cc": [
130133
"+src/heap/read-only-promotion.h",
131134
],

‎src/builtins/constants-table-builder.cc‎

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,6 @@ uint32_t BuiltinsConstantsTableBuilder::AddObject(Handle<Object> object) {
3838
DCHECK_EQ(ReadOnlyRoots(isolate_).empty_fixed_array(),
3939
isolate_->heap()->builtins_constants_table());
4040

41-
// Must be on the main thread.
42-
DCHECK_EQ(ThreadId::Current(), isolate_->thread_id());
43-
4441
// Must be generating embedded builtin code.
4542
DCHECK(isolate_->IsGeneratingEmbeddedBuiltins());
4643

@@ -49,12 +46,24 @@ uint32_t BuiltinsConstantsTableBuilder::AddObject(Handle<Object> object) {
4946
DCHECK(!IsInstructionStream(*object));
5047
#endif
5148

52-
auto find_result = map_.FindOrInsert(object);
53-
if (!find_result.already_exists) {
54-
DCHECK(IsHeapObject(*object));
55-
*find_result.entry = map_.size() - 1;
49+
// This method is called concurrently from both the main thread and
50+
// compilation threads. Constant indices need to be reproducible during
51+
// builtin generation, so the main thread pre-adds all the constants. A lock
52+
// is still needed since the map data structure is still being concurrently
53+
// accessed.
54+
base::MutexGuard guard(&mutex_);
55+
if (ThreadId::Current() != isolate_->thread_id()) {
56+
auto find_result = map_.Find(object);
57+
DCHECK_NOT_NULL(find_result);
58+
return *find_result;
59+
} else {
60+
auto find_result = map_.FindOrInsert(object);
61+
if (!find_result.already_exists) {
62+
DCHECK(IsHeapObject(*object));
63+
*find_result.entry = map_.size() - 1;
64+
}
65+
return *find_result.entry;
5666
}
57-
return *find_result.entry;
5867
}
5968

6069
namespace {

‎src/builtins/constants-table-builder.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,10 @@ class BuiltinsConstantsTableBuilder final {
5454
// Maps objects to corresponding indices within the constants list.
5555
using ConstantsMap = IdentityMap<uint32_t, FreeStoreAllocationPolicy>;
5656
ConstantsMap map_;
57+
58+
// Protects accesses to map_, which is concurrently accessed when generating
59+
// builtins off-main-thread.
60+
base::Mutex mutex_;
5761
};
5862

5963
} // namespace internal

0 commit comments

Comments
 (0)