Add option to allocate feedback vector based on bytecode size

Currently, feedback vectors are allocated on a fixed budget of 1024.
In some cases it might be beneficial to allocate feedback vectors based
on invocation count rather than fixed budget. For example, if we have
a large function that is only run once. This cl adds an option to
use interrupt budget based on the bytecode size. It kind of mimics
invocation count. We would allocate feedback vectors early when we
have loops which is also required.

This flag is turned off by default. In followup cl, we will enable it
and if the memory / performance tradeoff is good we might make it
default.

Change-Id: I9f7231119b5fd65fb3268e665e2e315fb2625e1b
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2584960Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Commit-Queue: Mythri Alle <mythria@chromium.org>
Cr-Commit-Position: refs/heads/master@{#72371}

src/codegen/compiler.cc

@@ -1845,8 +1845,13 @@ bool Compiler::Compile(Handle<JSFunction> function, ClearExceptionFlag flag,
   DCHECK(is_compiled_scope->is_compiled());
   Handle<Code> code = handle(shared_info->GetCode(), isolate);
 
-  // Initialize the feedback cell for this JSFunction.
-  JSFunction::InitializeFeedbackCell(function, is_compiled_scope);
+  // Initialize the feedback cell for this JSFunction and reset the interrupt
+  // budget for feedback vector allocation even if there is a closure feedback
+  // cell array. We are re-compiling when we have a closure feedback cell array
+  // which means we are compiling after a bytecode flush.
+  // TODO(verwaest/mythria): Investigate if allocating feedback vector
+  // immediately after a flush would be better.
+  JSFunction::InitializeFeedbackCell(function, is_compiled_scope, true);
 
   // Optimize now if --always-opt is enabled.
   if (FLAG_always_opt && !function->shared().HasAsmWasmData()) {
@@ -2060,7 +2065,9 @@ MaybeHandle<JSFunction> Compiler::GetFunctionFromEval(
       result = Factory::JSFunctionBuilder{isolate, shared_info, context}
                    .set_allocation_type(AllocationType::kYoung)
                    .Build();
-      JSFunction::InitializeFeedbackCell(result, &is_compiled_scope);
+      // TODO(mythria): I don't think we need this here. PostInstantiation
+      // already initializes feedback cell.
+      JSFunction::InitializeFeedbackCell(result, &is_compiled_scope, true);
       if (allow_eval_cache) {
         // Make sure to cache this result.
         Handle<FeedbackCell> new_feedback_cell(result->raw_feedback_cell(),
@@ -2073,7 +2080,9 @@ MaybeHandle<JSFunction> Compiler::GetFunctionFromEval(
     result = Factory::JSFunctionBuilder{isolate, shared_info, context}
                  .set_allocation_type(AllocationType::kYoung)
                  .Build();
-    JSFunction::InitializeFeedbackCell(result, &is_compiled_scope);
+    // TODO(mythria): I don't think we need this here. PostInstantiation
+    // already initializes feedback cell.
+    JSFunction::InitializeFeedbackCell(result, &is_compiled_scope, true);
     if (allow_eval_cache) {
       // Add the SharedFunctionInfo and the LiteralsArray to the eval cache if
       // we didn't retrieve from there.
@@ -3107,7 +3116,9 @@ void Compiler::PostInstantiation(Handle<JSFunction> function) {
   // If code is compiled to bytecode (i.e., isn't asm.js), then allocate a
   // feedback and check for optimized code.
   if (is_compiled_scope.is_compiled() && shared->HasBytecodeArray()) {
-    JSFunction::InitializeFeedbackCell(function, &is_compiled_scope);
+    // Don't reset budget if there is a closure feedback cell array already. We
+    // are just creating a new closure that shares the same feedback cell.
+    JSFunction::InitializeFeedbackCell(function, &is_compiled_scope, false);
 
     if (function->has_feedback_vector()) {
       // Evict any deoptimized code on feedback vector. We need to do this after

src/flags/flag-definitions.h

@@ -505,6 +505,11 @@ DEFINE_BOOL(use_ic, true, "use inline caching")
 DEFINE_INT(budget_for_feedback_vector_allocation, 1 * KB,
            "The budget in amount of bytecode executed by a function before we "
            "decide to allocate feedback vectors")
+DEFINE_INT(scale_factor_for_feedback_allocation, 4,
+           "scale bytecode size for feedback vector allocation.")
+DEFINE_BOOL(feedback_allocation_on_bytecode_size, false,
+            "Instead of a fixed budget for lazy feedback vector allocation, "
+            "scale it based in the bytecode size.")
 DEFINE_BOOL(lazy_feedback_allocation, true, "Allocate feedback vectors lazily")
 
 // Flags for Ignition.

src/objects/js-function.cc

@@ -259,18 +259,37 @@ Handle<NativeContext> JSFunction::GetFunctionRealm(
 }
 
 // static
-void JSFunction::EnsureClosureFeedbackCellArray(Handle<JSFunction> function) {
+void JSFunction::EnsureClosureFeedbackCellArray(
+    Handle<JSFunction> function, bool reset_budget_for_feedback_allocation) {
   Isolate* const isolate = function->GetIsolate();
   DCHECK(function->shared().is_compiled());
   DCHECK(function->shared().HasFeedbackMetadata());
-  if (function->has_closure_feedback_cell_array() ||
-      function->has_feedback_vector()) {
-    return;
-  }
   if (function->shared().HasAsmWasmData()) return;
 
   Handle<SharedFunctionInfo> shared(function->shared(), isolate);
   DCHECK(function->shared().HasBytecodeArray());
+
+  bool has_closure_feedback_cell_array =
+      (function->has_closure_feedback_cell_array() ||
+       function->has_feedback_vector());
+  // Initialize the interrupt budget to the feedback vector allocation budget
+  // when initializing the feedback cell for the first time or after a bytecode
+  // flush. We retain the closure feedback cell array on bytecode flush, so
+  // reset_budget_for_feedback_allocation is used to reset the budget in these
+  // cases. When using a fixed allocation budget, we reset it on a bytecode
+  // flush so no additional initialization is required here.
+  if (V8_UNLIKELY(FLAG_feedback_allocation_on_bytecode_size) &&
+      (reset_budget_for_feedback_allocation ||
+       !has_closure_feedback_cell_array)) {
+    int budget = function->shared().GetBytecodeArray(isolate).length() *
+                 FLAG_scale_factor_for_feedback_allocation;
+    function->raw_feedback_cell().set_interrupt_budget(budget);
+  }
+
+  if (has_closure_feedback_cell_array) {
+    return;
+  }
+
   Handle<HeapObject> feedback_cell_array =
       ClosureFeedbackCellArray::New(isolate, shared);
   // Many closure cell is used as a way to specify that there is no
@@ -301,7 +320,7 @@ void JSFunction::EnsureFeedbackVector(Handle<JSFunction> function,
   Handle<SharedFunctionInfo> shared(function->shared(), isolate);
   DCHECK(function->shared().HasBytecodeArray());
 
-  EnsureClosureFeedbackCellArray(function);
+  EnsureClosureFeedbackCellArray(function, false);
   Handle<ClosureFeedbackCellArray> closure_feedback_cell_array =
       handle(function->closure_feedback_cell_array(), isolate);
   Handle<HeapObject> feedback_vector = FeedbackVector::New(
@@ -316,8 +335,9 @@ void JSFunction::EnsureFeedbackVector(Handle<JSFunction> function,
 }
 
 // static
-void JSFunction::InitializeFeedbackCell(Handle<JSFunction> function,
-                                        IsCompiledScope* is_compiled_scope) {
+void JSFunction::InitializeFeedbackCell(
+    Handle<JSFunction> function, IsCompiledScope* is_compiled_scope,
+    bool reset_budget_for_feedback_allocation) {
   Isolate* const isolate = function->GetIsolate();
 
   if (function->has_feedback_vector()) {
@@ -343,7 +363,8 @@ void JSFunction::InitializeFeedbackCell(Handle<JSFunction> function,
   if (needs_feedback_vector) {
     EnsureFeedbackVector(function, is_compiled_scope);
   } else {
-    EnsureClosureFeedbackCellArray(function);
+    EnsureClosureFeedbackCellArray(function,
+                                   reset_budget_for_feedback_allocation);
   }
 }
 

src/objects/js-function.h

@@ -184,14 +184,16 @@ class JSFunction : public JSFunctionOrBoundFunction {
   // lazily.
   inline bool has_closure_feedback_cell_array() const;
   inline ClosureFeedbackCellArray closure_feedback_cell_array() const;
-  static void EnsureClosureFeedbackCellArray(Handle<JSFunction> function);
+  static void EnsureClosureFeedbackCellArray(
+      Handle<JSFunction> function, bool reset_budget_for_feedback_allocation);
 
   // Initializes the feedback cell of |function|. In lite mode, this would be
   // initialized to the closure feedback cell array that holds the feedback
   // cells for create closure calls from this function. In the regular mode,
   // this allocates feedback vector.
   static void InitializeFeedbackCell(Handle<JSFunction> function,
-                                     IsCompiledScope* compiled_scope);
+                                     IsCompiledScope* compiled_scope,
+                                     bool reset_budget_for_feedback_allocation);
 
   // Unconditionally clear the type feedback vector.
   void ClearTypeFeedbackInfo();

test/mjsunit/compiler/number-divide.js

@@ -10,6 +10,7 @@
 (function() {
   // We need a separately polluted % with NumberOrOddball feedback.
   function bar(x) { return x / 2; }
+  %EnsureFeedbackVectorForFunction(bar);
   bar(undefined);  // The % feedback is now NumberOrOddball.
 
   // Now just use the gadget above in a way that only after RETYPE
@@ -40,6 +41,7 @@
 (function() {
   // We need a separately polluted % with NumberOrOddball feedback.
   function bar(x) { return x / 2; }
+  %EnsureFeedbackVectorForFunction(bar);
   bar(undefined);  // The % feedback is now NumberOrOddball.
 
   // Now just use the gadget above in a way that only after RETYPE

test/mjsunit/es6/collections-constructor-iterator-side-effect.js

@@ -26,6 +26,8 @@ function TestSetWithModifiedIterator(ctor) {
   arrayIteratorProto.next = originalNext;
 }
 %PrepareFunctionForOptimization(TestSetWithModifiedIterator);
+%EnsureFeedbackVectorForFunction(assertTrue);
+%EnsureFeedbackVectorForFunction(assertEquals);
 TestSetWithModifiedIterator(Set);
 TestSetWithModifiedIterator(Set);
 TestSetWithModifiedIterator(Set);

test/mjsunit/es6/map-constructor-entry-side-effect3.js

@@ -29,6 +29,9 @@ function TestMapConstructorEntrySideEffect(ctor) {
 }
 
 %PrepareFunctionForOptimization(TestMapConstructorEntrySideEffect);
+%EnsureFeedbackVectorForFunction(assertTrue);
+%EnsureFeedbackVectorForFunction(assertFalse);
+%EnsureFeedbackVectorForFunction(assertEquals);
 TestMapConstructorEntrySideEffect(Map);
 TestMapConstructorEntrySideEffect(Map);
 TestMapConstructorEntrySideEffect(Map);

test/mjsunit/smi-mul-const.js

@@ -50,6 +50,7 @@ function limit_range(a) {
   // Limit the range of 'a' to enable no-overflow optimizations.
   return Math.max(Math.min(a | 0, 10), -10);
 }
+%EnsureFeedbackVectorForFunction(limit_range);
 
 function mul_by_neg_127(a) { return limit_range(a) * -127; }
 function mul_by_neg_128(a) { return limit_range(a) * -128; }