[ic] Allow resetting interrupt budget on IC change

Add an alternative to any_ic_changed_, where instead of a global flag
that is updated on ICs changed (which prevents small function
optimisation), the interrupt budget of the particular function whose IC
was updated is reset to a default value.

This should have a similar effect, allowing small functions to tier up
quickly but still only once they have been stable enough for some time,
but should prevent cross-contamination of different functions'
stabilities due to the global nature of the flag.

It does, however, require a back pointer from the feedback vector to its
parent feedback cell (which holds the interrupt budget).

Drive-by, use any_ic_changed_ for Maglev tierup, to match small
function behaviour.

Change-Id: I7109cf3aff536af7ab36d3564ec8005ee7aa44f6
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4156472
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Cr-Commit-Position: refs/heads/main@{#85240}
This commit is contained in:
Leszek Swirski 2023-01-11 17:08:13 +01:00 committed by V8 LUCI CQ
parent 7777e0321d
commit ed47b8cd0c
14 changed files with 62 additions and 57 deletions

View File

@ -15,6 +15,7 @@
#include "src/diagnostics/code-tracer.h"
#include "src/execution/execution.h"
#include "src/execution/frames-inl.h"
#include "src/flags/flags.h"
#include "src/handles/global-handles.h"
#include "src/init/bootstrapper.h"
#include "src/interpreter/interpreter.h"
@ -344,6 +345,7 @@ OptimizationDecision TieringManager::ShouldOptimize(
if (TiersUpToMaglev(calling_code_kind) &&
function.shared().PassesFilter(v8_flags.maglev_filter) &&
!function.shared(isolate_).maglev_compilation_failed()) {
if (any_ic_changed_) return OptimizationDecision::DoNotOptimize();
return OptimizationDecision::Maglev();
} else if (calling_code_kind == CodeKind::TURBOFAN) {
// Already in the top tier.
@ -385,6 +387,22 @@ OptimizationDecision TieringManager::ShouldOptimize(
return OptimizationDecision::DoNotOptimize();
}
void TieringManager::NotifyICChanged(FeedbackVector vector) {
if (v8_flags.global_ic_updated_flag) {
any_ic_changed_ = true;
}
if (v8_flags.reset_interrupt_on_ic_update) {
CodeKind code_kind = vector.has_optimized_code()
? vector.optimized_code().kind()
: vector.shared_function_info().HasBaselineCode()
? CodeKind::BASELINE
: CodeKind::INTERPRETED_FUNCTION;
int interrupt_budget =
::i::InterruptBudgetFor(code_kind, vector.tiering_state());
vector.parent_feedback_cell().set_interrupt_budget(interrupt_budget);
}
}
TieringManager::OnInterruptTickScope::OnInterruptTickScope(
TieringManager* profiler)
: profiler_(profiler) {

View File

@ -28,7 +28,7 @@ class TieringManager {
void OnInterruptTick(Handle<JSFunction> function, CodeKind code_kind);
void NotifyICChanged() { any_ic_changed_ = true; }
void NotifyICChanged(FeedbackVector vector);
// After this request, the next JumpLoop will perform OSR.
void RequestOsrAtNextOpportunity(JSFunction function);

View File

@ -632,6 +632,11 @@ DEFINE_INT(bytecode_size_allowance_per_tick, 150,
DEFINE_INT(
max_bytecode_size_for_early_opt, 81,
"Maximum bytecode length for a function to be optimized on the first tick")
DEFINE_BOOL(global_ic_updated_flag, true,
"Track, globally, whether any IC changed, and use this in tierup "
"heuristics.")
DEFINE_BOOL(reset_interrupt_on_ic_update, false,
"On IC change, reset the interrupt budget for just that function.")
// Flags for inline caching and feedback vectors.
DEFINE_BOOL(use_ic, true, "use inline caching")

View File

@ -495,7 +495,8 @@ Handle<ClosureFeedbackCellArray> Factory::NewClosureFeedbackCellArray(
Handle<FeedbackVector> Factory::NewFeedbackVector(
Handle<SharedFunctionInfo> shared,
Handle<ClosureFeedbackCellArray> closure_feedback_cell_array) {
Handle<ClosureFeedbackCellArray> closure_feedback_cell_array,
Handle<FeedbackCell> parent_feedback_cell) {
int length = shared->feedback_metadata().slot_count();
DCHECK_LE(0, length);
int size = FeedbackVector::SizeFor(length);
@ -514,6 +515,7 @@ Handle<FeedbackVector> Factory::NewFeedbackVector(
vector.reset_flags();
vector.set_log_next_execution(v8_flags.log_function_events);
vector.set_closure_feedback_cell_array(*closure_feedback_cell_array);
vector.set_parent_feedback_cell(*parent_feedback_cell);
// TODO(leszeks): Initialize based on the feedback metadata.
MemsetTagged(ObjectSlot(vector.slots_start()), *undefined_value(), length);

View File

@ -148,7 +148,8 @@ class V8_EXPORT_PRIVATE Factory : public FactoryBase<Factory> {
// values.
Handle<FeedbackVector> NewFeedbackVector(
Handle<SharedFunctionInfo> shared,
Handle<ClosureFeedbackCellArray> closure_feedback_cell_array);
Handle<ClosureFeedbackCellArray> closure_feedback_cell_array,
Handle<FeedbackCell> parent_feedback_cell);
// Allocates a clean embedder data array with given capacity.
Handle<EmbedderDataArray> NewEmbedderDataArray(int length);

View File

@ -341,7 +341,7 @@ void IC::OnFeedbackChanged(Isolate* isolate, FeedbackVector vector,
}
#endif
isolate->tiering_manager()->NotifyICChanged();
isolate->tiering_manager()->NotifyICChanged(vector);
}
namespace {

View File

@ -215,6 +215,7 @@ Handle<ClosureFeedbackCellArray> ClosureFeedbackCellArray::New(
Handle<FeedbackVector> FeedbackVector::New(
Isolate* isolate, Handle<SharedFunctionInfo> shared,
Handle<ClosureFeedbackCellArray> closure_feedback_cell_array,
Handle<FeedbackCell> parent_feedback_cell,
IsCompiledScope* is_compiled_scope) {
DCHECK(is_compiled_scope->is_compiled());
Factory* factory = isolate->factory();
@ -223,8 +224,8 @@ Handle<FeedbackVector> FeedbackVector::New(
isolate);
const int slot_count = feedback_metadata->slot_count();
Handle<FeedbackVector> vector =
factory->NewFeedbackVector(shared, closure_feedback_cell_array);
Handle<FeedbackVector> vector = factory->NewFeedbackVector(
shared, closure_feedback_cell_array, parent_feedback_cell);
DCHECK_EQ(vector->length(), slot_count);
@ -292,16 +293,15 @@ Handle<FeedbackVector> FeedbackVector::New(
i += entry_size;
}
Handle<FeedbackVector> result = Handle<FeedbackVector>::cast(vector);
if (!isolate->is_best_effort_code_coverage()) {
AddToVectorsForProfilingTools(isolate, result);
AddToVectorsForProfilingTools(isolate, vector);
}
return result;
parent_feedback_cell->set_value(*vector, kReleaseStore);
return vector;
}
namespace {
Handle<FeedbackVector> NewFeedbackVectorForTesting(
// static
Handle<FeedbackVector> FeedbackVector::NewForTesting(
Isolate* isolate, const FeedbackVectorSpec* spec) {
Handle<FeedbackMetadata> metadata = FeedbackMetadata::New(isolate, spec);
Handle<SharedFunctionInfo> shared =
@ -312,20 +312,20 @@ Handle<FeedbackVector> NewFeedbackVectorForTesting(
shared->set_raw_outer_scope_info_or_feedback_metadata(*metadata);
Handle<ClosureFeedbackCellArray> closure_feedback_cell_array =
ClosureFeedbackCellArray::New(isolate, shared);
Handle<FeedbackCell> parent_cell = isolate->factory()->NewNoClosuresCell(
isolate->factory()->undefined_value());
IsCompiledScope is_compiled_scope(shared->is_compiled_scope(isolate));
return FeedbackVector::New(isolate, shared, closure_feedback_cell_array,
&is_compiled_scope);
parent_cell, &is_compiled_scope);
}
} // namespace
// static
Handle<FeedbackVector> FeedbackVector::NewWithOneBinarySlotForTesting(
Zone* zone, Isolate* isolate) {
FeedbackVectorSpec one_slot(zone);
one_slot.AddBinaryOpICSlot();
return NewFeedbackVectorForTesting(isolate, &one_slot);
return NewForTesting(isolate, &one_slot);
}
// static
@ -333,7 +333,7 @@ Handle<FeedbackVector> FeedbackVector::NewWithOneCompareSlotForTesting(
Zone* zone, Isolate* isolate) {
FeedbackVectorSpec one_slot(zone);
one_slot.AddCompareICSlot();
return NewFeedbackVectorForTesting(isolate, &one_slot);
return NewForTesting(isolate, &one_slot);
}
// static

View File

@ -25,6 +25,7 @@ namespace v8 {
namespace internal {
class IsCompiledScope;
class FeedbackVectorSpec;
enum class UpdateFeedbackMode { kOptionalFeedback, kGuaranteedFeedback };
@ -319,8 +320,11 @@ class FeedbackVector
V8_EXPORT_PRIVATE static Handle<FeedbackVector> New(
Isolate* isolate, Handle<SharedFunctionInfo> shared,
Handle<ClosureFeedbackCellArray> closure_feedback_cell_array,
Handle<FeedbackCell> parent_feedback_cell,
IsCompiledScope* is_compiled_scope);
V8_EXPORT_PRIVATE static Handle<FeedbackVector> NewForTesting(
Isolate* isolate, const FeedbackVectorSpec* spec);
V8_EXPORT_PRIVATE static Handle<FeedbackVector>
NewWithOneBinarySlotForTesting(Zone* zone, Isolate* isolate);
V8_EXPORT_PRIVATE static Handle<FeedbackVector>

View File

@ -44,6 +44,7 @@ extern class FeedbackVector extends HeapObject {
flags: FeedbackVectorFlags;
shared_function_info: SharedFunctionInfo;
closure_feedback_cell_array: ClosureFeedbackCellArray;
parent_feedback_cell: FeedbackCell;
@if(V8_EXTERNAL_CODE_SPACE) maybe_optimized_code: Weak<CodeDataContainer>;
@ifnot(V8_EXTERNAL_CODE_SPACE) maybe_optimized_code: Weak<Code>;
@cppRelaxedLoad raw_feedback_slots[length]: MaybeObject;

View File

@ -585,13 +585,15 @@ void JSFunction::CreateAndAttachFeedbackVector(
Handle<ClosureFeedbackCellArray> closure_feedback_cell_array =
handle(function->closure_feedback_cell_array(), isolate);
Handle<FeedbackVector> feedback_vector = FeedbackVector::New(
isolate, shared, closure_feedback_cell_array, compiled_scope);
isolate, shared, closure_feedback_cell_array,
handle(function->raw_feedback_cell(isolate), isolate), compiled_scope);
USE(feedback_vector);
// EnsureClosureFeedbackCellArray should handle the special case where we need
// to allocate a new feedback cell. Please look at comment in that function
// for more details.
DCHECK(function->raw_feedback_cell() !=
isolate->heap()->many_closures_cell());
function->raw_feedback_cell().set_value(*feedback_vector, kReleaseStore);
DCHECK_EQ(function->raw_feedback_cell().value(), *feedback_vector);
function->SetInterruptBudget(isolate);
DCHECK_EQ(v8_flags.log_function_events,

View File

@ -2637,11 +2637,11 @@ TEST(AllocationSitesAreVisible) {
const v8::HeapGraphNode* vector = GetProperty(
env->GetIsolate(), feedback_cell, v8::HeapGraphEdge::kInternal, "value");
CHECK_EQ(v8::HeapGraphNode::kCode, vector->GetType());
CHECK_EQ(4, vector->GetChildrenCount());
CHECK_EQ(5, vector->GetChildrenCount());
// The last value in the feedback vector should be the boilerplate,
// found in AllocationSite.transition_info.
const v8::HeapGraphEdge* prop = vector->GetChild(3);
const v8::HeapGraphEdge* prop = vector->GetChild(4);
const v8::HeapGraphNode* allocation_site = prop->GetToNode();
v8::String::Utf8Value name(env->GetIsolate(), allocation_site->GetName());
CHECK_EQ(0, strcmp("system / AllocationSite", *name));
@ -4094,8 +4094,9 @@ TEST(WeakReference) {
i_isolate);
i::Handle<i::ClosureFeedbackCellArray> feedback_cell_array =
i::ClosureFeedbackCellArray::New(i_isolate, shared_function);
i::Handle<i::FeedbackVector> fv =
factory->NewFeedbackVector(shared_function, feedback_cell_array);
i::Handle<i::FeedbackVector> fv = factory->NewFeedbackVector(
shared_function, feedback_cell_array,
handle(i::JSFunction::cast(*obj).raw_feedback_cell(), i_isolate));
// Create a Code.
i::Assembler assm(i::AssemblerOptions{});

View File

@ -101,18 +101,8 @@ class JSCallReducerTest : public TypedGraphTest {
const Operator* Call(int arity) {
FeedbackVectorSpec spec(zone());
spec.AddCallICSlot();
Handle<FeedbackMetadata> metadata = FeedbackMetadata::New(isolate(), &spec);
Handle<SharedFunctionInfo> shared =
isolate()->factory()->NewSharedFunctionInfoForBuiltin(
isolate()->factory()->empty_string(), Builtin::kIllegal);
// Set the raw feedback metadata to circumvent checks that we are not
// overwriting existing metadata.
shared->set_raw_outer_scope_info_or_feedback_metadata(*metadata);
Handle<ClosureFeedbackCellArray> closure_feedback_cell_array =
ClosureFeedbackCellArray::New(isolate(), shared);
IsCompiledScope is_compiled_scope(shared->is_compiled_scope(isolate()));
Handle<FeedbackVector> vector = FeedbackVector::New(
isolate(), shared, closure_feedback_cell_array, &is_compiled_scope);
Handle<FeedbackVector> vector =
FeedbackVector::NewForTesting(isolate(), &spec);
FeedbackSource feedback(vector, FeedbackSlot(0));
return javascript()->Call(JSCallNode::ArityForArgc(arity), CallFrequency(),
feedback, ConvertReceiverMode::kAny,

View File

@ -31,16 +31,8 @@ class RedundancyEliminationTest : public GraphTest {
FeedbackVectorSpec spec(zone());
FeedbackSlot slot1 = spec.AddCallICSlot();
FeedbackSlot slot2 = spec.AddCallICSlot();
Handle<FeedbackMetadata> metadata = FeedbackMetadata::New(isolate(), &spec);
Handle<SharedFunctionInfo> shared =
isolate()->factory()->NewSharedFunctionInfoForBuiltin(
isolate()->factory()->empty_string(), Builtin::kIllegal);
shared->set_raw_outer_scope_info_or_feedback_metadata(*metadata);
Handle<ClosureFeedbackCellArray> closure_feedback_cell_array =
ClosureFeedbackCellArray::New(isolate(), shared);
IsCompiledScope is_compiled_scope(shared->is_compiled_scope(isolate()));
Handle<FeedbackVector> feedback_vector = FeedbackVector::New(
isolate(), shared, closure_feedback_cell_array, &is_compiled_scope);
Handle<FeedbackVector> feedback_vector =
FeedbackVector::NewForTesting(isolate(), &spec);
vector_slot_pairs_.push_back(FeedbackSource());
vector_slot_pairs_.push_back(FeedbackSource(feedback_vector, slot1));
vector_slot_pairs_.push_back(FeedbackSource(feedback_vector, slot2));

View File

@ -574,18 +574,7 @@ class FeedbackVectorHelper {
template <typename Spec>
Handle<FeedbackVector> NewFeedbackVector(Isolate* isolate, Spec* spec) {
Handle<FeedbackMetadata> metadata = FeedbackMetadata::New(isolate, spec);
Handle<SharedFunctionInfo> shared =
isolate->factory()->NewSharedFunctionInfoForBuiltin(
isolate->factory()->empty_string(), Builtin::kIllegal);
// Set the raw feedback metadata to circumvent checks that we are not
// overwriting existing metadata.
shared->set_raw_outer_scope_info_or_feedback_metadata(*metadata);
Handle<ClosureFeedbackCellArray> closure_feedback_cell_array =
ClosureFeedbackCellArray::New(isolate, shared);
IsCompiledScope is_compiled_scope(shared->is_compiled_scope(isolate));
return FeedbackVector::New(isolate, shared, closure_feedback_cell_array,
&is_compiled_scope);
return FeedbackVector::NewForTesting(isolate, spec);
}
class ParkingThread : public v8::base::Thread {