Monomorphic prototype failures should be reserved for already-seen keys.

src/ic.cc

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 230 matching lines @@
       return;
     }
 
     object = proto;
   }
 }
 
 
 bool IC::TryRemoveInvalidPrototypeDependentStub(Handle<Object> receiver,
                                                 Handle<String> name) {
-  if (target()->is_keyed_stub()) {
-    // Determine whether the failure is due to a name failure.
-    if (!name->IsName()) return false;
-    Name* stub_name = target()->FindFirstName();
-    if (*name != stub_name) return false;
-  }
-
+  ASSERT(IsNameCompatibleWithMonomorphicPrototypeFailure(name));
   InlineCacheHolderFlag cache_holder =
       Code::ExtractCacheHolderFromFlags(target()->flags());
 
   switch (cache_holder) {
     case OWN_MAP:
       // The stub was generated for JSObject but called for non-JSObject.
       // IC::GetCodeCacheHolder is not applicable.
       if (!receiver->IsJSObject()) return false;
       break;
     case PROTOTYPE_MAP:
@@ skipping 61 matching lines @@
     Handle<Code> handler = handlers.at(i);
     int index = map->IndexInCodeCache(*name, *handler);
     if (index >= 0) {
       map->RemoveFromCodeCache(*name, *handler, index);
       return;
     }
   }
 }
 
 
+bool IC::IsNameCompatibleWithMonomorphicPrototypeFailure(Handle<Object> name) {
+  if (target()->is_keyed_stub()) {
+    // Determine whether the failure is due to a name failure.
+    if (!name->IsName()) return false;
+    Name* stub_name = target()->FindFirstName();
+    if (*name != stub_name) return false;
+  }
+
+  return true;
+}
+
+
 void IC::UpdateState(Handle<Object> receiver, Handle<Object> name) {
   if (!name->IsString()) return;
   if (state() != MONOMORPHIC) {
     if (state() == POLYMORPHIC && receiver->IsHeapObject()) {
       TryRemoveInvalidHandlers(
           handle(Handle<HeapObject>::cast(receiver)->map()),
           Handle<String>::cast(name));
     }
     return;
   }
   if (receiver->IsUndefined() || receiver->IsNull()) return;
 
   // Remove the target from the code cache if it became invalid
   // because of changes in the prototype chain to avoid hitting it
   // again.
-  if (TryRemoveInvalidPrototypeDependentStub(
+  if (IsNameCompatibleWithMonomorphicPrototypeFailure(name) &&
+      TryRemoveInvalidPrototypeDependentStub(
           receiver, Handle<String>::cast(name))) {
-    return MarkMonomorphicPrototypeFailure();
+    MarkMonomorphicPrototypeFailure(name);
+    return;
   }
 
   // The builtins object is special.  It only changes when JavaScript
   // builtins are loaded lazily.  It is important to keep inline
   // caches for the builtins object monomorphic.  Therefore, if we get
   // an inline cache miss for the builtins object after lazily loading
   // JavaScript builtins, we return uninitialized as the state to
   // force the inline cache back to monomorphic state.
   if (receiver->IsJSBuiltinsObject()) state_ = UNINITIALIZED;
 }
@@ skipping 801 matching lines @@
   PropertyDetails target_details = lookup->GetTransitionDetails();
   if (target_details.IsReadOnly()) return false;
 
   // If the value that's being stored does not fit in the field that the
   // instance would transition to, create a new transition that fits the value.
   // This has to be done before generating the IC, since that IC will embed the
   // transition target.
   // Ensure the instance and its map were migrated before trying to update the
   // transition target.
   ASSERT(!receiver->map()->is_deprecated());
+
+  if (!ic->IsNameCompatibleWithMonomorphicPrototypeFailure(name)) return false;

[Toon Verwaest, 2014/03/31 12:34:44]

This is not exactly the same...
The name only has to be compatible for the MarkMonomorphicPrototypeFailure(name)
below. That's why I suggested (and still suggest) moving it inside of
MarkMonomorphicPrototypeFailure(name), so that we don't need to explicitly guard
it at every use of MarkMonomorphicPrototypeFailure(name) (and asserting inside
of the method).

[mvstanton, 2014/03/31 13:30:57]

I ran into an issue with that idea. TryRemoiveInvalidPrototypeDependentStub()
carries out a lot of actions and is better served with a guard against going
forward if the name isn't found in the stub, hence the two-part nature of the
fix.

What I've done now is to

1) change MarkMonomorphicPrototypeFailure to TryMarkMonomorphicPrototypeFailure,
so this code site doesn't need to call
IsNameCompatibleWithMonomorphicPrototypeFailure().
2) In TryRemoiveInvalidPrototypeDependentStub(), an explicit call to
IsNameCompatibleWithMonomorphicPrototypeFailure is made.
3) No more dubious asserts.

   if (!value->FitsRepresentation(target_details.representation())) {
     Handle<Map> target(lookup->GetTransitionTarget());
     Map::GeneralizeRepresentation(
         target, target->LastAdded(),
         value->OptimalRepresentation(), FORCE_FIELD);
     // Lookup the transition again since the transition tree may have changed
     // entirely by the migration above.
     receiver->map()->LookupTransition(*holder, *name, lookup);
     if (!lookup->IsTransition()) return false;
-    ic->MarkMonomorphicPrototypeFailure();
+    ic->MarkMonomorphicPrototypeFailure(name);
   }
   return true;
 }
 
 
 MaybeObject* StoreIC::Store(Handle<Object> object,
                             Handle<String> name,
                             Handle<Object> value,
                             JSReceiver::StoreFromKeyed store_mode) {
   if (MigrateDeprecated(object) || object->IsJSProxy()) {
@@ skipping 1640 matching lines @@
 #undef ADDR
 };
 
 
 Address IC::AddressFromUtilityId(IC::UtilityId id) {
   return IC_utilities[id];
 }
 
 
 } }  // namespace v8::internal