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

This reverts commit f3994485.

Reason for revert:
https://ci.chromium.org/p/v8/builders/ci/V8%20Linux%20-%20arm64%20-%20sim%20-%20MSAN/26128

Original change's description:
> [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}

TBR=jarin@chromium.org,mlippautz@chromium.org,mythria@chromium.org,jgruber@chromium.org,bmeurer@chromium.org

Change-Id: Icbec4d28d6ac258827e222461cff51f2a2f42472
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: chromium:948835, v8:8394
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1560990Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Commit-Queue: Michael Achenbach <machenbach@chromium.org>
Cr-Commit-Position: refs/heads/master@{#60735}

src/feedback-vector-inl.h

@@ -96,8 +96,11 @@ 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)), GetIsolate());
+  return handle(FeedbackCell::cast(get(index + kFeedbackCellStartIndex)),
+                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, *cell);
+    feedback_cell_array->set(i + kFeedbackCellStartIndex, *cell);
   }
   return feedback_cell_array;
 }

src/feedback-vector.h

@@ -161,9 +161,13 @@ 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,13 +419,17 @@ Handle<FixedArray> Factory::NewUninitializedFixedArray(
 }
 
 Handle<ClosureFeedbackCellArray> Factory::NewClosureFeedbackCellArray(
-    int length, AllocationType allocation) {
-  if (length == 0) return empty_closure_feedback_cell_array();
-
+    int num_slots, AllocationType allocation) {
+  int length = ClosureFeedbackCellArray::kFeedbackCellStartIndex + num_slots;
   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;
 }
 
@@ -1819,6 +1823,7 @@ 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);
@@ -1886,7 +1891,6 @@ 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;
 }
 
@@ -1896,7 +1900,6 @@ 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;
 }
 
@@ -1906,7 +1909,6 @@ 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;
 }
 
@@ -2947,6 +2949,7 @@ 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,17 +602,6 @@ 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,14 +1259,30 @@ 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<FeedbackCell> feedback_cell =
-      CAST(LoadObjectField(function, JSFunction::kFeedbackCellOffset));
-  TNode<Int32T> old_budget = LoadObjectField<Int32T>(
-      feedback_cell, FeedbackCell::kInterruptBudgetOffset);
-
+  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);
   // Make sure we include the current bytecode in the budget calculation.
-  TNode<Int32T> budget_after_bytecode =
-      Signed(Int32Sub(old_budget, Int32Constant(CurrentBytecodeSize())));
+  TNode<Int32T> budget_after_bytecode = Signed(
+      Int32Sub(old_budget.value(), Int32Constant(CurrentBytecodeSize())));
 
   TVARIABLE(Int32T, new_budget);
   if (backward) {
@@ -1274,7 +1290,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::kDeferred);
+    Label ok(this), interrupt_check(this);
     Branch(condition, &ok, &interrupt_check);
 
     BIND(&interrupt_check);
@@ -1290,9 +1306,21 @@ void InterpreterAssembler::UpdateInterruptBudget(Node* weight, bool backward) {
   }
 
   // Update budget.
-  StoreObjectFieldNoWriteBarrier(
-      feedback_cell, FeedbackCell::kInterruptBudgetOffset, new_budget.value(),
-      MachineRepresentation::kWord32);
+  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);
   Comment("] UpdateInterruptBudget");
 }
 

src/interpreter/interpreter-generator.cc

@@ -2632,8 +2632,9 @@ 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));
+  TNode<FeedbackCell> feedback_cell = CAST(LoadFixedArrayElement(
+      feedback_cell_array, slot,
+      ClosureFeedbackCellArray::kFeedbackCellStartIndex * kTaggedSize));
 
   Label if_fast(this), if_slow(this, Label::kDeferred);
   Branch(IsSetWord32<CreateClosureFlags::FastNewClosureBit>(flags), &if_fast,

src/objects/code-inl.h

@@ -670,6 +670,15 @@ 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,6 +758,10 @@ 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);
@@ -837,6 +841,7 @@ 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,7 +22,6 @@ 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,16 +20,8 @@ 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)
 
@@ -38,20 +30,15 @@ class FeedbackCell : public Struct {
   DECL_VERIFIER(FeedbackCell)
 
 // Layout description.
-#define FEEDBACK_CELL_FIELDS(V)         \
-  V(kValueOffset, kTaggedSize)          \
-  /* Non-pointer fields */              \
-  V(kInterruptBudgetOffset, kInt32Size) \
-  /* Total size. */                     \
-  V(kUnalignedSize, 0)
+#define FEEDBACK_CELL_FIELDS(V) \
+  V(kValueOffset, kTaggedSize)  \
+  /* Total size. */             \
+  V(kSize, 0)
 
   DEFINE_FIELD_OFFSET_CONSTANTS(HeapObject::kHeaderSize, FEEDBACK_CELL_FIELDS)
 #undef FEEDBACK_CELL_FIELDS
 
-  static const int kSize = RoundUp<kObjectAlignment>(int{kUnalignedSize});
-
-  using BodyDescriptor =
-      FixedBodyDescriptor<kValueOffset, kInterruptBudgetOffset, kSize>;
+  using BodyDescriptor = FixedBodyDescriptor<kValueOffset, kSize, kSize>;
 
   OBJECT_CONSTRUCTORS(FeedbackCell, Struct);
 };

src/roots.h

@@ -182,8 +182,6 @@ 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,6 +306,8 @@ 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,12 +101,6 @@ 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,6 +2442,8 @@ 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);
 
     {