[interpreter] Move interrupt budget from BytecodeArray to FeedbackCell

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: Ie3ac389e1c082d1671efd4d74abc076ce943301b
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1558088
Commit-Queue: Mythri Alle <mythria@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#60734}

src/feedback-vector-inl.h

@@ -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,

src/feedback-vector.cc

@@ -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;
 }

src/feedback-vector.h

@@ -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);
 };

src/heap/factory.cc

@@ -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,7 @@ 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());
   return cell;
 }
 
@@ -1900,6 +1896,7 @@ 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());
   return cell;
 }
 
@@ -1909,6 +1906,7 @@ 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());
   return cell;
 }
 
@@ -2949,7 +2947,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);

src/heap/setup-heap-internal.cc

@@ -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;                                              \

src/interpreter/interpreter-assembler.cc

@@ -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");
 }
 

src/interpreter/interpreter-generator.cc

@@ -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,

src/objects/code-inl.h

@@ -670,15 +670,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);
 }

src/objects/code.h

@@ -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. */                                        \

src/objects/feedback-cell-inl.h

@@ -22,6 +22,7 @@ OBJECT_CONSTRUCTORS_IMPL(FeedbackCell, Struct)
 CAST_ACCESSOR(FeedbackCell)
 
 ACCESSORS(FeedbackCell, value, HeapObject, kValueOffset)
+INT32_ACCESSORS(FeedbackCell, interrupt_budget, kInterruptBudgetOffset)
 
 }  // namespace internal
 }  // namespace v8

src/objects/feedback-cell.h

@@ -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)
 
@@ -32,13 +40,18 @@ class FeedbackCell : public Struct {
 // Layout description.
 #define FEEDBACK_CELL_FIELDS(V)         \
   V(kValueOffset, kTaggedSize)          \
+  /* Non-pointer fields */              \
+  V(kInterruptBudgetOffset, kInt32Size) \
   /* Total size. */                     \
-  V(kSize, 0)
+  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});
+
+  using BodyDescriptor =
+      FixedBodyDescriptor<kValueOffset, kInterruptBudgetOffset, kSize>;
 
   OBJECT_CONSTRUCTORS(FeedbackCell, Struct);
 };

src/roots.h

@@ -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)      \

src/snapshot/deserializer.cc

@@ -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

src/snapshot/partial-serializer.cc

@@ -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;
   }

test/cctest/test-serialize.cc

@@ -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);
 
     {