factory: Move FeedbackVector construction to type

Drive-by: Avoid unnecessary memset and clean up Init().

Bug: v8:12559
Change-Id: I6a79f42dd62b47397d70f92efec3b569ca664c3e
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3404097Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#78722}

src/heap/factory-inl.h

@@ -85,6 +85,49 @@ Factory::CodeBuilder& Factory::CodeBuilder::set_interpreter_data(
   return *this;
 }
 
+// static
+void Factory::VerifyInit(Isolate* isolate, HeapObject heap_object) {
+#ifdef MEMORY_SANITIZER
+  // T::Init() must initialize all memory.
+  __msan_check_mem_is_initialized(reinterpret_cast<void*>(heap_object.ptr()),
+                                  heap_object.Size());
+#endif  // MEMORY_SANITIZER
+#if VERIFY_HEAP
+  if (FLAG_verify_heap) {
+    heap_object.HeapObjectVerify(isolate);
+  }
+#endif  // VERIFY_HEAP
+}
+
+template <typename T, typename... Params>
+// static
+Handle<T> Factory::InitializeAndVerify(
+    Isolate* isolate, WriteBarrierMode write_barrier_mode_for_regular_writes,
+    T raw, Params&&... params) {
+  {
+    DisallowGarbageCollection no_gc;
+    T::Init(isolate, no_gc, write_barrier_mode_for_regular_writes, raw,
+            std::forward<Params>(params)...);
+    VerifyInit(isolate, raw);
+  }
+  Handle<T> result(raw, isolate);
+  T::PostInit(isolate, result);
+  return result;
+}
+
+template <typename... Params>
+V8_INLINE Handle<FeedbackVector> Factory::NewFeedbackVector(
+    Handle<SharedFunctionInfo> shared, Params&&... params) {
+  const int length = shared->feedback_metadata().slot_count();
+  DCHECK_LE(0, length);
+  const int size = FeedbackVector::SizeFor(length);
+  FeedbackVector raw_result = FeedbackVector::cast(AllocateRawWithImmortalMap(
+      size, AllocationType::kOld, *feedback_vector_map()));
+  return InitializeAndVerify(isolate(), WriteBarrierMode::UPDATE_WRITE_BARRIER,
+                             raw_result, shared, length,
+                             std::forward<Params>(params)...);
+}
+
 }  // namespace internal
 }  // namespace v8
 

src/heap/factory.cc

@@ -490,30 +490,6 @@ Handle<ClosureFeedbackCellArray> Factory::NewClosureFeedbackCellArray(
   return feedback_cell_array;
 }
 
-Handle<FeedbackVector> Factory::NewFeedbackVector(
-    Handle<SharedFunctionInfo> shared,
-    Handle<ClosureFeedbackCellArray> closure_feedback_cell_array) {
-  int length = shared->feedback_metadata().slot_count();
-  DCHECK_LE(0, length);
-  int size = FeedbackVector::SizeFor(length);
-
-  FeedbackVector vector = FeedbackVector::cast(AllocateRawWithImmortalMap(
-      size, AllocationType::kOld, *feedback_vector_map()));
-  DisallowGarbageCollection no_gc;
-  vector.set_shared_function_info(*shared);
-  vector.set_maybe_optimized_code(HeapObjectReference::ClearedValue(isolate()),
-                                  kReleaseStore);
-  vector.set_length(length);
-  vector.set_invocation_count(0);
-  vector.set_profiler_ticks(0);
-  vector.InitializeOptimizationState();
-  vector.set_closure_feedback_cell_array(*closure_feedback_cell_array);
-
-  // TODO(leszeks): Initialize based on the feedback metadata.
-  MemsetTagged(ObjectSlot(vector.slots_start()), *undefined_value(), length);
-  return handle(vector, isolate());
-}
-
 Handle<EmbedderDataArray> Factory::NewEmbedderDataArray(int length) {
   DCHECK_LE(0, length);
   int size = EmbedderDataArray::SizeFor(length);

src/heap/factory.h

@@ -142,11 +142,9 @@ class V8_EXPORT_PRIVATE Factory : public FactoryBase<Factory> {
   // initialized with undefined values.
   Handle<ClosureFeedbackCellArray> NewClosureFeedbackCellArray(int num_slots);
 
-  // Allocates a feedback vector whose slots are initialized with undefined
-  // values.
-  Handle<FeedbackVector> NewFeedbackVector(
-      Handle<SharedFunctionInfo> shared,
-      Handle<ClosureFeedbackCellArray> closure_feedback_cell_array);
+  template <typename... Params>
+  V8_INLINE Handle<FeedbackVector> NewFeedbackVector(
+      Handle<SharedFunctionInfo> shared, Params&&... params);
 
   // Allocates a clean embedder data array with given capacity.
   Handle<EmbedderDataArray> NewEmbedderDataArray(int length);
@@ -1000,6 +998,13 @@ class V8_EXPORT_PRIVATE Factory : public FactoryBase<Factory> {
  private:
   friend class FactoryBase<Factory>;
 
+  template <typename T, typename... Params>
+  static V8_INLINE Handle<T> InitializeAndVerify(
+      Isolate* isolate, WriteBarrierMode write_barrier_mode_for_regular_writes,
+      T raw, Params&&... params);
+
+  static V8_INLINE void VerifyInit(Isolate*, HeapObject heap_object);
+
   // ------
   // Customization points for FactoryBase
   HeapObject AllocateRaw(int size, AllocationType allocation,

src/objects/feedback-vector.cc

@@ -244,59 +244,57 @@ Handle<ClosureFeedbackCellArray> ClosureFeedbackCellArray::New(
 }
 
 // static
-Handle<FeedbackVector> FeedbackVector::New(
-    Isolate* isolate, Handle<SharedFunctionInfo> shared,
+void FeedbackVector::Init(
+    Isolate* isolate, const DisallowGarbageCollection&,
+    WriteBarrierMode write_barrier_mode, FeedbackVector vector,
+    Handle<SharedFunctionInfo> shared, int length,
     Handle<ClosureFeedbackCellArray> closure_feedback_cell_array,
     IsCompiledScope* is_compiled_scope) {
   DCHECK(is_compiled_scope->is_compiled());
-  Factory* factory = isolate->factory();
-
-  Handle<FeedbackMetadata> feedback_metadata(shared->feedback_metadata(),
-                                             isolate);
-  const int slot_count = feedback_metadata->slot_count();
-
-  Handle<FeedbackVector> vector =
-      factory->NewFeedbackVector(shared, closure_feedback_cell_array);
 
-  DCHECK_EQ(vector->length(), slot_count);
-
-  DCHECK_EQ(vector->shared_function_info(), *shared);
-  DCHECK_EQ(vector->optimization_marker(),
+  vector.set_shared_function_info(*shared, write_barrier_mode);
+  vector.set_maybe_optimized_code(
+      HeapObjectReference::ClearedValue(vector.GetIsolate()), kReleaseStore,
+      write_barrier_mode);
+  vector.set_length(length);
+  vector.set_invocation_count(0);
+  vector.set_profiler_ticks(0);
+  vector.InitializeOptimizationState();
+  vector.set_closure_feedback_cell_array(*closure_feedback_cell_array,
+                                         write_barrier_mode);
+
+  const FeedbackMetadata feedback_metadata = shared->feedback_metadata();
+  DCHECK_EQ(length, feedback_metadata.slot_count());
+  DCHECK_EQ(vector.optimization_marker(),
             FLAG_log_function_events ? OptimizationMarker::kLogFirstExecution
                                      : OptimizationMarker::kNone);
-  DCHECK_EQ(vector->optimization_tier(), OptimizationTier::kNone);
-  DCHECK_EQ(vector->invocation_count(), 0);
-  DCHECK_EQ(vector->profiler_ticks(), 0);
-  DCHECK(vector->maybe_optimized_code()->IsCleared());
-
-  // Ensure we can skip the write barrier
-  Handle<Symbol> uninitialized_sentinel = UninitializedSentinel(isolate);
-  DCHECK_EQ(ReadOnlyRoots(isolate).uninitialized_symbol(),
-            *uninitialized_sentinel);
-  for (int i = 0; i < slot_count;) {
-    FeedbackSlot slot(i);
-    FeedbackSlotKind kind = feedback_metadata->GetKind(slot);
-    int entry_size = FeedbackMetadata::GetSlotSize(kind);
+  DCHECK_EQ(vector.optimization_tier(), OptimizationTier::kNone);
+  DCHECK(vector.maybe_optimized_code()->IsCleared());
 
-    MaybeObject extra_value = MaybeObject::FromObject(*uninitialized_sentinel);
+  const Symbol uninitialized_sentinel =
+      ReadOnlyRoots(isolate).uninitialized_symbol();
+  for (int i = 0; i < length;) {
+    FeedbackSlot slot(i);
+    const FeedbackSlotKind kind = feedback_metadata.GetKind(slot);
+    MaybeObject extra_value = MaybeObject::FromObject(uninitialized_sentinel);
     switch (kind) {
       case FeedbackSlotKind::kLoadGlobalInsideTypeof:
       case FeedbackSlotKind::kLoadGlobalNotInsideTypeof:
       case FeedbackSlotKind::kStoreGlobalSloppy:
       case FeedbackSlotKind::kStoreGlobalStrict:
-        vector->Set(slot, HeapObjectReference::ClearedValue(isolate),
-                    SKIP_WRITE_BARRIER);
+        vector.Set(slot, HeapObjectReference::ClearedValue(isolate),
+                   SKIP_WRITE_BARRIER);
         break;
       case FeedbackSlotKind::kForIn:
       case FeedbackSlotKind::kCompareOp:
       case FeedbackSlotKind::kBinaryOp:
-        vector->Set(slot, Smi::zero(), SKIP_WRITE_BARRIER);
+        vector.Set(slot, Smi::zero(), SKIP_WRITE_BARRIER);
         break;
       case FeedbackSlotKind::kLiteral:
-        vector->Set(slot, Smi::zero(), SKIP_WRITE_BARRIER);
+        vector.Set(slot, Smi::zero(), SKIP_WRITE_BARRIER);
         break;
       case FeedbackSlotKind::kCall:
-        vector->Set(slot, *uninitialized_sentinel, SKIP_WRITE_BARRIER);
+        vector.Set(slot, uninitialized_sentinel, SKIP_WRITE_BARRIER);
         extra_value = MaybeObject::FromObject(Smi::zero());
         break;
       case FeedbackSlotKind::kCloneObject:
@@ -313,25 +311,27 @@ Handle<FeedbackVector> FeedbackVector::New(
       case FeedbackSlotKind::kStoreDataPropertyInLiteral:
       case FeedbackSlotKind::kTypeProfile:
       case FeedbackSlotKind::kInstanceOf:
-        vector->Set(slot, *uninitialized_sentinel, SKIP_WRITE_BARRIER);
+        vector.Set(slot, uninitialized_sentinel, SKIP_WRITE_BARRIER);
         break;
 
       case FeedbackSlotKind::kInvalid:
       case FeedbackSlotKind::kKindsNumber:
         UNREACHABLE();
     }
+    const int entry_size = FeedbackMetadata::GetSlotSize(kind);
     for (int j = 1; j < entry_size; j++) {
-      vector->Set(slot.WithOffset(j), extra_value, SKIP_WRITE_BARRIER);
+      vector.Set(slot.WithOffset(j), extra_value, SKIP_WRITE_BARRIER);
     }
     i += entry_size;
   }
+}
 
-  Handle<FeedbackVector> result = Handle<FeedbackVector>::cast(vector);
+// static
+void FeedbackVector::PostInit(Isolate* isolate, Handle<FeedbackVector> vector) {
   if (!isolate->is_best_effort_code_coverage() ||
       isolate->is_collecting_type_profile()) {
-    AddToVectorsForProfilingTools(isolate, result);
+    AddToVectorsForProfilingTools(isolate, vector);
   }
-  return result;
 }
 
 namespace {
@@ -349,8 +349,8 @@ Handle<FeedbackVector> NewFeedbackVectorForTesting(
       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 isolate->factory()->NewFeedbackVector(
+      shared, closure_feedback_cell_array, &is_compiled_scope);
 }
 
 }  // namespace

src/objects/feedback-vector.h

@@ -15,6 +15,7 @@
 #include "src/objects/map.h"
 #include "src/objects/maybe-object.h"
 #include "src/objects/name.h"
+#include "src/objects/objects.h"
 #include "src/objects/type-hints.h"
 #include "src/zone/zone-containers.h"
 
@@ -222,6 +223,15 @@ class FeedbackVector
       OptimizationTierBits::kMask |
       kHasCompileOptimizedOrLogFirstExecutionMarker;
 
+  V8_EXPORT_PRIVATE static void Init(
+      Isolate*, const DisallowGarbageCollection&,
+      WriteBarrierMode write_barrier_mode, FeedbackVector vector,
+      Handle<SharedFunctionInfo> shared, int length,
+      Handle<ClosureFeedbackCellArray> closure_feedback_cell_array,
+      IsCompiledScope* is_compiled_scope);
+  V8_EXPORT_PRIVATE static void PostInit(Isolate* isolate,
+                                         Handle<FeedbackVector> vector);
+
   inline bool is_empty() const;
 
   inline FeedbackMetadata metadata() const;
@@ -289,11 +299,6 @@ class FeedbackVector
 
   FeedbackSlot GetTypeProfileSlot() const;
 
-  V8_EXPORT_PRIVATE static Handle<FeedbackVector> New(
-      Isolate* isolate, Handle<SharedFunctionInfo> shared,
-      Handle<ClosureFeedbackCellArray> closure_feedback_cell_array,
-      IsCompiledScope* is_compiled_scope);
-
   V8_EXPORT_PRIVATE static Handle<FeedbackVector>
   NewWithOneBinarySlotForTesting(Zone* zone, Isolate* isolate);
   V8_EXPORT_PRIVATE static Handle<FeedbackVector>
@@ -359,7 +364,7 @@ class FeedbackVector
   static void AddToVectorsForProfilingTools(Isolate* isolate,
                                             Handle<FeedbackVector> vector);
 
-  // Private for initializing stores in FeedbackVector::New().
+  // Private for initializing stores in FeedbackVector::Init().
   inline void Set(FeedbackSlot slot, MaybeObject value,
                   WriteBarrierMode mode = UPDATE_WRITE_BARRIER);
   inline void Set(FeedbackSlot slot, Object value,

src/objects/heap-object.h

@@ -26,6 +26,9 @@ class PrimitiveHeapObject;
 // objects.
 class HeapObject : public Object {
  public:
+  template <typename T>
+  static void PostInit(Isolate* isolate, Handle<T> object) {}
+
   bool is_null() const {
     return static_cast<Tagged_t>(ptr()) == static_cast<Tagged_t>(kNullAddress);
   }

src/objects/js-function.cc

@@ -295,8 +295,9 @@ void JSFunction::EnsureFeedbackVector(Handle<JSFunction> function,
   EnsureClosureFeedbackCellArray(function, false);
   Handle<ClosureFeedbackCellArray> closure_feedback_cell_array =
       handle(function->closure_feedback_cell_array(), isolate);
-  Handle<HeapObject> feedback_vector = FeedbackVector::New(
-      isolate, shared, closure_feedback_cell_array, is_compiled_scope);
+  Handle<FeedbackVector> feedback_vector =
+      isolate->factory()->NewFeedbackVector(shared, closure_feedback_cell_array,
+                                            is_compiled_scope);
   // 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.

test/cctest/test-feedback-vector.h

@@ -5,6 +5,7 @@
 #ifndef V8_TEST_FEEDBACK_VECTOR_H_
 #define V8_TEST_FEEDBACK_VECTOR_H_
 
+#include "src/execution/isolate.h"
 #include "src/objects/feedback-vector.h"
 #include "src/objects/objects.h"
 #include "src/objects/shared-function-info.h"
@@ -52,8 +53,8 @@ Handle<FeedbackVector> NewFeedbackVector(Isolate* isolate, Spec* spec) {
   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 isolate->factory()->NewFeedbackVector(
+      shared, closure_feedback_cell_array, &is_compiled_scope);
 }
 
 template <typename Spec>

test/cctest/test-heap-profiler.cc

@@ -4119,8 +4119,10 @@ 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);
+  v8::internal::IsCompiledScope is_compiled_scope(
+      shared_function->is_compiled_scope(i_isolate));
+  i::Handle<i::FeedbackVector> fv = factory->NewFeedbackVector(
+      shared_function, feedback_cell_array, &is_compiled_scope);
 
   // Create a Code.
   i::Assembler assm(i::AssemblerOptions{});

test/unittests/compiler/js-call-reducer-unittest.cc

@@ -111,8 +111,8 @@ class JSCallReducerTest : public TypedGraphTest {
     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 = isolate()->factory()->NewFeedbackVector(
+        shared, closure_feedback_cell_array, &is_compiled_scope);
     FeedbackSource feedback(vector, FeedbackSlot(0));
     return javascript()->Call(JSCallNode::ArityForArgc(arity), CallFrequency(),
                               feedback, ConvertReceiverMode::kAny,

test/unittests/compiler/redundancy-elimination-unittest.cc

@@ -40,8 +40,9 @@ class RedundancyEliminationTest : public GraphTest {
     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 =
+        isolate()->factory()->NewFeedbackVector(
+            shared, closure_feedback_cell_array, &is_compiled_scope);
     vector_slot_pairs_.push_back(FeedbackSource());
     vector_slot_pairs_.push_back(FeedbackSource(feedback_vector, slot1));
     vector_slot_pairs_.push_back(FeedbackSource(feedback_vector, slot2));