[ic] Remember the receiver map in PREMONOMORPHIC state.

When going to PREMONOMORPHIC state we previously stored the
premonomorphic sentinel into the first IC slot and then the
second slot was storing the uninitialized sentinel. So when
TurboFan kicked in and optimized the function we'd just put
a LOAD_IC in there and hoped for the best that this is either
not in hot code or will reoptimize for another reason later
to fixup the LOAD_IC.

This is a quite annoying footgun for developers because the
performance inevitably depends on timing of when the optimizing
compiler kicks in.

To fix this issue we now keep a weak reference to the receiver
map in the second slot of the IC in PREMONOMORPHIC state and
use that to speculatively optimize when we go to TurboFan. This
improves the performance on the reported bug from

  spread: 2342 ms.
  spread: 2352 ms.
  spread: 2339 ms.

to

  spread: 1490 ms.
  spread: 1451 ms.
  spread: 1445 ms.

which corresponds to a 36% improvement in this particular case.
In general you'll get more predictable performance with this
change.

We might want to also use the map when going to MONOMORPHIC
state at a later point to maybe skip the additional transition
to POLYMORPHIC in some cases, but that's independent of this
bug.

Bug: v8:5267, v8:7973
Change-Id: Ia4eef7651e219a40927531cdffe320ade1dd19a4
Reviewed-on: https://chromium-review.googlesource.com/1148205Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#54682}

src/code-stub-assembler.cc

@@ -9632,10 +9632,13 @@ TNode<AllocationSite> CodeStubAssembler::CreateAllocationSiteInFeedbackVector(
 }
 
 TNode<MaybeObject> CodeStubAssembler::StoreWeakReferenceInFeedbackVector(
-    SloppyTNode<FeedbackVector> feedback_vector, SloppyTNode<IntPtrT> slot,
-    TNode<HeapObject> value) {
+    SloppyTNode<FeedbackVector> feedback_vector, Node* slot,
+    SloppyTNode<HeapObject> value, int additional_offset,
+    ParameterMode parameter_mode) {
   TNode<MaybeObject> weak_value = MakeWeak(value);
-  StoreFeedbackVectorSlot(feedback_vector, slot, weak_value);
+  StoreFeedbackVectorSlot(feedback_vector, slot, weak_value,
+                          UPDATE_WRITE_BARRIER, additional_offset,
+                          parameter_mode);
   return weak_value;
 }
 

src/code-stub-assembler.h

@@ -2463,8 +2463,9 @@ class V8_EXPORT_PRIVATE CodeStubAssembler : public compiler::CodeAssembler {
 
   // Store a weak in-place reference into the FeedbackVector.
   TNode<MaybeObject> StoreWeakReferenceInFeedbackVector(
-      SloppyTNode<FeedbackVector> feedback_vector, SloppyTNode<IntPtrT> slot,
-      TNode<HeapObject> value);
+      SloppyTNode<FeedbackVector> feedback_vector, Node* slot,
+      SloppyTNode<HeapObject> value, int additional_offset = 0,
+      ParameterMode parameter_mode = INTPTR_PARAMETERS);
 
   // Create a new AllocationSite and install it into a feedback vector.
   TNode<AllocationSite> CreateAllocationSiteInFeedbackVector(

src/feedback-vector.cc

@@ -431,6 +431,14 @@ void FeedbackNexus::ConfigureUninitialized() {
                   SKIP_WRITE_BARRIER);
       break;
     }
+    case FeedbackSlotKind::kStoreNamedSloppy:
+    case FeedbackSlotKind::kStoreNamedStrict:
+    case FeedbackSlotKind::kStoreKeyedSloppy:
+    case FeedbackSlotKind::kStoreKeyedStrict:
+    case FeedbackSlotKind::kStoreInArrayLiteral:
+    case FeedbackSlotKind::kStoreOwnNamed:
+    case FeedbackSlotKind::kLoadProperty:
+    case FeedbackSlotKind::kLoadKeyed:
     case FeedbackSlotKind::kStoreDataPropertyInLiteral: {
       SetFeedback(*FeedbackVector::UninitializedSentinel(isolate),
                   SKIP_WRITE_BARRIER);
@@ -471,12 +479,6 @@ bool FeedbackNexus::Clear() {
     case FeedbackSlotKind::kStoreOwnNamed:
     case FeedbackSlotKind::kLoadProperty:
     case FeedbackSlotKind::kLoadKeyed:
-      if (!IsCleared()) {
-        ConfigurePremonomorphic();
-        feedback_updated = true;
-      }
-      break;
-
     case FeedbackSlotKind::kStoreGlobalSloppy:
     case FeedbackSlotKind::kStoreGlobalStrict:
     case FeedbackSlotKind::kLoadGlobalNotInsideTypeof:
@@ -499,11 +501,10 @@ bool FeedbackNexus::Clear() {
   return feedback_updated;
 }
 
-void FeedbackNexus::ConfigurePremonomorphic() {
+void FeedbackNexus::ConfigurePremonomorphic(Handle<Map> receiver_map) {
   SetFeedback(*FeedbackVector::PremonomorphicSentinel(GetIsolate()),
               SKIP_WRITE_BARRIER);
-  SetFeedbackExtra(*FeedbackVector::UninitializedSentinel(GetIsolate()),
-                   SKIP_WRITE_BARRIER);
+  SetFeedbackExtra(HeapObjectReference::Weak(*receiver_map));
 }
 
 bool FeedbackNexus::ConfigureMegamorphic() {
@@ -931,6 +932,14 @@ int FeedbackNexus::ExtractMaps(MapHandles* maps) const {
     Map* map = Map::cast(heap_object);
     maps->push_back(handle(map, isolate));
     return 1;
+  } else if (feedback->ToStrongHeapObject(&heap_object) &&
+             heap_object ==
+                 heap_object->GetReadOnlyRoots().premonomorphic_symbol()) {
+    if (GetFeedbackExtra()->ToWeakHeapObject(&heap_object)) {
+      Map* map = Map::cast(heap_object);
+      maps->push_back(handle(map, isolate));
+      return 1;
+    }
   }
 
   return 0;

src/feedback-vector.h

@@ -610,7 +610,7 @@ class FeedbackNexus final {
   // Clear() returns true if the state of the underlying vector was changed.
   bool Clear();
   void ConfigureUninitialized();
-  void ConfigurePremonomorphic();
+  void ConfigurePremonomorphic(Handle<Map> receiver_map);
   // ConfigureMegamorphic() returns true if the state of the underlying vector
   // was changed. Extra feedback is cleared if the 0 parameter version is used.
   bool ConfigureMegamorphic();

src/ic/accessor-assembler.cc

@@ -2482,6 +2482,8 @@ void AccessorAssembler::LoadIC_Uninitialized(const LoadICParameters* p) {
   StoreFeedbackVectorSlot(p->vector, p->slot,
                           LoadRoot(Heap::kpremonomorphic_symbolRootIndex),
                           SKIP_WRITE_BARRIER, 0, SMI_PARAMETERS);
+  StoreWeakReferenceInFeedbackVector(p->vector, p->slot, receiver_map,
+                                     kPointerSize, SMI_PARAMETERS);
 
   {
     // Special case for Function.prototype load, because it's very common

src/ic/ic.cc

@@ -369,26 +369,24 @@ static bool MigrateDeprecated(Handle<Object> object) {
 }
 
 bool IC::ConfigureVectorState(IC::State new_state, Handle<Object> key) {
-  bool changed = true;
-  if (new_state == PREMONOMORPHIC) {
-    nexus()->ConfigurePremonomorphic();
-  } else if (new_state == MEGAMORPHIC) {
-    DCHECK_IMPLIES(!is_keyed(), key->IsName());
-    // Even though we don't change the feedback data, we still want to reset the
-    // profiler ticks. Real-world observations suggest that optimizing these
-    // functions doesn't improve performance.
-    changed = nexus()->ConfigureMegamorphic(key->IsName() ? PROPERTY : ELEMENT);
-  } else {
-    UNREACHABLE();
-  }
-
+  DCHECK_EQ(MEGAMORPHIC, new_state);
+  DCHECK_IMPLIES(!is_keyed(), key->IsName());
+  // Even though we don't change the feedback data, we still want to reset the
+  // profiler ticks. Real-world observations suggest that optimizing these
+  // functions doesn't improve performance.
+  bool changed =
+      nexus()->ConfigureMegamorphic(key->IsName() ? PROPERTY : ELEMENT);
   vector_set_ = true;
-  OnFeedbackChanged(
-      isolate(), nexus(), GetHostFunction(),
-      new_state == PREMONOMORPHIC ? "Premonomorphic" : "Megamorphic");
+  OnFeedbackChanged(isolate(), nexus(), GetHostFunction(), "Megamorphic");
   return changed;
 }
 
+void IC::ConfigureVectorState(Handle<Map> map) {
+  nexus()->ConfigurePremonomorphic(map);
+  vector_set_ = true;
+  OnFeedbackChanged(isolate(), nexus(), GetHostFunction(), "Premonomorphic");
+}
+
 void IC::ConfigureVectorState(Handle<Name> name, Handle<Map> map,
                               Handle<Object> handler) {
   ConfigureVectorState(name, map, MaybeObjectHandle(handler));
@@ -679,7 +677,7 @@ void LoadIC::UpdateCaches(LookupIterator* lookup) {
     // This is the first time we execute this inline cache. Set the target to
     // the pre monomorphic stub to delay setting the monomorphic state.
     TRACE_HANDLER_STATS(isolate(), LoadIC_Premonomorphic);
-    ConfigureVectorState(PREMONOMORPHIC, Handle<Object>());
+    ConfigureVectorState(receiver_map());
     TraceIC("LoadIC", lookup->name());
     return;
   }
@@ -1436,10 +1434,10 @@ MaybeHandle<Object> StoreIC::Store(Handle<Object> object, Handle<Name> name,
 void StoreIC::UpdateCaches(LookupIterator* lookup, Handle<Object> value,
                            JSReceiver::StoreFromKeyed store_mode) {
   if (state() == UNINITIALIZED && !IsStoreGlobalIC()) {
-    // This is the first time we execute this inline cache. Set the target to
-    // the pre monomorphic stub to delay setting the monomorphic state.
+    // This is the first time we execute this inline cache. Transition
+    // to premonomorphic state to delay setting the monomorphic state.
     TRACE_HANDLER_STATS(isolate(), StoreIC_Premonomorphic);
-    ConfigureVectorState(PREMONOMORPHIC, Handle<Object>());
+    ConfigureVectorState(receiver_map());
     TraceIC("StoreIC", lookup->name());
     return;
   }
@@ -1788,7 +1786,10 @@ void KeyedStoreIC::UpdateStoreElement(Handle<Map> receiver_map,
   handlers.reserve(target_receiver_maps.size());
   StoreElementPolymorphicHandlers(&target_receiver_maps, &handlers, store_mode);
   if (target_receiver_maps.size() == 0) {
-    ConfigureVectorState(PREMONOMORPHIC, Handle<Name>());
+    // Transition to PREMONOMORPHIC state here and remember a weak-reference
+    // to the {receiver_map} in case TurboFan sees this function before the
+    // IC can transition further.
+    ConfigureVectorState(receiver_map);
   } else if (target_receiver_maps.size() == 1) {
     ConfigureVectorState(Handle<Name>(), target_receiver_maps[0], handlers[0]);
   } else {

src/ic/ic.h

@@ -93,6 +93,8 @@ class IC {
 
   // Configure for most states.
   bool ConfigureVectorState(IC::State new_state, Handle<Object> key);
+  // Configure the vector for PREMONOMORPHIC.
+  void ConfigureVectorState(Handle<Map> map);
   // Configure the vector for MONOMORPHIC.
   void ConfigureVectorState(Handle<Name> name, Handle<Map> map,
                             Handle<Object> handler);