Revert "factory: Move FeedbackVector construction to type"

This reverts commit 77be1c60.

Reason for revert: breaks msan https://ci.chromium.org/ui/p/v8/builders/ci/V8%20Linux%20-%20arm64%20-%20sim%20-%20MSAN/42062/overview

Original change's description:
> 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/+/3404097
> Reviewed-by: Leszek Swirski <leszeks@chromium.org>
> Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#78722}

Bug: v8:12559
Change-Id: I79bea5753eeadf209dc2867c8387cc42c675e567
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3406757
Auto-Submit: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#78724}

src/heap/factory-inl.h

@@ -85,49 +85,6 @@ 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,6 +490,30 @@ 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,9 +142,11 @@ class V8_EXPORT_PRIVATE Factory : public FactoryBase<Factory> {
   // initialized with undefined values.
   Handle<ClosureFeedbackCellArray> NewClosureFeedbackCellArray(int num_slots);
 
-  template <typename... Params>
-  V8_INLINE Handle<FeedbackVector> NewFeedbackVector(
-      Handle<SharedFunctionInfo> shared, Params&&... params);
+  // Allocates a feedback vector whose slots are initialized with undefined
+  // values.
+  Handle<FeedbackVector> NewFeedbackVector(
+      Handle<SharedFunctionInfo> shared,
+      Handle<ClosureFeedbackCellArray> closure_feedback_cell_array);
 
   // Allocates a clean embedder data array with given capacity.
   Handle<EmbedderDataArray> NewEmbedderDataArray(int length);
@@ -998,13 +1000,6 @@ 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,57 +244,59 @@ Handle<ClosureFeedbackCellArray> ClosureFeedbackCellArray::New(
 }
 
 // static
-void FeedbackVector::Init(
-    Isolate* isolate, const DisallowGarbageCollection&,
-    WriteBarrierMode write_barrier_mode, FeedbackVector vector,
-    Handle<SharedFunctionInfo> shared, int length,
+Handle<FeedbackVector> FeedbackVector::New(
+    Isolate* isolate, Handle<SharedFunctionInfo> shared,
     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);
 
-  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(),
+  DCHECK_EQ(vector->length(), slot_count);
+
+  DCHECK_EQ(vector->shared_function_info(), *shared);
+  DCHECK_EQ(vector->optimization_marker(),
             FLAG_log_function_events ? OptimizationMarker::kLogFirstExecution
                                      : OptimizationMarker::kNone);
-  DCHECK_EQ(vector.optimization_tier(), OptimizationTier::kNone);
-  DCHECK(vector.maybe_optimized_code()->IsCleared());
-
-  const Symbol uninitialized_sentinel =
-      ReadOnlyRoots(isolate).uninitialized_symbol();
-  for (int i = 0; i < length;) {
+  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);
-    const FeedbackSlotKind kind = feedback_metadata.GetKind(slot);
-    MaybeObject extra_value = MaybeObject::FromObject(uninitialized_sentinel);
+    FeedbackSlotKind kind = feedback_metadata->GetKind(slot);
+    int entry_size = FeedbackMetadata::GetSlotSize(kind);
+
+    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:
@@ -311,27 +313,25 @@ void FeedbackVector::Init(
       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;
   }
-}
 
-// static
-void FeedbackVector::PostInit(Isolate* isolate, Handle<FeedbackVector> vector) {
+  Handle<FeedbackVector> result = Handle<FeedbackVector>::cast(vector);
   if (!isolate->is_best_effort_code_coverage() ||
       isolate->is_collecting_type_profile()) {
-    AddToVectorsForProfilingTools(isolate, vector);
+    AddToVectorsForProfilingTools(isolate, result);
   }
+  return result;
 }
 
 namespace {
@@ -349,8 +349,8 @@ Handle<FeedbackVector> NewFeedbackVectorForTesting(
       ClosureFeedbackCellArray::New(isolate, shared);
 
   IsCompiledScope is_compiled_scope(shared->is_compiled_scope(isolate));
-  return isolate->factory()->NewFeedbackVector(
-      shared, closure_feedback_cell_array, &is_compiled_scope);
+  return FeedbackVector::New(isolate, shared, closure_feedback_cell_array,
+                             &is_compiled_scope);
 }
 
 }  // namespace

src/objects/feedback-vector.h

@@ -15,7 +15,6 @@
 #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"
 
@@ -223,15 +222,6 @@ 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;
@@ -299,6 +289,11 @@ 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>
@@ -364,7 +359,7 @@ class FeedbackVector
   static void AddToVectorsForProfilingTools(Isolate* isolate,
                                             Handle<FeedbackVector> vector);
 
-  // Private for initializing stores in FeedbackVector::Init().
+  // Private for initializing stores in FeedbackVector::New().
   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,9 +26,6 @@ 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,9 +295,8 @@ void JSFunction::EnsureFeedbackVector(Handle<JSFunction> function,
   EnsureClosureFeedbackCellArray(function, false);
   Handle<ClosureFeedbackCellArray> closure_feedback_cell_array =
       handle(function->closure_feedback_cell_array(), isolate);
-  Handle<FeedbackVector> feedback_vector =
-      isolate->factory()->NewFeedbackVector(shared, closure_feedback_cell_array,
-                                            is_compiled_scope);
+  Handle<HeapObject> feedback_vector = FeedbackVector::New(
+      isolate, 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,7 +5,6 @@
 #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"
@@ -53,8 +52,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 isolate->factory()->NewFeedbackVector(
-      shared, closure_feedback_cell_array, &is_compiled_scope);
+  return FeedbackVector::New(isolate, shared, closure_feedback_cell_array,
+                             &is_compiled_scope);
 }
 
 template <typename Spec>

test/cctest/test-heap-profiler.cc

@@ -4119,10 +4119,8 @@ TEST(WeakReference) {
                                        i_isolate);
   i::Handle<i::ClosureFeedbackCellArray> feedback_cell_array =
       i::ClosureFeedbackCellArray::New(i_isolate, shared_function);
-  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);
+  i::Handle<i::FeedbackVector> fv =
+      factory->NewFeedbackVector(shared_function, feedback_cell_array);
 
   // 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 = isolate()->factory()->NewFeedbackVector(
-        shared, closure_feedback_cell_array, &is_compiled_scope);
+    Handle<FeedbackVector> vector = FeedbackVector::New(
+        isolate(), 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,9 +40,8 @@ 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 =
-        isolate()->factory()->NewFeedbackVector(
-            shared, closure_feedback_cell_array, &is_compiled_scope);
+    Handle<FeedbackVector> feedback_vector = FeedbackVector::New(
+        isolate(), 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));