Skip to content

Commit a55803a

Browse files
rmcilroyCommit Bot
rmcilroy
authored and
Commit Bot
committed
[SFI] Add support for flushing old Bytecode from SharedFunctionInfos.
This change makes the SFI to bytecode link pseudo-weak. The marking visitors check whether the bytecode is old, and if so, don't mark it and instead push the SFI onto a bytecode_flushing_candidates worklist. Once marking is complete, this list is walked, and for any of the candidates who's bytecode has not been marked (i.e., is only referenced by the shared function info), the bytecode is flushed and the SFI has the function data replaced with an UncompiledData (which overwrites the flushed bytecode array). Since we don't track JSFunctions, these can still think the underlying function is compiled, and so calling them will invoke InterpreterEntryTrampoline. As such, logic is added to InterpreterEntryTrampoline to detect flushed functions, and enter CompileLazy instead. BUG=v8:8395 Change-Id: I4afba79f814ca9a92dec45d59485935845a6669d Reviewed-on: https://chromium-review.googlesource.com/c/1348433 Commit-Queue: Ross McIlroy <rmcilroy@chromium.org> Reviewed-by: Ulan Degenbaev <ulan@chromium.org> Cr-Commit-Position: refs/heads/master@{#58158}
1 parent c1bf25b commit a55803a

19 files changed

+578
-134
lines changed

‎src/builtins/arm/builtins-arm.cc‎

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1026,6 +1026,20 @@ void Builtins::Generate_InterpreterEntryTrampoline(MacroAssembler* masm) {
10261026
Register closure = r1;
10271027
Register feedback_vector = r2;
10281028

1029+
// Get the bytecode array from the function object and load it into
1030+
// kInterpreterBytecodeArrayRegister.
1031+
__ ldr(r0, FieldMemOperand(closure, JSFunction::kSharedFunctionInfoOffset));
1032+
__ ldr(kInterpreterBytecodeArrayRegister,
1033+
FieldMemOperand(r0, SharedFunctionInfo::kFunctionDataOffset));
1034+
GetSharedFunctionInfoBytecode(masm, kInterpreterBytecodeArrayRegister, r4);
1035+
1036+
// The bytecode array could have been flushed from the shared function info,
1037+
// if so, call into CompileLazy.
1038+
Label compile_lazy;
1039+
__ CompareObjectType(kInterpreterBytecodeArrayRegister, r0, no_reg,
1040+
BYTECODE_ARRAY_TYPE);
1041+
__ b(ne, &compile_lazy);
1042+
10291043
// Load the feedback vector from the closure.
10301044
__ ldr(feedback_vector,
10311045
FieldMemOperand(closure, JSFunction::kFeedbackCellOffset));
@@ -1055,24 +1069,6 @@ void Builtins::Generate_InterpreterEntryTrampoline(MacroAssembler* masm) {
10551069
FrameScope frame_scope(masm, StackFrame::MANUAL);
10561070
__ PushStandardFrame(closure);
10571071

1058-
// Get the bytecode array from the function object and load it into
1059-
// kInterpreterBytecodeArrayRegister.
1060-
__ ldr(r0, FieldMemOperand(closure, JSFunction::kSharedFunctionInfoOffset));
1061-
__ ldr(kInterpreterBytecodeArrayRegister,
1062-
FieldMemOperand(r0, SharedFunctionInfo::kFunctionDataOffset));
1063-
GetSharedFunctionInfoBytecode(masm, kInterpreterBytecodeArrayRegister, r4);
1064-
1065-
// Check function data field is actually a BytecodeArray object.
1066-
if (FLAG_debug_code) {
1067-
__ SmiTst(kInterpreterBytecodeArrayRegister);
1068-
__ Assert(
1069-
ne, AbortReason::kFunctionDataShouldBeBytecodeArrayOnInterpreterEntry);
1070-
__ CompareObjectType(kInterpreterBytecodeArrayRegister, r0, no_reg,
1071-
BYTECODE_ARRAY_TYPE);
1072-
__ Assert(
1073-
eq, AbortReason::kFunctionDataShouldBeBytecodeArrayOnInterpreterEntry);
1074-
}
1075-
10761072
// Reset code age.
10771073
__ mov(r9, Operand(BytecodeArray::kNoAgeBytecodeAge));
10781074
__ strb(r9, FieldMemOperand(kInterpreterBytecodeArrayRegister,
@@ -1164,6 +1160,10 @@ void Builtins::Generate_InterpreterEntryTrampoline(MacroAssembler* masm) {
11641160
// The return value is in r0.
11651161
LeaveInterpreterFrame(masm, r2);
11661162
__ Jump(lr);
1163+
1164+
__ bind(&compile_lazy);
1165+
GenerateTailCallToReturnedCode(masm, Runtime::kCompileLazy);
1166+
__ bkpt(0); // Should not return.
11671167
}
11681168

11691169
static void Generate_InterpreterPushArgs(MacroAssembler* masm,

‎src/builtins/arm64/builtins-arm64.cc‎

Lines changed: 30 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -438,6 +438,17 @@ void Builtins::Generate_ConstructedNonConstructable(MacroAssembler* masm) {
438438
__ CallRuntime(Runtime::kThrowConstructedNonConstructable);
439439
}
440440

441+
static void GetSharedFunctionInfoBytecode(MacroAssembler* masm,
442+
Register sfi_data,
443+
Register scratch1) {
444+
Label done;
445+
__ CompareObjectType(sfi_data, scratch1, scratch1, INTERPRETER_DATA_TYPE);
446+
__ B(ne, &done);
447+
__ Ldr(sfi_data,
448+
FieldMemOperand(sfi_data, InterpreterData::kBytecodeArrayOffset));
449+
__ Bind(&done);
450+
}
451+
441452
// static
442453
void Builtins::Generate_ResumeGeneratorTrampoline(MacroAssembler* masm) {
443454
// ----------- S t a t e -------------
@@ -529,13 +540,9 @@ void Builtins::Generate_ResumeGeneratorTrampoline(MacroAssembler* masm) {
529540

530541
// Underlying function needs to have bytecode available.
531542
if (FLAG_debug_code) {
532-
Label check_has_bytecode_array;
533543
__ Ldr(x3, FieldMemOperand(x4, JSFunction::kSharedFunctionInfoOffset));
534544
__ Ldr(x3, FieldMemOperand(x3, SharedFunctionInfo::kFunctionDataOffset));
535-
__ CompareObjectType(x3, x0, x0, INTERPRETER_DATA_TYPE);
536-
__ B(ne, &check_has_bytecode_array);
537-
__ Ldr(x3, FieldMemOperand(x3, InterpreterData::kBytecodeArrayOffset));
538-
__ Bind(&check_has_bytecode_array);
545+
GetSharedFunctionInfoBytecode(masm, x3, x0);
539546
__ CompareObjectType(x3, x3, x3, BYTECODE_ARRAY_TYPE);
540547
__ Assert(eq, AbortReason::kMissingBytecodeArray);
541548
}
@@ -1134,6 +1141,20 @@ void Builtins::Generate_InterpreterEntryTrampoline(MacroAssembler* masm) {
11341141
Register closure = x1;
11351142
Register feedback_vector = x2;
11361143

1144+
// Get the bytecode array from the function object and load it into
1145+
// kInterpreterBytecodeArrayRegister.
1146+
__ Ldr(x0, FieldMemOperand(closure, JSFunction::kSharedFunctionInfoOffset));
1147+
__ Ldr(kInterpreterBytecodeArrayRegister,
1148+
FieldMemOperand(x0, SharedFunctionInfo::kFunctionDataOffset));
1149+
GetSharedFunctionInfoBytecode(masm, kInterpreterBytecodeArrayRegister, x11);
1150+
1151+
// The bytecode array could have been flushed from the shared function info,
1152+
// if so, call into CompileLazy.
1153+
Label compile_lazy;
1154+
__ CompareObjectType(kInterpreterBytecodeArrayRegister, x0, x0,
1155+
BYTECODE_ARRAY_TYPE);
1156+
__ B(ne, &compile_lazy);
1157+
11371158
// Load the feedback vector from the closure.
11381159
__ Ldr(feedback_vector,
11391160
FieldMemOperand(closure, JSFunction::kFeedbackCellOffset));
@@ -1165,31 +1186,6 @@ void Builtins::Generate_InterpreterEntryTrampoline(MacroAssembler* masm) {
11651186
__ Push(lr, fp, cp, closure);
11661187
__ Add(fp, sp, StandardFrameConstants::kFixedFrameSizeFromFp);
11671188

1168-
// Get the bytecode array from the function object and load it into
1169-
// kInterpreterBytecodeArrayRegister.
1170-
Label has_bytecode_array;
1171-
__ Ldr(x0, FieldMemOperand(closure, JSFunction::kSharedFunctionInfoOffset));
1172-
__ Ldr(kInterpreterBytecodeArrayRegister,
1173-
FieldMemOperand(x0, SharedFunctionInfo::kFunctionDataOffset));
1174-
__ CompareObjectType(kInterpreterBytecodeArrayRegister, x11, x11,
1175-
INTERPRETER_DATA_TYPE);
1176-
__ B(ne, &has_bytecode_array);
1177-
__ Ldr(kInterpreterBytecodeArrayRegister,
1178-
FieldMemOperand(kInterpreterBytecodeArrayRegister,
1179-
InterpreterData::kBytecodeArrayOffset));
1180-
__ Bind(&has_bytecode_array);
1181-
1182-
// Check function data field is actually a BytecodeArray object.
1183-
if (FLAG_debug_code) {
1184-
__ AssertNotSmi(
1185-
kInterpreterBytecodeArrayRegister,
1186-
AbortReason::kFunctionDataShouldBeBytecodeArrayOnInterpreterEntry);
1187-
__ CompareObjectType(kInterpreterBytecodeArrayRegister, x0, x0,
1188-
BYTECODE_ARRAY_TYPE);
1189-
__ Assert(
1190-
eq, AbortReason::kFunctionDataShouldBeBytecodeArrayOnInterpreterEntry);
1191-
}
1192-
11931189
// Reset code age.
11941190
__ Mov(x10, Operand(BytecodeArray::kNoAgeBytecodeAge));
11951191
__ Strb(x10, FieldMemOperand(kInterpreterBytecodeArrayRegister,
@@ -1289,6 +1285,10 @@ void Builtins::Generate_InterpreterEntryTrampoline(MacroAssembler* masm) {
12891285
// The return value is in x0.
12901286
LeaveInterpreterFrame(masm, x2);
12911287
__ Ret();
1288+
1289+
__ bind(&compile_lazy);
1290+
GenerateTailCallToReturnedCode(masm, Runtime::kCompileLazy);
1291+
__ Unreachable(); // Should not return.
12921292
}
12931293

12941294
static void Generate_InterpreterPushArgs(MacroAssembler* masm,

‎src/builtins/ia32/builtins-ia32.cc‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -946,6 +946,15 @@ static void AdvanceBytecodeOffsetOrReturn(MacroAssembler* masm,
946946
void Builtins::Generate_InterpreterEntryTrampoline(MacroAssembler* masm) {
947947
Register closure = edi;
948948

949+
// The bytecode array could have been flushed from the shared function info,
950+
// if so, call into CompileLazy.
951+
Label compile_lazy;
952+
__ mov(ecx, FieldOperand(edi, JSFunction::kSharedFunctionInfoOffset));
953+
__ mov(ecx, FieldOperand(ecx, SharedFunctionInfo::kFunctionDataOffset));
954+
GetSharedFunctionInfoBytecode(masm, ecx, eax);
955+
__ CmpObjectType(ecx, BYTECODE_ARRAY_TYPE, eax);
956+
__ j(not_equal, &compile_lazy);
957+
949958
Register feedback_vector = ecx;
950959
Label push_stack_frame;
951960
// Load feedback vector and check if it is valid. If valid, check for
@@ -981,9 +990,7 @@ void Builtins::Generate_InterpreterEntryTrampoline(MacroAssembler* masm) {
981990
__ mov(eax, FieldOperand(edi, JSFunction::kSharedFunctionInfoOffset));
982991
__ mov(kInterpreterBytecodeArrayRegister,
983992
FieldOperand(eax, SharedFunctionInfo::kFunctionDataOffset));
984-
__ Push(eax);
985993
GetSharedFunctionInfoBytecode(masm, kInterpreterBytecodeArrayRegister, eax);
986-
__ Pop(eax);
987994

988995
// Check function data field is actually a BytecodeArray object.
989996
if (FLAG_debug_code) {
@@ -1087,6 +1094,10 @@ void Builtins::Generate_InterpreterEntryTrampoline(MacroAssembler* masm) {
10871094
// The return value is in eax.
10881095
LeaveInterpreterFrame(masm, edx, ecx);
10891096
__ ret(0);
1097+
1098+
__ bind(&compile_lazy);
1099+
GenerateTailCallToReturnedCode(masm, Runtime::kCompileLazy);
1100+
__ int3(); // Should not return.
10901101
}
10911102

10921103

‎src/builtins/x64/builtins-x64.cc‎

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1049,6 +1049,20 @@ void Builtins::Generate_InterpreterEntryTrampoline(MacroAssembler* masm) {
10491049
Register closure = rdi;
10501050
Register feedback_vector = rbx;
10511051

1052+
// Get the bytecode array from the function object and load it into
1053+
// kInterpreterBytecodeArrayRegister.
1054+
__ movp(rax, FieldOperand(closure, JSFunction::kSharedFunctionInfoOffset));
1055+
__ movp(kInterpreterBytecodeArrayRegister,
1056+
FieldOperand(rax, SharedFunctionInfo::kFunctionDataOffset));
1057+
GetSharedFunctionInfoBytecode(masm, kInterpreterBytecodeArrayRegister,
1058+
kScratchRegister);
1059+
1060+
// The bytecode array could have been flushed from the shared function info,
1061+
// if so, call into CompileLazy.
1062+
Label compile_lazy;
1063+
__ CmpObjectType(kInterpreterBytecodeArrayRegister, BYTECODE_ARRAY_TYPE, rax);
1064+
__ j(not_equal, &compile_lazy);
1065+
10521066
// Load the feedback vector from the closure.
10531067
__ movp(feedback_vector,
10541068
FieldOperand(closure, JSFunction::kFeedbackCellOffset));
@@ -1077,24 +1091,6 @@ void Builtins::Generate_InterpreterEntryTrampoline(MacroAssembler* masm) {
10771091
__ Push(rsi); // Callee's context.
10781092
__ Push(rdi); // Callee's JS function.
10791093

1080-
// Get the bytecode array from the function object and load it into
1081-
// kInterpreterBytecodeArrayRegister.
1082-
__ movp(rax, FieldOperand(closure, JSFunction::kSharedFunctionInfoOffset));
1083-
__ movp(kInterpreterBytecodeArrayRegister,
1084-
FieldOperand(rax, SharedFunctionInfo::kFunctionDataOffset));
1085-
GetSharedFunctionInfoBytecode(masm, kInterpreterBytecodeArrayRegister,
1086-
kScratchRegister);
1087-
1088-
// Check function data field is actually a BytecodeArray object.
1089-
if (FLAG_debug_code) {
1090-
__ AssertNotSmi(kInterpreterBytecodeArrayRegister);
1091-
__ CmpObjectType(kInterpreterBytecodeArrayRegister, BYTECODE_ARRAY_TYPE,
1092-
rax);
1093-
__ Assert(
1094-
equal,
1095-
AbortReason::kFunctionDataShouldBeBytecodeArrayOnInterpreterEntry);
1096-
}
1097-
10981094
// Reset code age.
10991095
__ movb(FieldOperand(kInterpreterBytecodeArrayRegister,
11001096
BytecodeArray::kBytecodeAgeOffset),
@@ -1192,6 +1188,10 @@ void Builtins::Generate_InterpreterEntryTrampoline(MacroAssembler* masm) {
11921188
// The return value is in rax.
11931189
LeaveInterpreterFrame(masm, rbx, rcx);
11941190
__ ret(0);
1191+
1192+
__ bind(&compile_lazy);
1193+
GenerateTailCallToReturnedCode(masm, Runtime::kCompileLazy);
1194+
__ int3(); // Should not return.
11951195
}
11961196

11971197
static void Generate_InterpreterPushArgs(MacroAssembler* masm,

‎src/compiler.cc‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -369,7 +369,8 @@ void InstallUnoptimizedCode(UnoptimizedCompilationInfo* compilation_info,
369369
}
370370

371371
// Install coverage info on the shared function info.
372-
if (compilation_info->has_coverage_info()) {
372+
if (compilation_info->has_coverage_info() &&
373+
!shared_info->HasCoverageInfo()) {
373374
DCHECK(isolate->is_block_code_coverage());
374375
isolate->debug()->InstallCoverageInfo(shared_info,
375376
compilation_info->coverage_info());
@@ -1159,7 +1160,7 @@ bool Compiler::Compile(Handle<SharedFunctionInfo> shared_info,
11591160
}
11601161

11611162
// Parse and update ParseInfo with the results.
1162-
if (!parsing::ParseFunction(&parse_info, shared_info, isolate)) {
1163+
if (!parsing::ParseAny(&parse_info, shared_info, isolate)) {
11631164
return FailWithPendingException(isolate, &parse_info, flag);
11641165
}
11651166

‎src/flag-definitions.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -765,6 +765,9 @@ DEFINE_BOOL(always_compact, false, "Perform compaction on every full GC")
765765
DEFINE_BOOL(never_compact, false,
766766
"Never perform compaction on full GC - testing only")
767767
DEFINE_BOOL(compact_code_space, true, "Compact code space on full collections")
768+
DEFINE_BOOL(flush_bytecode, false,
769+
"flush of bytecode when it has not been executed recently")
770+
DEFINE_BOOL(stress_flush_bytecode, false, "stress bytecode flushing")
768771
DEFINE_BOOL(use_marking_progress_bar, true,
769772
"Use a progress bar to scan large objects in increments when "
770773
"incremental marking is active.")

‎src/heap-symbols.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -381,6 +381,7 @@
381381
F(HEAP_PROLOGUE) \
382382
TOP_MC_SCOPES(F) \
383383
F(MC_CLEAR_DEPENDENT_CODE) \
384+
F(MC_CLEAR_FLUSHABLE_BYTECODE) \
384385
F(MC_CLEAR_MAPS) \
385386
F(MC_CLEAR_SLOTS_BUFFER) \
386387
F(MC_CLEAR_STORE_BUFFER) \

‎src/heap/concurrent-marking.cc‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -316,6 +316,25 @@ class ConcurrentMarkingVisitor final
316316
// Side-effectful visitation.
317317
// ===========================================================================
318318

319+
int VisitSharedFunctionInfo(Map map, SharedFunctionInfo shared_info) {
320+
if (!ShouldVisit(shared_info)) return 0;
321+
322+
int size = SharedFunctionInfo::BodyDescriptor::SizeOf(map, shared_info);
323+
VisitMapPointer(shared_info, shared_info->map_slot());
324+
SharedFunctionInfo::BodyDescriptor::IterateBody(map, shared_info, size,
325+
this);
326+
327+
// If the SharedFunctionInfo has old bytecode, mark it as flushable,
328+
// otherwise visit the function data field strongly.
329+
if (shared_info->ShouldFlushBytecode()) {
330+
weak_objects_->bytecode_flushing_candidates.Push(task_id_, shared_info);
331+
} else {
332+
VisitPointer(shared_info, shared_info->RawField(
333+
SharedFunctionInfo::kFunctionDataOffset));
334+
}
335+
return size;
336+
}
337+
319338
int VisitBytecodeArray(Map map, BytecodeArray object) {
320339
if (!ShouldVisit(object)) return 0;
321340
int size = BytecodeArray::BodyDescriptor::SizeOf(map, object);
@@ -700,6 +719,7 @@ void ConcurrentMarking::Run(int task_id, TaskState* task_state) {
700719
weak_objects_->weak_references.FlushToGlobal(task_id);
701720
weak_objects_->js_weak_cells.FlushToGlobal(task_id);
702721
weak_objects_->weak_objects_in_code.FlushToGlobal(task_id);
722+
weak_objects_->bytecode_flushing_candidates.FlushToGlobal(task_id);
703723
base::AsAtomicWord::Relaxed_Store<size_t>(&task_state->marked_bytes, 0);
704724
total_marked_bytes_ += marked_bytes;
705725

‎src/heap/factory.cc‎

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2645,12 +2645,9 @@ Factory::NewUncompiledDataWithoutPreParsedScope(Handle<String> inferred_name,
26452645
UncompiledDataWithoutPreParsedScope::cast(
26462646
New(uncompiled_data_without_pre_parsed_scope_map(), TENURED)),
26472647
isolate());
2648-
result->set_inferred_name(*inferred_name);
2649-
result->set_start_position(start_position);
2650-
result->set_end_position(end_position);
2651-
result->set_function_literal_id(function_literal_id);
26522648

2653-
result->clear_padding();
2649+
UncompiledData::Initialize(*result, *inferred_name, start_position,
2650+
end_position, function_literal_id);
26542651
return result;
26552652
}
26562653

@@ -2663,13 +2660,11 @@ Factory::NewUncompiledDataWithPreParsedScope(
26632660
UncompiledDataWithPreParsedScope::cast(
26642661
New(uncompiled_data_with_pre_parsed_scope_map(), TENURED)),
26652662
isolate());
2666-
result->set_inferred_name(*inferred_name);
2667-
result->set_start_position(start_position);
2668-
result->set_end_position(end_position);
2669-
result->set_function_literal_id(function_literal_id);
2670-
result->set_pre_parsed_scope_data(*pre_parsed_scope_data);
26712663

2672-
result->clear_padding();
2664+
UncompiledDataWithPreParsedScope::Initialize(
2665+
*result, *inferred_name, start_position, end_position,
2666+
function_literal_id, *pre_parsed_scope_data);
2667+
26732668
return result;
26742669
}
26752670

‎src/heap/incremental-marking.cc‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -738,6 +738,12 @@ void IncrementalMarking::UpdateWeakReferencesAfterScavenge() {
738738
weak_objects_->current_ephemerons.Update(ephemeron_updater);
739739
weak_objects_->next_ephemerons.Update(ephemeron_updater);
740740
weak_objects_->discovered_ephemerons.Update(ephemeron_updater);
741+
#ifdef DEBUG
742+
weak_objects_->bytecode_flushing_candidates.Iterate(
743+
[](SharedFunctionInfo candidate) {
744+
DCHECK(!Heap::InNewSpace(candidate));
745+
});
746+
#endif
741747
}
742748

743749
void IncrementalMarking::UpdateMarkedBytesAfterScavenge(

0 commit comments

Comments
 (0)