Skip to content

Commit

Permalink
deps: V8: cherry-pick cd10ad7cdbe5
Browse files Browse the repository at this point in the history
Original commit message:

    [compiler] reset script details in functions deserialized from code cache

    During the serialization of the code cache, V8 would wipe out the
    host-defined options, so after a script id deserialized from the
    code cache, the host-defined options need to be reset on the script
    using what's provided by the embedder when doing the deserializing
    compilation, otherwise the HostImportModuleDynamically callbacks
    can't get the data it needs to implement dynamic import().

    Change-Id: I33cc6a5e43b6469d3527242e083f7ae6d8ed0c6a
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5401780
    Reviewed-by: Leszek Swirski <[email protected]>
    Commit-Queue: Joyee Cheung <[email protected]>
    Cr-Commit-Position: refs/heads/main@{#93323}

Refs: v8/v8@cd10ad7
PR-URL: #52535
Refs: #47472
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
PR-URL: #52465
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
  • Loading branch information
joyeecheung authored and RafaelGSS committed Apr 22, 2024
1 parent 521b629 commit 30801b8
Show file tree
Hide file tree
Showing 7 changed files with 247 additions and 27 deletions.
2 changes: 1 addition & 1 deletion common.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@

# Reset this number to 0 on major V8 upgrades.
# Increment by one for each non-official patch applied to deps/v8.
'v8_embedder_string': '-node.9',
'v8_embedder_string': '-node.10',

##### V8 defaults for Node.js #####

Expand Down
19 changes: 9 additions & 10 deletions deps/v8/src/codegen/compiler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1720,10 +1720,8 @@ BackgroundCompileTask::BackgroundCompileTask(

BackgroundCompileTask::~BackgroundCompileTask() = default;

namespace {

void SetScriptFieldsFromDetails(Isolate* isolate, Tagged<Script> script,
ScriptDetails script_details,
const ScriptDetails& script_details,
DisallowGarbageCollection* no_gc) {
Handle<Object> script_name;
if (script_details.name_obj.ToHandle(&script_name)) {
Expand All @@ -1749,6 +1747,8 @@ void SetScriptFieldsFromDetails(Isolate* isolate, Tagged<Script> script,
}
}

namespace {

#ifdef ENABLE_SLOW_DCHECKS

// A class which traverses the object graph for a newly compiled Script and
Expand Down Expand Up @@ -2459,10 +2459,10 @@ void BackgroundDeserializeTask::MergeWithExistingScript() {

MaybeHandle<SharedFunctionInfo> BackgroundDeserializeTask::Finish(
Isolate* isolate, Handle<String> source,
ScriptOriginOptions origin_options) {
const ScriptDetails& script_details) {
return CodeSerializer::FinishOffThreadDeserialize(
isolate, std::move(off_thread_data_), &cached_data_, source,
origin_options, &background_merge_task_);
script_details, &background_merge_task_);
}

// ----------------------------------------------------------------------------
Expand Down Expand Up @@ -3639,8 +3639,8 @@ MaybeHandle<SharedFunctionInfo> GetSharedFunctionInfoForScriptImpl(
"V8.CompileDeserialize");
if (deserialize_task) {
// If there's a cache consume task, finish it.
maybe_result = deserialize_task->Finish(isolate, source,
script_details.origin_options);
maybe_result =
deserialize_task->Finish(isolate, source, script_details);
// It is possible at this point that there is a Script object for this
// script in the compilation cache (held in the variable maybe_script),
// which does not match maybe_result->script(). This could happen any of
Expand All @@ -3661,8 +3661,7 @@ MaybeHandle<SharedFunctionInfo> GetSharedFunctionInfoForScriptImpl(
// would be non-trivial.
} else {
maybe_result = CodeSerializer::Deserialize(
isolate, cached_data, source, script_details.origin_options,
maybe_script);
isolate, cached_data, source, script_details, maybe_script);
}

bool consuming_code_cache_succeeded = false;
Expand Down Expand Up @@ -3838,7 +3837,7 @@ MaybeHandle<JSFunction> Compiler::GetWrappedFunction(
TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("v8.compile"),
"V8.CompileDeserialize");
maybe_result = CodeSerializer::Deserialize(isolate, cached_data, source,
script_details.origin_options);
script_details);
bool consuming_code_cache_succeeded = false;
if (maybe_result.ToHandle(&result)) {
is_compiled_scope = result->is_compiled_scope(isolate);
Expand Down
2 changes: 1 addition & 1 deletion deps/v8/src/codegen/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -682,7 +682,7 @@ class V8_EXPORT_PRIVATE BackgroundDeserializeTask {

MaybeHandle<SharedFunctionInfo> Finish(Isolate* isolate,
Handle<String> source,
ScriptOriginOptions origin_options);
const ScriptDetails& script_details);

bool rejected() const { return cached_data_.rejected(); }

Expand Down
3 changes: 3 additions & 0 deletions deps/v8/src/codegen/script-details.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ struct ScriptDetails {
const ScriptOriginOptions origin_options;
};

void SetScriptFieldsFromDetails(Isolate* isolate, Tagged<Script> script,
const ScriptDetails& script_details,
DisallowGarbageCollection* no_gc);
} // namespace internal
} // namespace v8

Expand Down
34 changes: 21 additions & 13 deletions deps/v8/src/snapshot/code-serializer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -313,12 +313,12 @@ class StressOffThreadDeserializeThread final : public base::Thread {
CodeSerializer::StartDeserializeOffThread(&local_isolate, cached_data_);
}

MaybeHandle<SharedFunctionInfo> Finalize(Isolate* isolate,
Handle<String> source,
ScriptOriginOptions origin_options) {
MaybeHandle<SharedFunctionInfo> Finalize(
Isolate* isolate, Handle<String> source,
const ScriptDetails& script_details) {
return CodeSerializer::FinishOffThreadDeserialize(
isolate, std::move(off_thread_data_), cached_data_, source,
origin_options);
script_details);
}

private:
Expand All @@ -329,7 +329,8 @@ class StressOffThreadDeserializeThread final : public base::Thread {

void FinalizeDeserialization(Isolate* isolate,
Handle<SharedFunctionInfo> result,
const base::ElapsedTimer& timer) {
const base::ElapsedTimer& timer,
const ScriptDetails& script_details) {
// Devtools can report time in this function as profiler overhead, since none
// of the following tasks would need to happen normally.
TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("v8.compile"),
Expand All @@ -342,10 +343,16 @@ void FinalizeDeserialization(Isolate* isolate,
log_code_creation);
}

Handle<Script> script(Script::cast(result->script()), isolate);
// Reset the script details, including host-defined options.
{
DisallowGarbageCollection no_gc;
SetScriptFieldsFromDetails(isolate, *script, script_details, &no_gc);
}

bool needs_source_positions = isolate->NeedsSourcePositions();
if (!log_code_creation && !needs_source_positions) return;

Handle<Script> script(Script::cast(result->script()), isolate);
if (needs_source_positions) {
Script::InitLineEnds(isolate, script);
}
Expand Down Expand Up @@ -429,13 +436,13 @@ const char* ToString(SerializedCodeSanityCheckResult result) {

MaybeHandle<SharedFunctionInfo> CodeSerializer::Deserialize(
Isolate* isolate, AlignedCachedData* cached_data, Handle<String> source,
ScriptOriginOptions origin_options,
const ScriptDetails& script_details,
MaybeHandle<Script> maybe_cached_script) {
if (v8_flags.stress_background_compile) {
StressOffThreadDeserializeThread thread(isolate, cached_data);
CHECK(thread.Start());
thread.Join();
return thread.Finalize(isolate, source, origin_options);
return thread.Finalize(isolate, source, script_details);
// TODO(leszeks): Compare off-thread deserialized data to on-thread.
}

Expand All @@ -450,7 +457,7 @@ MaybeHandle<SharedFunctionInfo> CodeSerializer::Deserialize(
SerializedCodeSanityCheckResult::kSuccess;
const SerializedCodeData scd = SerializedCodeData::FromCachedData(
isolate, cached_data,
SerializedCodeData::SourceHash(source, origin_options),
SerializedCodeData::SourceHash(source, script_details.origin_options),
&sanity_check_result);
if (sanity_check_result != SerializedCodeSanityCheckResult::kSuccess) {
if (v8_flags.profile_deserialization) {
Expand Down Expand Up @@ -497,7 +504,7 @@ MaybeHandle<SharedFunctionInfo> CodeSerializer::Deserialize(
PrintF("[Deserializing from %d bytes took %0.3f ms]\n", length, ms);
}

FinalizeDeserialization(isolate, result, timer);
FinalizeDeserialization(isolate, result, timer, script_details);

return scope.CloseAndEscape(result);
}
Expand Down Expand Up @@ -552,7 +559,7 @@ CodeSerializer::StartDeserializeOffThread(LocalIsolate* local_isolate,
MaybeHandle<SharedFunctionInfo> CodeSerializer::FinishOffThreadDeserialize(
Isolate* isolate, OffThreadDeserializeData&& data,
AlignedCachedData* cached_data, Handle<String> source,
ScriptOriginOptions origin_options,
const ScriptDetails& script_details,
BackgroundMergeTask* background_merge_task) {
base::ElapsedTimer timer;
if (v8_flags.profile_deserialization || v8_flags.log_function_events) {
Expand All @@ -568,7 +575,8 @@ MaybeHandle<SharedFunctionInfo> CodeSerializer::FinishOffThreadDeserialize(
data.sanity_check_result;
const SerializedCodeData scd =
SerializedCodeData::FromPartiallySanityCheckedCachedData(
cached_data, SerializedCodeData::SourceHash(source, origin_options),
cached_data,
SerializedCodeData::SourceHash(source, script_details.origin_options),
&sanity_check_result);
if (sanity_check_result != SerializedCodeSanityCheckResult::kSuccess) {
// The only case where the deserialization result could exist despite a
Expand Down Expand Up @@ -641,7 +649,7 @@ MaybeHandle<SharedFunctionInfo> CodeSerializer::FinishOffThreadDeserialize(
length, ms);
}

FinalizeDeserialization(isolate, result, timer);
FinalizeDeserialization(isolate, result, timer, script_details);

DCHECK(!background_merge_task ||
!background_merge_task->HasPendingForegroundWork());
Expand Down
5 changes: 3 additions & 2 deletions deps/v8/src/snapshot/code-serializer.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#define V8_SNAPSHOT_CODE_SERIALIZER_H_

#include "src/base/macros.h"
#include "src/codegen/script-details.h"
#include "src/snapshot/serializer.h"
#include "src/snapshot/snapshot-data.h"

Expand Down Expand Up @@ -81,7 +82,7 @@ class CodeSerializer : public Serializer {

V8_WARN_UNUSED_RESULT static MaybeHandle<SharedFunctionInfo> Deserialize(
Isolate* isolate, AlignedCachedData* cached_data, Handle<String> source,
ScriptOriginOptions origin_options,
const ScriptDetails& script_details,
MaybeHandle<Script> maybe_cached_script = {});

V8_WARN_UNUSED_RESULT static OffThreadDeserializeData
Expand All @@ -92,7 +93,7 @@ class CodeSerializer : public Serializer {
FinishOffThreadDeserialize(
Isolate* isolate, OffThreadDeserializeData&& data,
AlignedCachedData* cached_data, Handle<String> source,
ScriptOriginOptions origin_options,
const ScriptDetails& script_details,
BackgroundMergeTask* background_merge_task = nullptr);

uint32_t source_hash() const { return source_hash_; }
Expand Down
Loading

0 comments on commit 30801b8

Please sign in to comment.