AuroraMiddleware/v8
1
0
Fork 0
Code Issues 2 Pull Requests Releases Wiki Activity

Merge with cached Script after streaming compilation

Browse Source 
Currently, if a script is compiled on the main thread or deserialized on
any thread, and a matching Script object is found in the Isolate
compilation cache, the new content is merged into the existing Script.
This CL implements the same merging for scripts which were compiled by a
background task. I expect speed changes to be minimal, because merging
is only needed in a small minority of compilations. When needed, it
usually takes about 10% as long as the deserialization of the script,
which in turn is faster than compilation from source text.

This CL also removes some code which I added in preparation for merging
on a background thread in this case. Upon further discussion, we've
determined that the extra round trip to a background thread when the
main thread is likely just waiting for completion would do more harm
than good, and performing the compilation cache lookup from the
background thread would be quite cumbersome.

Bug: v8:12808
Change-Id: Ia7a14a739779ab658b505572d19df4ec489a078e
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4023904
Commit-Queue: Seth Brenith <seth.brenith@microsoft.com>
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/main@{#84402}
This commit is contained in:
Seth Brenith 2022-11-16 10:18:33 -08:00 committed by V8 LUCI CQ
parent 7096e7a689
commit 1c90992ffc
3 changed files with 18 additions and 84 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

70
src/codegen/compiler.cc
View File

@ -2148,7 +2148,8 @@ Handle<SharedFunctionInfo> BackgroundMergeTask::CompleteMergeInForeground(
MaybeHandle<SharedFunctionInfo> BackgroundCompileTask::FinalizeScript(
Isolate* isolate, Handle<String> source,
const ScriptDetails& script_details) {
const ScriptDetails& script_details,
MaybeHandle<Script> maybe_cached_script) {
ScriptOriginOptions origin_options = script_details.origin_options;
DCHECK(flags_.is_toplevel());
@ -2166,11 +2167,15 @@ MaybeHandle<SharedFunctionInfo> BackgroundCompileTask::FinalizeScript(
maybe_result = outer_function_sfi_;
}
if (!maybe_result.is_null() &&
background_merge_task_.HasPendingForegroundWork()) {
DCHECK(flags().is_toplevel());
if (Handle<Script> cached_script;
maybe_cached_script.ToHandle(&cached_script) && !maybe_result.is_null()) {
BackgroundMergeTask merge;
merge.SetUpOnMainThread(isolate, cached_script);
CHECK(merge.HasPendingBackgroundWork());
merge.BeginMergeInBackground(isolate->AsLocalIsolate(), script);
CHECK(merge.HasPendingForegroundWork());
Handle<SharedFunctionInfo> result =
background_merge_task_.CompleteMergeInForeground(isolate, script);
merge.CompleteMergeInForeground(isolate, script);
maybe_result = result;
script = handle(Script::cast(result->script()), isolate);
DCHECK(script->source().StrictEquals(*source));
@ -2213,7 +2218,6 @@ MaybeHandle<SharedFunctionInfo> BackgroundCompileTask::FinalizeScript(
bool BackgroundCompileTask::FinalizeFunction(
Isolate* isolate, Compiler::ClearExceptionFlag flag) {
DCHECK(!flags_.is_toplevel());
DCHECK(!background_merge_task_.HasPendingForegroundWork());
MaybeHandle<SharedFunctionInfo> maybe_result;
Handle<SharedFunctionInfo> input_shared_info =
@ -2306,33 +2310,12 @@ void BackgroundDeserializeTask::SourceTextAvailable(
language_mode);
}
void BackgroundCompileTask::SourceTextAvailable(
Isolate* isolate, Handle<String> source_text,
const ScriptDetails& script_details) {
DCHECK_EQ(isolate, isolate_for_local_isolate_);
// Non-toplevel compilations already refer to an existing Script; there is no
// need to look anything up in the compilation cache.
if (!flags().is_toplevel()) return;
LanguageMode language_mode = flags().outer_language_mode();
background_merge_task_.SetUpOnMainThread(isolate, source_text, script_details,
language_mode);
}
bool BackgroundDeserializeTask::ShouldMergeWithExistingScript() const {
DCHECK(v8_flags.merge_background_deserialized_script_with_compilation_cache);
return background_merge_task_.HasPendingBackgroundWork() &&
off_thread_data_.HasResult();
}
bool BackgroundCompileTask::ShouldMergeWithExistingScript() const {
DCHECK(v8_flags.stress_background_compile);
DCHECK(!script_.is_null());
return background_merge_task_.HasPendingBackgroundWork() &&
jobs_to_retry_finalization_on_main_thread_.empty();
}
void BackgroundDeserializeTask::MergeWithExistingScript() {
DCHECK(ShouldMergeWithExistingScript());
@ -2344,21 +2327,6 @@ void BackgroundDeserializeTask::MergeWithExistingScript() {
&isolate, off_thread_data_.GetOnlyScript(isolate.heap()));
}
void BackgroundCompileTask::MergeWithExistingScript() {
DCHECK(ShouldMergeWithExistingScript());
LocalIsolate isolate(isolate_for_local_isolate_, ThreadKind::kBackground);
UnparkedScope unparked_scope(&isolate);
LocalHandleScope handle_scope(isolate.heap());
// Get a non-persistent handle to the newly compiled script.
isolate.heap()->AttachPersistentHandles(std::move(persistent_handles_));
Handle<Script> script = handle(*script_, &isolate);
persistent_handles_ = isolate.heap()->DetachPersistentHandles();
background_merge_task_.BeginMergeInBackground(&isolate, script);
}
MaybeHandle<SharedFunctionInfo> BackgroundDeserializeTask::Finish(
Isolate* isolate, Handle<String> source,
ScriptOriginOptions origin_options) {
@ -3306,15 +3274,9 @@ class StressBackgroundCompileThread : public base::Thread {
data()->task = std::make_unique<i::BackgroundCompileTask>(
data(), isolate, type,
ScriptCompiler::CompileOptions::kNoCompileOptions);
data()->task->SourceTextAvailable(isolate, source, script_details);
}
void Run() override {
data()->task->Run();
if (data()->task->ShouldMergeWithExistingScript()) {
data()->task->MergeWithExistingScript();
}
}
void Run() override { data()->task->Run(); }
ScriptStreamingData* data() { return streamed_source_.impl(); }
@ -3748,6 +3710,7 @@ Compiler::GetSharedFunctionInfoForStreamedScript(
BackgroundCompileTask* task = streaming_data->task.get();
MaybeHandle<SharedFunctionInfo> maybe_result;
MaybeHandle<Script> maybe_cached_script;
// Check if compile cache already holds the SFI, if so no need to finalize
// the code compiled on the background thread.
CompilationCache* compilation_cache = isolate->compilation_cache();
@ -3758,16 +3721,14 @@ Compiler::GetSharedFunctionInfoForStreamedScript(
compilation_cache->LookupScript(source, script_details,
task->flags().outer_language_mode());
// TODO(v8:12808): Determine what to do if we finish streaming and find that
// another copy of the Script already exists but has no root
// SharedFunctionInfo or has an uncompiled SharedFunctionInfo. For now, we
// just ignore it and create a new Script.
if (!lookup_result.toplevel_sfi().is_null()) {
maybe_result = lookup_result.toplevel_sfi();
}
if (!maybe_result.is_null()) {
compile_timer.set_hit_isolate_cache();
} else {
maybe_cached_script = lookup_result.script();
}
}
@ -3779,7 +3740,8 @@ Compiler::GetSharedFunctionInfoForStreamedScript(
TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("v8.compile"),
"V8.OffThreadFinalization.Publish");
maybe_result = task->FinalizeScript(isolate, source, script_details);
maybe_result = task->FinalizeScript(isolate, source, script_details,
maybe_cached_script);
Handle<SharedFunctionInfo> result;
if (maybe_result.ToHandle(&result)) {

26
src/codegen/compiler.h
View File

@ -534,28 +534,10 @@ class V8_EXPORT_PRIVATE BackgroundCompileTask {
void Run(LocalIsolate* isolate,
ReusableUnoptimizedCompileState* reusable_state);
// Checks the Isolate compilation cache to see whether it will be necessary to
// merge the newly compiled objects into an existing Script. This can change
// the value of ShouldMergeWithExistingScript, and embedders should check the
// latter after calling this. May only be called on a thread where the Isolate
// is currently entered.
void SourceTextAvailable(Isolate* isolate, Handle<String> source_text,
const ScriptDetails& script_details);
// Returns whether the embedder should call MergeWithExistingScript. This
// function may be called from any thread, any number of times, but its return
// value is only meaningful after SourceTextAvailable has completed.
bool ShouldMergeWithExistingScript() const;
// Partially merges newly compiled objects into an existing Script with the
// same source, and generates a list of follow-up work for the main thread.
// May be called from any thread, only once, and only if
// ShouldMergeWithExistingScript returned true.
void MergeWithExistingScript();
MaybeHandle<SharedFunctionInfo> FinalizeScript(
Isolate* isolate, Handle<String> source,
const ScriptDetails& script_details);
const ScriptDetails& script_details,
MaybeHandle<Script> maybe_cached_script);
bool FinalizeFunction(Isolate* isolate, Compiler::ClearExceptionFlag flag);
@ -593,10 +575,6 @@ class V8_EXPORT_PRIVATE BackgroundCompileTask {
int start_position_;
int end_position_;
int function_literal_id_;
// Task that merges newly compiled content into an existing Script from the
// Isolate compilation cache, if necessary.
BackgroundMergeTask background_merge_task_;
};
// Contains all data which needs to be transmitted between threads for

6
test/cctest/test-api.cc
View File

@ -23792,12 +23792,6 @@ TEST(StreamingWithIsolateScriptCache) {
// Variant of the above test which evicts the root SharedFunctionInfo from the
// Isolate script cache but still reuses the same Script.
TEST(StreamingWithIsolateScriptCacheClearingRootSFI) {
// TODO(v8:12808): Remove this check once background compilation is capable of
// reusing an existing Script.
if (i::v8_flags.stress_background_compile) {
return;
}
StreamingWithIsolateScriptCache(true);
}