Skip to content

Commit b132bd4

Browse files
sygV8 LUCI CQ
syg
authored and
V8 LUCI CQ
committed
[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 <[email protected]> Commit-Queue: Shu-yu Guo <[email protected]> Cr-Commit-Position: refs/heads/main@{#94750}
1 parent b8235a6 commit b132bd4

17 files changed

Lines changed: 670 additions & 132 deletions

‎src/DEPS‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,9 @@ specific_include_rules = {
121121
"+src/heap/factory-base.h",
122122
"+src/heap/local-factory.h",
123123
],
124+
"setup-builtins-internal\.cc": [
125+
"+src/compiler/pipeline.h",
126+
],
124127
"snapshot\.cc": [
125128
"+src/heap/read-only-promotion.h",
126129
],

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

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

39-
// Must be on the main thread.
40-
DCHECK_EQ(ThreadId::Current(), isolate_->thread_id());
41-
4239
// Must be generating embedded builtin code.
4340
DCHECK(isolate_->IsGeneratingEmbeddedBuiltins());
4441

@@ -47,12 +44,24 @@ uint32_t BuiltinsConstantsTableBuilder::AddObject(Handle<Object> object) {
4744
DCHECK(!IsInstructionStream(*object));
4845
#endif
4946

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

5867
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

‎src/builtins/setup-builtins-internal.cc‎

Lines changed: 82 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,13 @@
77
#include "src/builtins/builtins-inl.h"
88
#include "src/builtins/profile-data-reader.h"
99
#include "src/codegen/assembler-inl.h"
10+
#include "src/codegen/compiler.h"
1011
#include "src/codegen/interface-descriptors.h"
1112
#include "src/codegen/macro-assembler-inl.h"
1213
#include "src/codegen/macro-assembler.h"
1314
#include "src/codegen/reloc-info-inl.h"
1415
#include "src/common/globals.h"
16+
#include "src/compiler-dispatcher/optimizing-compile-dispatcher.h"
1517
#include "src/compiler/code-assembler.h"
1618
#include "src/compiler/pipeline.h"
1719
#include "src/compiler/turboshaft/phase.h"
@@ -92,10 +94,11 @@ AssemblerOptions BuiltinAssemblerOptions(Isolate* isolate, Builtin builtin) {
9294
}
9395

9496
using MacroAssemblerGenerator = void (*)(MacroAssembler*);
95-
using CodeAssemblerGenerator = void (*)(compiler::CodeAssemblerState*);
9697
using TurboshaftAssemblerGenerator =
9798
void (*)(compiler::turboshaft::PipelineData*, Isolate*,
9899
compiler::turboshaft::Graph&, Zone*);
100+
using CodeAssemblerGenerator = compiler::Pipeline::CodeAssemblerGenerator;
101+
using CodeAssemblerInstaller = compiler::Pipeline::CodeAssemblerInstaller;
99102

100103
Handle<Code> BuildPlaceholder(Isolate* isolate, Builtin builtin) {
101104
HandleScope scope(isolate);
@@ -189,20 +192,16 @@ Tagged<Code> BuildAdaptor(Isolate* isolate, Builtin builtin,
189192
return *code;
190193
}
191194

192-
// Builder for builtins implemented in TurboFan with JS linkage.
193-
V8_NOINLINE Tagged<Code> BuildWithCodeStubAssemblerJS(
194-
Isolate* isolate, Builtin builtin, CodeAssemblerGenerator generator,
195-
int argc, const char* name) {
196-
HandleScope scope(isolate);
197-
198-
Zone zone(isolate->allocator(), ZONE_NAME, kCompressGraphZone);
199-
compiler::CodeAssemblerState state(isolate, &zone, argc, CodeKind::BUILTIN,
200-
name, builtin);
201-
generator(&state);
202-
DirectHandle<Code> code = compiler::CodeAssembler::GenerateCode(
203-
&state, BuiltinAssemblerOptions(isolate, builtin),
204-
ProfileDataFromFile::TryRead(name));
205-
return *code;
195+
void CompileJSLinkageCodeStubBuiltin(Isolate* isolate, Builtin builtin,
196+
CodeAssemblerGenerator generator,
197+
CodeAssemblerInstaller installer, int argc,
198+
const char* name) {
199+
std::unique_ptr<TurbofanCompilationJob> job(
200+
compiler::Pipeline::NewJSLinkageCodeStubBuiltinCompilationJob(
201+
isolate, builtin, generator, installer,
202+
BuiltinAssemblerOptions(isolate, builtin), argc, name,
203+
ProfileDataFromFile::TryRead(name)));
204+
compiler::CodeAssembler::CompileCode(isolate, std::move(job));
206205
}
207206

208207
inline constexpr char kTempZoneName[] = "temp-zone";
@@ -245,26 +244,38 @@ V8_NOINLINE Tagged<Code> BuildWithTurboshaftAssemblerCS(
245244
}
246245

247246
// Builder for builtins implemented in TurboFan with CallStub linkage.
248-
V8_NOINLINE Tagged<Code> BuildWithCodeStubAssemblerCS(
249-
Isolate* isolate, Builtin builtin, CodeAssemblerGenerator generator,
250-
CallDescriptors::Key interface_descriptor, const char* name) {
247+
void CompileCSLinkageCodeStubBuiltin(Isolate* isolate, Builtin builtin,
248+
CodeAssemblerGenerator generator,
249+
CodeAssemblerInstaller installer,
250+
CallDescriptors::Key interface_descriptor,
251+
const char* name) {
251252
// TODO(nicohartmann): Remove this once `BuildWithTurboshaftAssemblerCS` has
252253
// an actual use.
253254
USE(&BuildWithTurboshaftAssemblerCS);
254-
HandleScope scope(isolate);
255-
Zone zone(isolate->allocator(), ZONE_NAME, kCompressGraphZone);
256-
// The interface descriptor with given key must be initialized at this point
257-
// and this construction just queries the details from the descriptors table.
258-
CallInterfaceDescriptor descriptor(interface_descriptor);
259-
// Ensure descriptor is already initialized.
260-
DCHECK_LE(0, descriptor.GetRegisterParameterCount());
261-
compiler::CodeAssemblerState state(isolate, &zone, descriptor,
262-
CodeKind::BUILTIN, name, builtin);
263-
generator(&state);
264-
DirectHandle<Code> code = compiler::CodeAssembler::GenerateCode(
265-
&state, BuiltinAssemblerOptions(isolate, builtin),
266-
ProfileDataFromFile::TryRead(name));
267-
return *code;
255+
std::unique_ptr<TurbofanCompilationJob> job(
256+
compiler::Pipeline::NewCSLinkageCodeStubBuiltinCompilationJob(
257+
isolate, builtin, generator, installer,
258+
BuiltinAssemblerOptions(isolate, builtin), interface_descriptor, name,
259+
ProfileDataFromFile::TryRead(name)));
260+
compiler::CodeAssembler::CompileCode(isolate, std::move(job));
261+
}
262+
263+
void CompileBytecodeHandler(
264+
Isolate* isolate, Builtin builtin, interpreter::OperandScale operand_scale,
265+
interpreter::Bytecode bytecode,
266+
compiler::Pipeline::CodeAssemblerInstaller installer) {
267+
DCHECK(interpreter::Bytecodes::BytecodeHasHandler(bytecode, operand_scale));
268+
const char* name = Builtins::name(builtin);
269+
auto generator = [bytecode,
270+
operand_scale](compiler::CodeAssemblerState* state) {
271+
interpreter::GenerateBytecodeHandler(state, bytecode, operand_scale);
272+
};
273+
std::unique_ptr<TurbofanCompilationJob> job(
274+
compiler::Pipeline::NewBytecodeHandlerCompilationJob(
275+
isolate, builtin, generator, installer,
276+
BuiltinAssemblerOptions(isolate, builtin), name,
277+
ProfileDataFromFile::TryRead(name)));
278+
compiler::CodeAssembler::CompileCode(isolate, std::move(job));
268279
}
269280

270281
} // anonymous namespace
@@ -342,20 +353,6 @@ void SetupIsolateDelegate::ReplacePlaceholders(Isolate* isolate) {
342353
}
343354
}
344355

345-
namespace {
346-
347-
V8_NOINLINE Tagged<Code> GenerateBytecodeHandler(
348-
Isolate* isolate, Builtin builtin, interpreter::OperandScale operand_scale,
349-
interpreter::Bytecode bytecode) {
350-
DCHECK(interpreter::Bytecodes::BytecodeHasHandler(bytecode, operand_scale));
351-
DirectHandle<Code> code = interpreter::GenerateBytecodeHandler(
352-
isolate, Builtins::name(builtin), bytecode, operand_scale, builtin,
353-
BuiltinAssemblerOptions(isolate, builtin));
354-
return *code;
355-
}
356-
357-
} // namespace
358-
359356
// static
360357
void SetupIsolateDelegate::SetupBuiltinsInternal(Isolate* isolate) {
361358
Builtins* builtins = isolate->builtins();
@@ -379,11 +376,14 @@ void SetupIsolateDelegate::SetupBuiltinsInternal(Isolate* isolate) {
379376
AddBuiltin(builtins, Builtin::k##Name, code); \
380377
index++;
381378

382-
#define BUILD_TFJ(Name, Argc, ...) \
383-
code = BuildWithCodeStubAssemblerJS( \
384-
isolate, Builtin::k##Name, &Builtins::Generate_##Name, Argc, #Name); \
385-
AddBuiltin(builtins, Builtin::k##Name, code); \
386-
index++;
379+
#define BUILD_TFJ(Name, Argc, ...) \
380+
CompileJSLinkageCodeStubBuiltin( \
381+
isolate, Builtin::k##Name, &Builtins::Generate_##Name, \
382+
[builtins, &index](Handle<Code> code) { \
383+
AddBuiltin(builtins, Builtin::k##Name, *code); \
384+
index++; \
385+
}, \
386+
Argc, #Name);
387387

388388
#define BUILD_TSC(Name, InterfaceDescriptor) \
389389
/* Return size is from the provided CallInterfaceDescriptor. */ \
@@ -395,33 +395,40 @@ void SetupIsolateDelegate::SetupBuiltinsInternal(Isolate* isolate) {
395395

396396
#define BUILD_TFC(Name, InterfaceDescriptor) \
397397
/* Return size is from the provided CallInterfaceDescriptor. */ \
398-
code = BuildWithCodeStubAssemblerCS( \
398+
CompileCSLinkageCodeStubBuiltin( \
399399
isolate, Builtin::k##Name, &Builtins::Generate_##Name, \
400-
CallDescriptors::InterfaceDescriptor, #Name); \
401-
AddBuiltin(builtins, Builtin::k##Name, code); \
402-
index++;
400+
[builtins, &index](Handle<Code> code) { \
401+
AddBuiltin(builtins, Builtin::k##Name, *code); \
402+
index++; \
403+
}, \
404+
CallDescriptors::InterfaceDescriptor, #Name);
403405

404406
#define BUILD_TFS(Name, ...) \
405407
/* Return size for generic TF builtins (stub linkage) is always 1. */ \
406-
code = BuildWithCodeStubAssemblerCS(isolate, Builtin::k##Name, \
407-
&Builtins::Generate_##Name, \
408-
CallDescriptors::Name, #Name); \
409-
AddBuiltin(builtins, Builtin::k##Name, code); \
410-
index++;
408+
CompileCSLinkageCodeStubBuiltin( \
409+
isolate, Builtin::k##Name, &Builtins::Generate_##Name, \
410+
[builtins, &index](Handle<Code> code) { \
411+
AddBuiltin(builtins, Builtin::k##Name, *code); \
412+
index++; \
413+
}, \
414+
CallDescriptors::Name, #Name);
411415

412416
#define BUILD_TFH(Name, InterfaceDescriptor) \
413417
/* Return size for IC builtins/handlers is always 1. */ \
414-
code = BuildWithCodeStubAssemblerCS( \
418+
CompileCSLinkageCodeStubBuiltin( \
415419
isolate, Builtin::k##Name, &Builtins::Generate_##Name, \
416-
CallDescriptors::InterfaceDescriptor, #Name); \
417-
AddBuiltin(builtins, Builtin::k##Name, code); \
418-
index++;
419-
420-
#define BUILD_BCH(Name, OperandScale, Bytecode) \
421-
code = GenerateBytecodeHandler(isolate, Builtin::k##Name, OperandScale, \
422-
Bytecode); \
423-
AddBuiltin(builtins, Builtin::k##Name, code); \
424-
index++;
420+
[builtins, &index](Handle<Code> code) { \
421+
AddBuiltin(builtins, Builtin::k##Name, *code); \
422+
index++; \
423+
}, \
424+
CallDescriptors::InterfaceDescriptor, #Name);
425+
426+
#define BUILD_BCH(Name, OperandScale, Bytecode) \
427+
CompileBytecodeHandler(isolate, Builtin::k##Name, OperandScale, Bytecode, \
428+
[builtins, &index](Handle<Code> code) { \
429+
AddBuiltin(builtins, Builtin::k##Name, *code); \
430+
index++; \
431+
});
425432

426433
#define BUILD_ASM(Name, InterfaceDescriptor) \
427434
code = BuildWithMacroAssembler(isolate, Builtin::k##Name, \
@@ -440,6 +447,11 @@ void SetupIsolateDelegate::SetupBuiltinsInternal(Isolate* isolate) {
440447
#undef BUILD_TFH
441448
#undef BUILD_BCH
442449
#undef BUILD_ASM
450+
451+
if (v8_flags.concurrent_builtin_generation) {
452+
isolate->optimizing_compile_dispatcher()->AwaitCompileTasks();
453+
isolate->optimizing_compile_dispatcher()->InstallGeneratedBuiltins();
454+
}
443455
CHECK_EQ(Builtins::kBuiltinCount, index);
444456

445457
ReplacePlaceholders(isolate);

‎src/compiler-dispatcher/optimizing-compile-dispatcher.cc‎

Lines changed: 39 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ void OptimizingCompileDispatcher::CompileNext(TurbofanCompilationJob* job,
101101
// Use a mutex to make sure that functions marked for install
102102
// are always also queued.
103103
base::MutexGuard access_output_queue_(&output_queue_mutex_);
104-
output_queue_.push(job);
104+
output_queue_.push_back(job);
105105
}
106106

107107
if (finalize()) isolate_->stack_guard()->RequestInstallCode();
@@ -114,7 +114,7 @@ void OptimizingCompileDispatcher::FlushOutputQueue(bool restore_function_code) {
114114
base::MutexGuard access_output_queue_(&output_queue_mutex_);
115115
if (output_queue_.empty()) return;
116116
job.reset(output_queue_.front());
117-
output_queue_.pop();
117+
output_queue_.pop_front();
118118
}
119119

120120
Compiler::DisposeTurbofanCompilationJob(isolate_, job.get(),
@@ -186,7 +186,7 @@ void OptimizingCompileDispatcher::InstallOptimizedFunctions() {
186186
base::MutexGuard access_output_queue_(&output_queue_mutex_);
187187
if (output_queue_.empty()) return;
188188
job.reset(output_queue_.front());
189-
output_queue_.pop();
189+
output_queue_.pop_front();
190190
}
191191
OptimizedCompilationInfo* info = job->compilation_info();
192192
DirectHandle<JSFunction> function(*info->closure(), isolate_);
@@ -208,6 +208,35 @@ void OptimizingCompileDispatcher::InstallOptimizedFunctions() {
208208
}
209209
}
210210

211+
void OptimizingCompileDispatcher::InstallGeneratedBuiltins() {
212+
// Builtin generation needs to be deterministic, meaning heap allocations
213+
// must happen in a deterministic order. To ensure determinism with
214+
// concurrent compilation, finalize all builtins at the end in ascending
215+
// order of their BuiltinId.
216+
217+
CHECK(isolate_->IsGeneratingEmbeddedBuiltins());
218+
CHECK_EQ(0, input_queue_.Length());
219+
220+
HandleScope handle_scope(isolate_);
221+
base::MutexGuard access_output_queue_(&output_queue_mutex_);
222+
223+
std::sort(output_queue_.begin(), output_queue_.end(),
224+
[](const TurbofanCompilationJob* job1,
225+
const TurbofanCompilationJob* job2) {
226+
Builtin builtin1 = job1->compilation_info()->builtin();
227+
Builtin builtin2 = job2->compilation_info()->builtin();
228+
DCHECK(Builtins::IsBuiltinId(builtin1));
229+
DCHECK(Builtins::IsBuiltinId(builtin2));
230+
return Builtins::ToInt(builtin1) < Builtins::ToInt(builtin2);
231+
});
232+
233+
while (!output_queue_.empty()) {
234+
std::unique_ptr<TurbofanCompilationJob> job(output_queue_.front());
235+
output_queue_.pop_front();
236+
CHECK_EQ(CompilationJob::SUCCEEDED, job->FinalizeJob(isolate_));
237+
}
238+
}
239+
211240
bool OptimizingCompileDispatcher::HasJobs() {
212241
DCHECK_EQ(ThreadId::Current(), isolate_->thread_id());
213242
return job_handle_->IsActive() || !output_queue_.empty();
@@ -246,9 +275,14 @@ void OptimizingCompileDispatcher::Prioritize(
246275

247276
OptimizingCompileDispatcher::OptimizingCompileDispatcher(Isolate* isolate)
248277
: isolate_(isolate),
249-
input_queue_(v8_flags.concurrent_recompilation_queue_length),
278+
input_queue_((v8_flags.concurrent_builtin_generation &&
279+
isolate->IsGeneratingEmbeddedBuiltins())
280+
? Builtins::kBuiltinCount
281+
: v8_flags.concurrent_recompilation_queue_length),
250282
recompilation_delay_(v8_flags.concurrent_recompilation_delay) {
251-
if (v8_flags.concurrent_recompilation) {
283+
if (v8_flags.concurrent_recompilation ||
284+
(v8_flags.concurrent_builtin_generation &&
285+
isolate->IsGeneratingEmbeddedBuiltins())) {
252286
job_handle_ = V8::GetCurrentPlatform()->PostJob(
253287
kTaskPriority, std::make_unique<CompileTask>(isolate, this));
254288
}

‎src/compiler-dispatcher/optimizing-compile-dispatcher.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@ class V8_EXPORT_PRIVATE OptimizingCompileDispatcher {
9191
void QueueForOptimization(TurbofanCompilationJob* job);
9292
void AwaitCompileTasks();
9393
void InstallOptimizedFunctions();
94+
void InstallGeneratedBuiltins();
9495

9596
inline bool IsQueueAvailable() { return input_queue_.IsAvailable(); }
9697

@@ -130,7 +131,7 @@ class V8_EXPORT_PRIVATE OptimizingCompileDispatcher {
130131
OptimizingCompileDispatcherQueue input_queue_;
131132

132133
// Queue of recompilation tasks ready to be installed (excluding OSR).
133-
std::queue<TurbofanCompilationJob*> output_queue_;
134+
std::deque<TurbofanCompilationJob*> output_queue_;
134135
// Used for job based recompilation which has multiple producers on
135136
// different threads.
136137
base::Mutex output_queue_mutex_;

0 commit comments

Comments
 (0)