Skip to content

Commit cd10ad7

Browse files
joyeecheungV8 LUCI CQ
joyeecheung
authored and
V8 LUCI CQ
committed
[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}
1 parent ad09ef3 commit cd10ad7

File tree

6 files changed

+246
-26
lines changed

6 files changed

+246
-26
lines changed

‎src/codegen/compiler.cc‎

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1720,10 +1720,8 @@ BackgroundCompileTask::BackgroundCompileTask(
17201720

17211721
BackgroundCompileTask::~BackgroundCompileTask() = default;
17221722

1723-
namespace {
1724-
17251723
void SetScriptFieldsFromDetails(Isolate* isolate, Tagged<Script> script,
1726-
ScriptDetails script_details,
1724+
const ScriptDetails& script_details,
17271725
DisallowGarbageCollection* no_gc) {
17281726
Handle<Object> script_name;
17291727
if (script_details.name_obj.ToHandle(&script_name)) {
@@ -1749,6 +1747,8 @@ void SetScriptFieldsFromDetails(Isolate* isolate, Tagged<Script> script,
17491747
}
17501748
}
17511749

1750+
namespace {
1751+
17521752
#ifdef ENABLE_SLOW_DCHECKS
17531753

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

24602460
MaybeHandle<SharedFunctionInfo> BackgroundDeserializeTask::Finish(
24612461
Isolate* isolate, Handle<String> source,
2462-
ScriptOriginOptions origin_options) {
2462+
const ScriptDetails& script_details) {
24632463
return CodeSerializer::FinishOffThreadDeserialize(
24642464
isolate, std::move(off_thread_data_), &cached_data_, source,
2465-
origin_options, &background_merge_task_);
2465+
script_details, &background_merge_task_);
24662466
}
24672467

24682468
// ----------------------------------------------------------------------------
@@ -3630,8 +3630,8 @@ MaybeHandle<SharedFunctionInfo> GetSharedFunctionInfoForScriptImpl(
36303630
"V8.CompileDeserialize");
36313631
if (deserialize_task) {
36323632
// If there's a cache consume task, finish it.
3633-
maybe_result = deserialize_task->Finish(isolate, source,
3634-
script_details.origin_options);
3633+
maybe_result =
3634+
deserialize_task->Finish(isolate, source, script_details);
36353635
// It is possible at this point that there is a Script object for this
36363636
// script in the compilation cache (held in the variable maybe_script),
36373637
// which does not match maybe_result->script(). This could happen any of
@@ -3652,8 +3652,7 @@ MaybeHandle<SharedFunctionInfo> GetSharedFunctionInfoForScriptImpl(
36523652
// would be non-trivial.
36533653
} else {
36543654
maybe_result = CodeSerializer::Deserialize(
3655-
isolate, cached_data, source, script_details.origin_options,
3656-
maybe_script);
3655+
isolate, cached_data, source, script_details, maybe_script);
36573656
}
36583657

36593658
bool consuming_code_cache_succeeded = false;
@@ -3829,7 +3828,7 @@ MaybeHandle<JSFunction> Compiler::GetWrappedFunction(
38293828
TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("v8.compile"),
38303829
"V8.CompileDeserialize");
38313830
maybe_result = CodeSerializer::Deserialize(isolate, cached_data, source,
3832-
script_details.origin_options);
3831+
script_details);
38333832
bool consuming_code_cache_succeeded = false;
38343833
if (maybe_result.ToHandle(&result)) {
38353834
is_compiled_scope = result->is_compiled_scope(isolate);

‎src/codegen/compiler.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -682,7 +682,7 @@ class V8_EXPORT_PRIVATE BackgroundDeserializeTask {
682682

683683
MaybeHandle<SharedFunctionInfo> Finish(Isolate* isolate,
684684
Handle<String> source,
685-
ScriptOriginOptions origin_options);
685+
const ScriptDetails& script_details);
686686

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

‎src/codegen/script-details.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,9 @@ struct ScriptDetails {
3535
const ScriptOriginOptions origin_options;
3636
};
3737

38+
void SetScriptFieldsFromDetails(Isolate* isolate, Tagged<Script> script,
39+
const ScriptDetails& script_details,
40+
DisallowGarbageCollection* no_gc);
3841
} // namespace internal
3942
} // namespace v8
4043

‎src/snapshot/code-serializer.cc‎

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -313,12 +313,12 @@ class StressOffThreadDeserializeThread final : public base::Thread {
313313
CodeSerializer::StartDeserializeOffThread(&local_isolate, cached_data_);
314314
}
315315

316-
MaybeHandle<SharedFunctionInfo> Finalize(Isolate* isolate,
317-
Handle<String> source,
318-
ScriptOriginOptions origin_options) {
316+
MaybeHandle<SharedFunctionInfo> Finalize(
317+
Isolate* isolate, Handle<String> source,
318+
const ScriptDetails& script_details) {
319319
return CodeSerializer::FinishOffThreadDeserialize(
320320
isolate, std::move(off_thread_data_), cached_data_, source,
321-
origin_options);
321+
script_details);
322322
}
323323

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

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

346+
Handle<Script> script(Script::cast(result->script()), isolate);
347+
// Reset the script details, including host-defined options.
348+
{
349+
DisallowGarbageCollection no_gc;
350+
SetScriptFieldsFromDetails(isolate, *script, script_details, &no_gc);
351+
}
352+
345353
bool needs_source_positions = isolate->NeedsSourcePositions();
346354
if (!log_code_creation && !needs_source_positions) return;
347355

348-
Handle<Script> script(Script::cast(result->script()), isolate);
349356
if (needs_source_positions) {
350357
Script::InitLineEnds(isolate, script);
351358
}
@@ -429,13 +436,13 @@ const char* ToString(SerializedCodeSanityCheckResult result) {
429436

430437
MaybeHandle<SharedFunctionInfo> CodeSerializer::Deserialize(
431438
Isolate* isolate, AlignedCachedData* cached_data, Handle<String> source,
432-
ScriptOriginOptions origin_options,
439+
const ScriptDetails& script_details,
433440
MaybeHandle<Script> maybe_cached_script) {
434441
if (v8_flags.stress_background_compile) {
435442
StressOffThreadDeserializeThread thread(isolate, cached_data);
436443
CHECK(thread.Start());
437444
thread.Join();
438-
return thread.Finalize(isolate, source, origin_options);
445+
return thread.Finalize(isolate, source, script_details);
439446
// TODO(leszeks): Compare off-thread deserialized data to on-thread.
440447
}
441448

@@ -450,7 +457,7 @@ MaybeHandle<SharedFunctionInfo> CodeSerializer::Deserialize(
450457
SerializedCodeSanityCheckResult::kSuccess;
451458
const SerializedCodeData scd = SerializedCodeData::FromCachedData(
452459
isolate, cached_data,
453-
SerializedCodeData::SourceHash(source, origin_options),
460+
SerializedCodeData::SourceHash(source, script_details.origin_options),
454461
&sanity_check_result);
455462
if (sanity_check_result != SerializedCodeSanityCheckResult::kSuccess) {
456463
if (v8_flags.profile_deserialization) {
@@ -497,7 +504,7 @@ MaybeHandle<SharedFunctionInfo> CodeSerializer::Deserialize(
497504
PrintF("[Deserializing from %d bytes took %0.3f ms]\n", length, ms);
498505
}
499506

500-
FinalizeDeserialization(isolate, result, timer);
507+
FinalizeDeserialization(isolate, result, timer, script_details);
501508

502509
return scope.CloseAndEscape(result);
503510
}
@@ -552,7 +559,7 @@ CodeSerializer::StartDeserializeOffThread(LocalIsolate* local_isolate,
552559
MaybeHandle<SharedFunctionInfo> CodeSerializer::FinishOffThreadDeserialize(
553560
Isolate* isolate, OffThreadDeserializeData&& data,
554561
AlignedCachedData* cached_data, Handle<String> source,
555-
ScriptOriginOptions origin_options,
562+
const ScriptDetails& script_details,
556563
BackgroundMergeTask* background_merge_task) {
557564
base::ElapsedTimer timer;
558565
if (v8_flags.profile_deserialization || v8_flags.log_function_events) {
@@ -568,7 +575,8 @@ MaybeHandle<SharedFunctionInfo> CodeSerializer::FinishOffThreadDeserialize(
568575
data.sanity_check_result;
569576
const SerializedCodeData scd =
570577
SerializedCodeData::FromPartiallySanityCheckedCachedData(
571-
cached_data, SerializedCodeData::SourceHash(source, origin_options),
578+
cached_data,
579+
SerializedCodeData::SourceHash(source, script_details.origin_options),
572580
&sanity_check_result);
573581
if (sanity_check_result != SerializedCodeSanityCheckResult::kSuccess) {
574582
// The only case where the deserialization result could exist despite a
@@ -641,7 +649,7 @@ MaybeHandle<SharedFunctionInfo> CodeSerializer::FinishOffThreadDeserialize(
641649
length, ms);
642650
}
643651

644-
FinalizeDeserialization(isolate, result, timer);
652+
FinalizeDeserialization(isolate, result, timer, script_details);
645653

646654
DCHECK(!background_merge_task ||
647655
!background_merge_task->HasPendingForegroundWork());

‎src/snapshot/code-serializer.h‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
#define V8_SNAPSHOT_CODE_SERIALIZER_H_
77

88
#include "src/base/macros.h"
9+
#include "src/codegen/script-details.h"
910
#include "src/snapshot/serializer.h"
1011
#include "src/snapshot/snapshot-data.h"
1112

@@ -81,7 +82,7 @@ class CodeSerializer : public Serializer {
8182

8283
V8_WARN_UNUSED_RESULT static MaybeHandle<SharedFunctionInfo> Deserialize(
8384
Isolate* isolate, AlignedCachedData* cached_data, Handle<String> source,
84-
ScriptOriginOptions origin_options,
85+
const ScriptDetails& script_details,
8586
MaybeHandle<Script> maybe_cached_script = {});
8687

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

9899
uint32_t source_hash() const { return source_hash_; }

0 commit comments

Comments
 (0)