Correct handling of arrays with callbacks in the prototype chain.

src/objects.cc

 // Copyright 2013 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 2905 matching lines @@
   bool has_pending_exception;
   Handle<Object> argv[] = { value };
   Execution::Call(
       isolate, setter, object, ARRAY_SIZE(argv), argv, &has_pending_exception);
   // Check for pending exception and return the result.
   if (has_pending_exception) return Handle<Object>();
   return value;
 }
 
 
+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,
     bool* found,
     StrictModeFlag strict_mode) {
   Heap* heap = GetHeap();
   for (Object* pt = GetPrototype();
        pt != heap->null_value();
        pt = pt->GetPrototype(GetIsolate())) {
     if (pt->IsJSProxy()) {
@@ skipping 3261 matching lines @@
   }
   return true;
 }
 
 
 void JSObject::SetElementCallback(Handle<JSObject> object,
                                   uint32_t index,
                                   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,
                                            details);
   dictionary->set_requires_slow_elements();
 
   // Update the dictionary backing store on the object.
   if (object->elements()->map() == heap->non_strict_arguments_elements_map()) {
     // Also delete any parameter alias.
     //
     // TODO(kmillikin): when deleting the last parameter alias we could
     // switch to a direct backing store without the parameter map.  This
     // would allow GC of the context.
     FixedArray* parameter_map = FixedArray::cast(object->elements());
     if (index < static_cast<uint32_t>(parameter_map->length()) - 2) {
       parameter_map->set(index + 2, heap->the_hole_value());
     }
     parameter_map->set(1, *dictionary);
   } 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();
 }
 
 
 void JSObject::SetPropertyCallback(Handle<JSObject> object,
                                    Handle<Name> name,
                                    Handle<Object> structure,
                                    PropertyAttributes attributes) {
   // Normalize object to make this operation simple.
   NormalizeProperties(object, CLEAR_INOBJECT_PROPERTIES, 0);
 
@@ skipping 4309 matching lines @@
     if (--n == 0) {
       info->set_target_cell(replace_with);
       return;
     }
   }
   UNREACHABLE();
 }
 
 
 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) |
              RelocInfo::ModeMask(RelocInfo::CODE_TARGET_CONTEXT);
   for (RelocIterator it(this, mask); !it.done(); it.next()) {
     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());
+      }
     }
   }
 }
 
 
 void Code::ClearTypeFeedbackCells(Heap* heap) {
   if (kind() != FUNCTION) return;
   Object* raw_info = type_feedback_info();
   if (raw_info->IsTypeFeedbackInfo()) {
     TypeFeedbackCells* type_feedback_cells =
@@ skipping 2149 matching lines @@
 MaybeObject* JSObject::TransitionElementsKind(ElementsKind to_kind) {
   ASSERT(!map()->is_observed());
   ElementsKind from_kind = map()->elements_kind();
 
   if (IsFastHoleyElementsKind(from_kind)) {
     to_kind = GetHoleyElementsKind(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() ||
       (IsFastSmiOrObjectElementsKind(from_kind) &&
        IsFastSmiOrObjectElementsKind(to_kind)) ||
       (from_kind == FAST_DOUBLE_ELEMENTS &&
        to_kind == FAST_HOLEY_DOUBLE_ELEMENTS)) {
     ASSERT(from_kind != TERMINAL_FAST_ELEMENTS_KIND);
     // No change is needed to the elements() buffer, the transition
     // only requires a map change.
@@ skipping 3621 matching lines @@
 #define ERROR_MESSAGES_TEXTS(C, T) T,
   static const char* error_messages_[] = {
       ERROR_MESSAGES_LIST(ERROR_MESSAGES_TEXTS)
   };
 #undef ERROR_MESSAGES_TEXTS
   return error_messages_[reason];
 }
 
 
 } }  // namespace v8::internal