Correct handling of arrays with callbacks in the prototype chain.

src/objects.cc

Index: src/objects.cc
diff --git a/src/objects.cc b/src/objects.cc
index 2f84975d6fa77d9ecf1e3f0aa9aaa9378027f24f..d2b988acf6d2ecd0c99ba6816b32bfee49190462 100644
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -2923,6 +2923,25 @@ Handle<Object> JSReceiver::SetPropertyWithDefinedSetter(
 }
 
 
+bool JSReceiver::MayHaveIndexedCallbacksInPrototypeChain() {
+  Heap* heap = GetHeap();
+  for (Object* pt = GetPrototype();
+       pt != heap->null_value();
+       pt = pt->GetPrototype(GetIsolate())) {
+    if (pt->IsJSProxy()) {
+      // Be conservative, don't walk into proxies.
+      return true;
+    }
+
+    if (JSObject::cast(pt)->map()->has_element_callbacks()) {
+      return true;
+    }
+  }
+
+  return false;
+}
+
+
 MaybeObject* JSObject::SetElementWithCallbackSetterInPrototypes(
     uint32_t index,
     Object* value,
@@ -6204,12 +6223,14 @@ void JSObject::SetElementCallback(Handle<JSObject> object,
                                   Handle<Object> structure,
                                   PropertyAttributes attributes) {
   Heap* heap = object->GetHeap();
+  Handle<Map> old_map(object->map());
   PropertyDetails details = PropertyDetails(attributes, CALLBACKS, 0);
 
   // Normalize elements to make this operation simple.
   Handle<SeededNumberDictionary> dictionary = NormalizeElements(object);
   ASSERT(object->HasDictionaryElements() ||
          object->HasDictionaryArgumentsElements());
+  bool map_changed = object->map() != *old_map;
 
   // Update the dictionary with the new CALLBACKS property.
   dictionary = SeededNumberDictionary::Set(dictionary, index, structure,
@@ -6231,6 +6252,34 @@ void JSObject::SetElementCallback(Handle<JSObject> object,
   } else {
     object->set_elements(*dictionary);
   }
+
+  if (!object->map()->has_element_callbacks()) {
+    if (!map_changed) {
+      Handle<Map> new_map = Map::Copy(handle(object->map()));
+      object->set_map(*new_map);
+    }
+    object->map()->set_has_element_callbacks(true);

[danno, 2013/10/23 12:22:11]

This isn't quite right. You might "pollute" normal maps that make this
transition that don't have setters/getters. You need to use the map transition
mechanism (see how freeze/seal map transitions works, this should essentially
work in exactly the same way).

+  }
+
+  // If we are the array prototype object, rebuild the cache.
+  Isolate* isolate = object->GetIsolate();
+  if (isolate->initial_array_prototype().is_identical_to(object)) {
+    // Install the initial map, new_ek_map in the function prototype for

[danno, 2013/10/23 12:22:11]

Is this comment correct?

+    // array.
+    Handle<JSFunction> array_function(
+        isolate->global_context()->array_function(), isolate);
+    JSFunction::SetPrototype(array_function, object);

[danno, 2013/10/23 12:22:11]

You are "rebuilding the cache" by side effect, which is kind of scary. Setting
the prototype triggers (might trigger?) DependentCode deoptimization for
prototype chains, which I don't think is relevant in this case. Please find a
way to do the cache re-building explicitly if it is even necessary at all
(didn't we convince ourselves that it's not?)

+  } else if (isolate->initial_object_prototype().is_identical_to(object)) {
+    Handle<JSFunction> object_function(
+        isolate->global_context()->object_function(), isolate);
+    JSFunction::SetPrototype(object_function, object);
+  }
+
+  // TODO(mvstanton): With a dependency group we can avoid the need to
+  // deoptimize code that doesn't depend on the changed maps. Address this
+  // in a follow-up checkin.
+  Deoptimizer::DeoptimizeAll(object->GetIsolate());

[danno, 2013/10/23 12:22:11]

Oh, *the humanity* Are you sure it can't be in this CL? It's pretty mechanical
extension of the current mechanism to add a new group and add the dependency in
hydrogen.cc

+  object->GetHeap()->ClearAllKeyedStoreICs();
 }
 
 
@@ -10560,6 +10609,16 @@ void Code::ReplaceNthCell(int n, Cell* replace_with) {
 
 
 void Code::ClearInlineCaches() {
+  ClearInlineCaches(NULL);
+}
+
+
+void Code::ClearInlineCaches(Code::Kind kind) {
+  ClearInlineCaches(&kind);
+}
+
+
+void Code::ClearInlineCaches(Code::Kind* kind) {
   int mask = RelocInfo::ModeMask(RelocInfo::CODE_TARGET) |
              RelocInfo::ModeMask(RelocInfo::CONSTRUCT_CALL) |
              RelocInfo::ModeMask(RelocInfo::CODE_TARGET_WITH_ID) |
@@ -10568,7 +10627,9 @@ void Code::ClearInlineCaches() {
     RelocInfo* info = it.rinfo();
     Code* target(Code::GetCodeFromTargetAddress(info->target_address()));
     if (target->is_inline_cache_stub()) {
-      IC::Clear(this->GetIsolate(), info->pc());
+      if (kind == NULL || *kind == target->kind()) {
+        IC::Clear(this->GetIsolate(), info->pc());
+      }
     }
   }
 }
@@ -12738,8 +12799,11 @@ MaybeObject* JSObject::TransitionElementsKind(ElementsKind to_kind) {
 
   if (from_kind == to_kind) return this;
 
-  MaybeObject* maybe_failure = UpdateAllocationSite(to_kind);
-  if (maybe_failure->IsFailure()) return maybe_failure;
+  // Don't update the site if to_kind isn't fast
+  if (IsFastElementsKind(to_kind)) {
+    MaybeObject* maybe_failure = UpdateAllocationSite(to_kind);
+    if (maybe_failure->IsFailure()) return maybe_failure;
+  }
 
   Isolate* isolate = GetIsolate();
   if (elements() == isolate->heap()->empty_fixed_array() ||