Commit f78f43b1 authored by Mythri A's avatar Mythri A Committed by Commit Bot

Reland "[interpreter] Move interrupt budget from BytecodeArray to FeedbackCell"

This is a reland of Ie3ac389e1c082d1671efd4d74abc076ce943301b with a fix
for MSAN failures.

Interrupt budget was store in bytecode array and used to be shared
across all contexts. With lazy feedback allocation, using context
independent interrupt budget might lead to performance cliffs when
we have closures that do not share the same feedback (for ex: across
contexts). This would be a problem even earlier but it could be
more pronounced with feedback vector allocation, since the budgets
for optimization is much higher (144x) than the budget for feedback
allocation.

Bug: chromium:948835, v8:8394
Change-Id: I74f998c30e27caf3bd34510f4d7f57b65e6c7f0d
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1561072Reviewed-by: 's avatarUlan Degenbaev <ulan@chromium.org>
Reviewed-by: 's avatarJakob Gruber <jgruber@chromium.org>
Reviewed-by: 's avatarJaroslav Sevcik <jarin@chromium.org>
Commit-Queue: Mythri Alle <mythria@chromium.org>
Cr-Commit-Position: refs/heads/master@{#60750}
parent 1f482f75
......@@ -96,11 +96,8 @@ int FeedbackMetadata::GetSlotSize(FeedbackSlotKind kind) {
return 1;
}
SMI_ACCESSORS(ClosureFeedbackCellArray, interrupt_budget,
FixedArray::OffsetOfElementAt(kInterruptBudgetIndex))
Handle<FeedbackCell> ClosureFeedbackCellArray::GetFeedbackCell(int index) {
return handle(FeedbackCell::cast(get(index + kFeedbackCellStartIndex)),
GetIsolate());
return handle(FeedbackCell::cast(get(index)), GetIsolate());
}
ACCESSORS(FeedbackVector, shared_function_info, SharedFunctionInfo,
......
......@@ -220,7 +220,7 @@ Handle<ClosureFeedbackCellArray> ClosureFeedbackCellArray::New(
for (int i = 0; i < num_feedback_cells; i++) {
Handle<FeedbackCell> cell =
factory->NewNoClosuresCell(factory->undefined_value());
feedback_cell_array->set(i + kFeedbackCellStartIndex, *cell);
feedback_cell_array->set(i, *cell);
}
return feedback_cell_array;
}
......
......@@ -161,13 +161,9 @@ class ClosureFeedbackCellArray : public FixedArray {
Isolate* isolate, Handle<SharedFunctionInfo> shared);
inline Handle<FeedbackCell> GetFeedbackCell(int index);
DECL_INT_ACCESSORS(interrupt_budget)
DECL_VERIFIER(ClosureFeedbackCellArray)
DECL_PRINTER(ClosureFeedbackCellArray)
enum { kInterruptBudgetIndex, kFeedbackCellStartIndex };
private:
OBJECT_CONSTRUCTORS(ClosureFeedbackCellArray, FixedArray);
};
......
......@@ -419,17 +419,13 @@ Handle<FixedArray> Factory::NewUninitializedFixedArray(
}
Handle<ClosureFeedbackCellArray> Factory::NewClosureFeedbackCellArray(
int num_slots, AllocationType allocation) {
int length = ClosureFeedbackCellArray::kFeedbackCellStartIndex + num_slots;
int length, AllocationType allocation) {
if (length == 0) return empty_closure_feedback_cell_array();
Handle<ClosureFeedbackCellArray> feedback_cell_array =
NewFixedArrayWithMap<ClosureFeedbackCellArray>(
RootIndex::kClosureFeedbackCellArrayMap, length, allocation);
// Initialize header fields
feedback_cell_array->set_interrupt_budget(
FLAG_budget_for_feedback_vector_allocation);
DCHECK_EQ(ClosureFeedbackCellArray::kFeedbackCellStartIndex, 1);
return feedback_cell_array;
}
......@@ -1823,7 +1819,6 @@ Handle<BytecodeArray> Factory::NewBytecodeArray(
instance->set_parameter_count(parameter_count);
instance->set_incoming_new_target_or_generator_register(
interpreter::Register::invalid_value());
instance->set_interrupt_budget(interpreter::Interpreter::InterruptBudget());
instance->set_osr_loop_nesting_level(0);
instance->set_bytecode_age(BytecodeArray::kNoAgeBytecodeAge);
instance->set_constant_pool(*constant_pool);
......@@ -1891,6 +1886,8 @@ Handle<FeedbackCell> Factory::NewNoClosuresCell(Handle<HeapObject> value) {
FeedbackCell::kSize, AllocationType::kOld, *no_closures_cell_map());
Handle<FeedbackCell> cell(FeedbackCell::cast(result), isolate());
cell->set_value(*value);
cell->set_interrupt_budget(FeedbackCell::GetInitialInterruptBudget());
cell->clear_padding();
return cell;
}
......@@ -1900,6 +1897,8 @@ Handle<FeedbackCell> Factory::NewOneClosureCell(Handle<HeapObject> value) {
FeedbackCell::kSize, AllocationType::kOld, *one_closure_cell_map());
Handle<FeedbackCell> cell(FeedbackCell::cast(result), isolate());
cell->set_value(*value);
cell->set_interrupt_budget(FeedbackCell::GetInitialInterruptBudget());
cell->clear_padding();
return cell;
}
......@@ -1909,6 +1908,8 @@ Handle<FeedbackCell> Factory::NewManyClosuresCell(Handle<HeapObject> value) {
FeedbackCell::kSize, AllocationType::kOld, *many_closures_cell_map());
Handle<FeedbackCell> cell(FeedbackCell::cast(result), isolate());
cell->set_value(*value);
cell->set_interrupt_budget(FeedbackCell::GetInitialInterruptBudget());
cell->clear_padding();
return cell;
}
......@@ -2949,7 +2950,6 @@ Handle<BytecodeArray> Factory::CopyBytecodeArray(
copy->set_constant_pool(bytecode_array->constant_pool());
copy->set_handler_table(bytecode_array->handler_table());
copy->set_source_position_table(bytecode_array->source_position_table());
copy->set_interrupt_budget(bytecode_array->interrupt_budget());
copy->set_osr_loop_nesting_level(bytecode_array->osr_loop_nesting_level());
copy->set_bytecode_age(bytecode_array->bytecode_age());
bytecode_array->CopyBytecodesTo(*copy);
......
......@@ -602,6 +602,17 @@ bool Heap::CreateInitialMaps() {
set_empty_property_array(PropertyArray::cast(obj));
}
{
if (!AllocateRaw(FixedArray::SizeFor(0), AllocationType::kReadOnly)
.To(&obj)) {
return false;
}
obj->set_map_after_allocation(roots.closure_feedback_cell_array_map(),
SKIP_WRITE_BARRIER);
FixedArray::cast(obj)->set_length(0);
set_empty_closure_feedback_cell_array(ClosureFeedbackCellArray::cast(obj));
}
#define ALLOCATE_EMPTY_FIXED_TYPED_ARRAY(Type, type, TYPE, ctype) \
{ \
FixedTypedArrayBase obj; \
......
......@@ -1259,30 +1259,14 @@ void InterpreterAssembler::UpdateInterruptBudget(Node* weight, bool backward) {
Label load_budget_from_bytecode(this), load_budget_done(this);
TNode<JSFunction> function = CAST(LoadRegister(Register::function_closure()));
TNode<HeapObject> feedback_cell_value = LoadFeedbackCellValue(function);
Node* budget_offset =
IntPtrConstant(BytecodeArray::kInterruptBudgetOffset - kHeapObjectTag);
TVARIABLE(Int32T, old_budget);
// TODO(mythria): We should use the interrupt budget on the feedback vector
// for updating runtime profiler ticks as well. That would avoid having two
// different places where we track interrupt budget.
GotoIf(IsFeedbackVector(feedback_cell_value), &load_budget_from_bytecode);
TNode<FixedArray> closure_feedback_cell_array = CAST(feedback_cell_value);
TNode<Smi> old_budget_smi = CAST(UnsafeLoadFixedArrayElement(
closure_feedback_cell_array,
ClosureFeedbackCellArray::kInterruptBudgetIndex));
old_budget = SmiToInt32(old_budget_smi);
Goto(&load_budget_done);
BIND(&load_budget_from_bytecode);
old_budget = UncheckedCast<Int32T>(
Load(MachineType::Int32(), BytecodeArrayTaggedPointer(), budget_offset));
Goto(&load_budget_done);
BIND(&load_budget_done);
TNode<FeedbackCell> feedback_cell =
CAST(LoadObjectField(function, JSFunction::kFeedbackCellOffset));
TNode<Int32T> old_budget = LoadObjectField<Int32T>(
feedback_cell, FeedbackCell::kInterruptBudgetOffset);
// Make sure we include the current bytecode in the budget calculation.
TNode<Int32T> budget_after_bytecode = Signed(
Int32Sub(old_budget.value(), Int32Constant(CurrentBytecodeSize())));
TNode<Int32T> budget_after_bytecode =
Signed(Int32Sub(old_budget, Int32Constant(CurrentBytecodeSize())));
TVARIABLE(Int32T, new_budget);
if (backward) {
......@@ -1290,7 +1274,7 @@ void InterpreterAssembler::UpdateInterruptBudget(Node* weight, bool backward) {
new_budget = Signed(Int32Sub(budget_after_bytecode, weight));
Node* condition =
Int32GreaterThanOrEqual(new_budget.value(), Int32Constant(0));
Label ok(this), interrupt_check(this);
Label ok(this), interrupt_check(this, Label::kDeferred);
Branch(condition, &ok, &interrupt_check);
BIND(&interrupt_check);
......@@ -1306,21 +1290,9 @@ void InterpreterAssembler::UpdateInterruptBudget(Node* weight, bool backward) {
}
// Update budget.
Label update_budget_in_bytecode(this), end(this);
GotoIf(IsFeedbackVector(feedback_cell_value), &update_budget_in_bytecode);
UnsafeStoreFixedArrayElement(closure_feedback_cell_array,
ClosureFeedbackCellArray::kInterruptBudgetIndex,
SmiFromInt32(new_budget.value()),
SKIP_WRITE_BARRIER);
Goto(&end);
BIND(&update_budget_in_bytecode);
StoreNoWriteBarrier(MachineRepresentation::kWord32,
BytecodeArrayTaggedPointer(), budget_offset,
new_budget.value());
Goto(&end);
BIND(&end);
StoreObjectFieldNoWriteBarrier(
feedback_cell, FeedbackCell::kInterruptBudgetOffset, new_budget.value(),
MachineRepresentation::kWord32);
Comment("] UpdateInterruptBudget");
}
......
......@@ -2632,9 +2632,8 @@ IGNITION_HANDLER(CreateClosure, InterpreterAssembler) {
Label if_undefined(this);
TNode<FixedArray> feedback_cell_array =
LoadClosureFeedbackArray(LoadRegister(Register::function_closure()));
TNode<FeedbackCell> feedback_cell = CAST(LoadFixedArrayElement(
feedback_cell_array, slot,
ClosureFeedbackCellArray::kFeedbackCellStartIndex * kTaggedSize));
TNode<FeedbackCell> feedback_cell =
CAST(LoadFixedArrayElement(feedback_cell_array, slot));
Label if_fast(this), if_slow(this, Label::kDeferred);
Branch(IsSetWord32<CreateClosureFlags::FastNewClosureBit>(flags), &if_fast,
......
......@@ -669,15 +669,6 @@ void BytecodeArray::set_incoming_new_target_or_generator_register(
}
}
int BytecodeArray::interrupt_budget() const {
return READ_INT_FIELD(*this, kInterruptBudgetOffset);
}
void BytecodeArray::set_interrupt_budget(int interrupt_budget) {
DCHECK_GE(interrupt_budget, 0);
WRITE_INT_FIELD(*this, kInterruptBudgetOffset, interrupt_budget);
}
int BytecodeArray::osr_loop_nesting_level() const {
return READ_INT8_FIELD(*this, kOSRNestingLevelOffset);
}
......
......@@ -758,10 +758,6 @@ class BytecodeArray : public FixedArrayBase {
inline void set_incoming_new_target_or_generator_register(
interpreter::Register incoming_new_target_or_generator_register);
// Accessors for profiling count.
inline int interrupt_budget() const;
inline void set_interrupt_budget(int interrupt_budget);
// Accessors for OSR loop nesting level.
inline int osr_loop_nesting_level() const;
inline void set_osr_loop_nesting_level(int depth);
......@@ -841,7 +837,6 @@ class BytecodeArray : public FixedArrayBase {
V(kFrameSizeOffset, kIntSize) \
V(kParameterSizeOffset, kIntSize) \
V(kIncomingNewTargetOrGeneratorRegisterOffset, kIntSize) \
V(kInterruptBudgetOffset, kIntSize) \
V(kOSRNestingLevelOffset, kCharSize) \
V(kBytecodeAgeOffset, kCharSize) \
/* Total size. */ \
......
......@@ -22,6 +22,14 @@ OBJECT_CONSTRUCTORS_IMPL(FeedbackCell, Struct)
CAST_ACCESSOR(FeedbackCell)
ACCESSORS(FeedbackCell, value, HeapObject, kValueOffset)
INT32_ACCESSORS(FeedbackCell, interrupt_budget, kInterruptBudgetOffset)
void FeedbackCell::clear_padding() {
if (FeedbackCell::kSize == FeedbackCell::kUnalignedSize) return;
DCHECK_GE(FeedbackCell::kSize, FeedbackCell::kUnalignedSize);
memset(reinterpret_cast<byte*>(address() + FeedbackCell::kUnalignedSize), 0,
FeedbackCell::kSize - FeedbackCell::kUnalignedSize);
}
} // namespace internal
} // namespace v8
......
......@@ -20,8 +20,16 @@ namespace internal {
// a native context.
class FeedbackCell : public Struct {
public:
static int GetInitialInterruptBudget() {
if (FLAG_lazy_feedback_allocation) {
return FLAG_budget_for_feedback_vector_allocation;
}
return FLAG_interrupt_budget;
}
// [value]: value of the cell.
DECL_ACCESSORS(value, HeapObject)
DECL_INT32_ACCESSORS(interrupt_budget)
DECL_CAST(FeedbackCell)
......@@ -30,15 +38,22 @@ class FeedbackCell : public Struct {
DECL_VERIFIER(FeedbackCell)
// Layout description.
#define FEEDBACK_CELL_FIELDS(V) \
V(kValueOffset, kTaggedSize) \
/* Total size. */ \
V(kSize, 0)
#define FEEDBACK_CELL_FIELDS(V) \
V(kValueOffset, kTaggedSize) \
/* Non-pointer fields */ \
V(kInterruptBudgetOffset, kInt32Size) \
/* Total size. */ \
V(kUnalignedSize, 0)
DEFINE_FIELD_OFFSET_CONSTANTS(HeapObject::kHeaderSize, FEEDBACK_CELL_FIELDS)
#undef FEEDBACK_CELL_FIELDS
using BodyDescriptor = FixedBodyDescriptor<kValueOffset, kSize, kSize>;
static const int kSize = RoundUp<kObjectAlignment>(int{kUnalignedSize});
inline void clear_padding();
using BodyDescriptor =
FixedBodyDescriptor<kValueOffset, kInterruptBudgetOffset, kSize>;
OBJECT_CONSTRUCTORS(FeedbackCell, Struct);
};
......
......@@ -182,6 +182,8 @@ class RootVisitor;
EmptyObjectBoilerplateDescription) \
V(ArrayBoilerplateDescription, empty_array_boilerplate_description, \
EmptyArrayBoilerplateDescription) \
V(ClosureFeedbackCellArray, empty_closure_feedback_cell_array, \
EmptyClosureFeedbackCellArray) \
V(FixedTypedArrayBase, empty_fixed_uint8_array, EmptyFixedUint8Array) \
V(FixedTypedArrayBase, empty_fixed_int8_array, EmptyFixedInt8Array) \
V(FixedTypedArrayBase, empty_fixed_uint16_array, EmptyFixedUint16Array) \
......
......@@ -306,8 +306,6 @@ HeapObject Deserializer::PostProcessNewObject(HeapObject obj, int space) {
// TODO(mythria): Remove these once we store the default values for these
// fields in the serializer.
BytecodeArray bytecode_array = BytecodeArray::cast(obj);
bytecode_array->set_interrupt_budget(
interpreter::Interpreter::InterruptBudget());
bytecode_array->set_osr_loop_nesting_level(0);
}
#ifdef DEBUG
......
......@@ -101,6 +101,12 @@ void PartialSerializer::SerializeObject(HeapObject obj) {
// Clear literal boilerplates and feedback.
if (obj->IsFeedbackVector()) FeedbackVector::cast(obj)->ClearSlots(isolate());
// Clear InterruptBudget when serializing FeedbackCell.
if (obj->IsFeedbackCell()) {
FeedbackCell::cast(obj)->set_interrupt_budget(
FeedbackCell::GetInitialInterruptBudget());
}
if (SerializeJSObjectWithEmbedderFields(obj)) {
return;
}
......
......@@ -2442,8 +2442,6 @@ TEST(CodeSerializerAfterExecute) {
Handle<SharedFunctionInfo> sfi = v8::Utils::OpenHandle(*script);
CHECK(sfi->HasBytecodeArray());
BytecodeArray bytecode = sfi->GetBytecodeArray();
CHECK_EQ(bytecode->interrupt_budget(),
interpreter::Interpreter::InterruptBudget());
CHECK_EQ(bytecode->osr_loop_nesting_level(), 0);
{
......
This diff is collapsed.
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment