Reland "Lazily register prototype users..."

...when handing out validity cells to handlers; because invalidating said cells is
the only time we'll need the user registrations.
Along the way, fix a corner case in WeakFixedArray, which can now be empty after
the recently introduced compaction support.

This reverts commit 968715c653.

Original review: https://codereview.chromium.org/1104813004/

Review URL: https://codereview.chromium.org/1110513002

Cr-Commit-Position: refs/heads/master@{#28076}

src/objects.cc

@@ -1911,6 +1911,22 @@ void JSObject::MigrateToMap(Handle<JSObject> object, Handle<Map> new_map,
   // prototype chains involving it.
   InvalidatePrototypeChains(object->map());
   Handle<Map> old_map(object->map());
+
+  // If the map was registered with its prototype before, ensure that it
+  // registers with its new prototype now. This preserves the invariant that
+  // when a map on a prototype chain is registered with its prototype, then
+  // all prototypes further up the chain are also registered with their
+  // respective prototypes.
+  Object* maybe_old_prototype = old_map->prototype();
+  if (maybe_old_prototype->IsJSObject()) {
+    Handle<JSObject> old_prototype(JSObject::cast(maybe_old_prototype));
+    bool was_registered =
+        JSObject::UnregisterPrototypeUser(old_prototype, old_map);
+    if (was_registered) {
+      JSObject::LazyRegisterPrototypeUser(new_map, new_map->GetIsolate());
+    }
+  }
+
   if (object->HasFastProperties()) {
     if (!new_map->is_dictionary_map()) {
       MigrateFastToFast(object, new_map);
@@ -1936,7 +1952,15 @@ void JSObject::MigrateToMap(Handle<JSObject> object, Handle<Map> new_map,
     // Slow-to-slow migration is trivial.
     object->set_map(*new_map);
   }
-  if (old_map->is_prototype_map()) {
+
+  // Careful: Don't allocate here!
+  // For some callers of this method, |object| might be in an inconsistent
+  // state now: the new map might have a new elements_kind, but the object's
+  // elements pointer hasn't been updated yet. Callers will fix this, but in
+  // the meantime, (indirectly) calling JSObjectVerify() must be avoided.
+  DisallowHeapAllocation no_object_verification;
+
+  if (old_map->is_prototype_map() && FLAG_track_prototype_users) {
     DCHECK(new_map->is_prototype_map());
     DCHECK(object->map() == *new_map);
     new_map->set_prototype_info(old_map->prototype_info());
@@ -7088,12 +7112,6 @@ void Map::ConnectTransition(Handle<Map> parent, Handle<Map> child,
 #endif
   } else {
     TransitionArray::Insert(parent, name, child, flag);
-    if (child->prototype()->IsJSObject()) {
-      Handle<JSObject> proto(JSObject::cast(child->prototype()));
-      if (!ShouldRegisterAsPrototypeUser(child, proto)) {
-        JSObject::UnregisterPrototypeUser(proto, child);
-      }
-    }
 #if TRACE_MAPS
     Map::TraceTransition("Transition", *parent, *child, *name);
 #endif
@@ -8258,15 +8276,18 @@ void WeakFixedArray::Set(Handle<WeakFixedArray> array, int index,
 // static
 Handle<WeakFixedArray> WeakFixedArray::Add(
     Handle<Object> maybe_array, Handle<HeapObject> value,
-    SearchForDuplicates search_for_duplicates) {
+    SearchForDuplicates search_for_duplicates, bool* was_present) {
   Handle<WeakFixedArray> array =
       (maybe_array.is_null() || !maybe_array->IsWeakFixedArray())
           ? Allocate(value->GetIsolate(), 1, Handle<WeakFixedArray>::null())
           : Handle<WeakFixedArray>::cast(maybe_array);
-
+  if (was_present != NULL) *was_present = false;
   if (search_for_duplicates == kAddIfNotFound) {
     for (int i = 0; i < array->Length(); ++i) {
-      if (array->Get(i) == *value) return array;
+      if (array->Get(i) == *value) {
+        if (was_present != NULL) *was_present = true;
+        return array;
+      }
     }
 #if 0  // Enable this if you want to check your search_for_duplicates flags.
   } else {
@@ -8279,20 +8300,23 @@ Handle<WeakFixedArray> WeakFixedArray::Add(
   // Try to store the new entry if there's room. Optimize for consecutive
   // accesses.
   int first_index = array->last_used_index();
-  for (int i = first_index;;) {
+  if (array->Length() > 0) {
-    if (array->IsEmptySlot((i))) {
+    for (int i = first_index;;) {
-      WeakFixedArray::Set(array, i, value);
+      if (array->IsEmptySlot((i))) {
-      return array;
+        WeakFixedArray::Set(array, i, value);
+        return array;
+      }
+      if (FLAG_trace_weak_arrays) {
+        PrintF("[WeakFixedArray: searching for free slot]\n");
+      }
+      i = (i + 1) % array->Length();
+      if (i == first_index) break;
     }
-    if (FLAG_trace_weak_arrays) {
-      PrintF("[WeakFixedArray: searching for free slot]\n");
-    }
-    i = (i + 1) % array->Length();
-    if (i == first_index) break;
   }
 
   // No usable slot found, grow the array.
-  int new_length = array->Length() + (array->Length() >> 1) + 4;
+  int new_length =
+      array->Length() == 0 ? 1 : array->Length() + (array->Length() >> 1) + 4;
   Handle<WeakFixedArray> new_array =
       Allocate(array->GetIsolate(), new_length, array);
   if (FLAG_trace_weak_arrays) {
@@ -8317,7 +8341,8 @@ void WeakFixedArray::Compact() {
 }
 
 
-void WeakFixedArray::Remove(Handle<HeapObject> value) {
+bool WeakFixedArray::Remove(Handle<HeapObject> value) {
+  if (Length() == 0) return false;
   // Optimize for the most recently added element to be removed again.
   int first_index = last_used_index();
   for (int i = first_index;;) {
@@ -8325,11 +8350,12 @@ void WeakFixedArray::Remove(Handle<HeapObject> value) {
       clear(i);
       // Users of WeakFixedArray should make sure that there are no duplicates,
       // they can use Add(..., kAddIfNotFound) if necessary.
-      return;
+      return true;
     }
     i = (i + 1) % Length();
-    if (i == first_index) break;
+    if (i == first_index) return false;
   }
+  UNREACHABLE();
 }
 
 
@@ -10062,36 +10088,55 @@ void JSObject::ReoptimizeIfPrototype(Handle<JSObject> object) {
 
 
 // static
-void JSObject::RegisterPrototypeUser(Handle<JSObject> prototype,
+void JSObject::LazyRegisterPrototypeUser(Handle<Map> user, Isolate* isolate) {
-                                     Handle<HeapObject> user) {
   DCHECK(FLAG_track_prototype_users);
-  Isolate* isolate = prototype->GetIsolate();
+  // Contract: In line with InvalidatePrototypeChains()'s requirements,
-  if (prototype->IsJSGlobalProxy()) {
+  // leaf maps don't need to register as users, only prototypes do.
-    PrototypeIterator iter(isolate, prototype);
+  DCHECK(user->is_prototype_map());
-    prototype = Handle<JSObject>::cast(PrototypeIterator::GetCurrent(iter));
+
-  }
+  Handle<Map> current_user = user;
-  Handle<PrototypeInfo> proto_info;
+  for (PrototypeIterator iter(user); !iter.IsAtEnd(); iter.Advance()) {
-  Object* maybe_proto_info = prototype->map()->prototype_info();
+    Handle<Object> maybe_proto = PrototypeIterator::GetCurrent(iter);
-  if (maybe_proto_info->IsPrototypeInfo()) {
+    if (maybe_proto->IsJSGlobalProxy()) continue;
-    proto_info = handle(PrototypeInfo::cast(maybe_proto_info), isolate);
+    // Proxies on the prototype chain are not supported.
-  } else {
+    if (maybe_proto->IsJSProxy()) return;
-    proto_info = isolate->factory()->NewPrototypeInfo();
+    Handle<JSObject> proto = Handle<JSObject>::cast(maybe_proto);
-    prototype->map()->set_prototype_info(*proto_info);
+    bool just_registered =
-  }
+        RegisterPrototypeUserIfNotRegistered(proto, current_user, isolate);
-  Handle<Object> maybe_registry(proto_info->prototype_users(), isolate);
+    // Walk up the prototype chain as far as links haven't been registered yet.
-  Handle<WeakFixedArray> new_array = WeakFixedArray::Add(maybe_registry, user);
+    if (!just_registered) break;
-  if (!maybe_registry.is_identical_to(new_array)) {
+    current_user = handle(proto->map(), isolate);
-    proto_info->set_prototype_users(*new_array);
-  }
-  if (FLAG_trace_prototype_users) {
-    PrintF("Registering %p as a user of prototype %p.\n",
-           reinterpret_cast<void*>(*user), reinterpret_cast<void*>(*prototype));
   }
 }
 
 
+// Returns true if the user was not yet registered.
 // static
-void JSObject::UnregisterPrototypeUser(Handle<JSObject> prototype,
+bool JSObject::RegisterPrototypeUserIfNotRegistered(Handle<JSObject> prototype,
+                                                    Handle<HeapObject> user,
+                                                    Isolate* isolate) {
+  Handle<PrototypeInfo> proto_info =
+      Map::GetOrCreatePrototypeInfo(prototype, isolate);
+  Handle<Object> maybe_registry(proto_info->prototype_users(), isolate);
+  bool was_present = false;
+  Handle<WeakFixedArray> new_array = WeakFixedArray::Add(
+      maybe_registry, user, WeakFixedArray::kAddIfNotFound, &was_present);
+  if (!maybe_registry.is_identical_to(new_array)) {
+    proto_info->set_prototype_users(*new_array);
+  }
+  if (FLAG_trace_prototype_users && !was_present) {
+    PrintF("Registering %p as a user of prototype %p (map=%p).\n",
+           reinterpret_cast<void*>(*user), reinterpret_cast<void*>(*prototype),
+           reinterpret_cast<void*>(prototype->map()));
+  }
+  return !was_present;
+}
+
+
+// Can be called regardless of whether |user| was actually registered with
+// |prototype|. Returns true when there was a registration.
+// static
+bool JSObject::UnregisterPrototypeUser(Handle<JSObject> prototype,
                                        Handle<HeapObject> user) {
   Isolate* isolate = prototype->GetIsolate();
   if (prototype->IsJSGlobalProxy()) {
@@ -10100,21 +10145,26 @@ void JSObject::UnregisterPrototypeUser(Handle<JSObject> prototype,
   }
   DCHECK(prototype->map()->is_prototype_map());
   Object* maybe_proto_info = prototype->map()->prototype_info();
-  if (!maybe_proto_info->IsPrototypeInfo()) return;
+  if (!maybe_proto_info->IsPrototypeInfo()) return false;
   Handle<PrototypeInfo> proto_info(PrototypeInfo::cast(maybe_proto_info),
                                    isolate);
   Object* maybe_registry = proto_info->prototype_users();
-  if (!maybe_registry->IsWeakFixedArray()) return;
+  if (!maybe_registry->IsWeakFixedArray()) return false;
-  WeakFixedArray::cast(maybe_registry)->Remove(user);
+  bool result = WeakFixedArray::cast(maybe_registry)->Remove(user);
-  if (FLAG_trace_prototype_users) {
+  if (FLAG_trace_prototype_users && result) {
     PrintF("Unregistering %p as a user of prototype %p.\n",
            reinterpret_cast<void*>(*user), reinterpret_cast<void*>(*prototype));
   }
+  return result;
 }
 
 
 static void InvalidatePrototypeChainsInternal(Map* map) {
   if (!map->is_prototype_map()) return;
+  if (FLAG_trace_prototype_users) {
+    PrintF("Invalidating prototype map %p 's cell\n",
+           reinterpret_cast<void*>(map));
+  }
   Object* maybe_proto_info = map->prototype_info();
   if (!maybe_proto_info->IsPrototypeInfo()) return;
   PrototypeInfo* proto_info = PrototypeInfo::cast(maybe_proto_info);
@@ -10153,6 +10203,19 @@ void JSObject::InvalidatePrototypeChains(Map* map) {
 }
 
 
+// static
+Handle<PrototypeInfo> Map::GetOrCreatePrototypeInfo(Handle<JSObject> prototype,
+                                                    Isolate* isolate) {
+  Object* maybe_proto_info = prototype->map()->prototype_info();
+  if (maybe_proto_info->IsPrototypeInfo()) {
+    return handle(PrototypeInfo::cast(maybe_proto_info), isolate);
+  }
+  Handle<PrototypeInfo> proto_info = isolate->factory()->NewPrototypeInfo();
+  prototype->map()->set_prototype_info(*proto_info);
+  return proto_info;
+}
+
+
 // static
 Handle<Cell> Map::GetOrCreatePrototypeChainValidityCell(Handle<Map> map,
                                                         Isolate* isolate) {
@@ -10163,14 +10226,18 @@ Handle<Cell> Map::GetOrCreatePrototypeChainValidityCell(Handle<Map> map,
     PrototypeIterator iter(isolate, prototype);
     prototype = Handle<JSObject>::cast(PrototypeIterator::GetCurrent(iter));
   }
-  Handle<PrototypeInfo> proto_info(
+  // Ensure the prototype is registered with its own prototypes so its cell
-      PrototypeInfo::cast(prototype->map()->prototype_info()), isolate);
+  // will be invalidated when necessary.
+  JSObject::LazyRegisterPrototypeUser(handle(prototype->map(), isolate),
+                                      isolate);
+  Handle<PrototypeInfo> proto_info =
+      GetOrCreatePrototypeInfo(prototype, isolate);
   Object* maybe_cell = proto_info->validity_cell();
   // Return existing cell if it's still valid.
   if (maybe_cell->IsCell()) {
     Handle<Cell> cell(Cell::cast(maybe_cell), isolate);
     if (cell->value() == Smi::FromInt(Map::kPrototypeChainValid)) {
-      return handle(Cell::cast(maybe_cell), isolate);
+      return cell;
     }
   }
   // Otherwise create a new cell.
@@ -10181,18 +10248,12 @@ Handle<Cell> Map::GetOrCreatePrototypeChainValidityCell(Handle<Map> map,
 }
 
 
+// static
 void Map::SetPrototype(Handle<Map> map, Handle<Object> prototype,
                        PrototypeOptimizationMode proto_mode) {
-  if (map->prototype()->IsJSObject() && FLAG_track_prototype_users) {
-    Handle<JSObject> old_prototype(JSObject::cast(map->prototype()));
-    JSObject::UnregisterPrototypeUser(old_prototype, map);
-  }
   if (prototype->IsJSObject()) {
     Handle<JSObject> prototype_jsobj = Handle<JSObject>::cast(prototype);
     JSObject::OptimizeAsPrototype(prototype_jsobj, proto_mode);
-    if (ShouldRegisterAsPrototypeUser(map, prototype_jsobj)) {
-      JSObject::RegisterPrototypeUser(prototype_jsobj, map);
-    }
   }
   WriteBarrierMode wb_mode =
       prototype->IsNull() ? SKIP_WRITE_BARRIER : UPDATE_WRITE_BARRIER;
@@ -10200,18 +10261,6 @@ void Map::SetPrototype(Handle<Map> map, Handle<Object> prototype,
 }
 
 
-// static
-bool Map::ShouldRegisterAsPrototypeUser(Handle<Map> map,
-                                        Handle<JSObject> prototype) {
-  if (!FLAG_track_prototype_users) return false;
-  if (map->is_prototype_map()) return true;
-  Object* back = map->GetBackPointer();
-  if (!back->IsMap()) return true;
-  if (Map::cast(back)->prototype() != *prototype) return true;
-  return false;
-}
-
-
 Handle<Object> CacheInitialJSArrayMaps(
     Handle<Context> native_context, Handle<Map> initial_map) {
   // Replace all of the cached initial array maps in the native context with

src/objects.h

@@ -1841,9 +1841,11 @@ class JSObject: public JSReceiver {
   static void OptimizeAsPrototype(Handle<JSObject> object,
                                   PrototypeOptimizationMode mode);
   static void ReoptimizeIfPrototype(Handle<JSObject> object);
-  static void RegisterPrototypeUser(Handle<JSObject> prototype,
+  static void LazyRegisterPrototypeUser(Handle<Map> user, Isolate* isolate);
-                                    Handle<HeapObject> user);
+  static bool RegisterPrototypeUserIfNotRegistered(Handle<JSObject> prototype,
-  static void UnregisterPrototypeUser(Handle<JSObject> prototype,
+                                                   Handle<HeapObject> user,
+                                                   Isolate* isolate);
+  static bool UnregisterPrototypeUser(Handle<JSObject> prototype,
                                       Handle<HeapObject> user);
   static void InvalidatePrototypeChains(Map* map);
 
@@ -2621,9 +2623,11 @@ class WeakFixedArray : public FixedArray {
   // If |maybe_array| is not a WeakFixedArray, a fresh one will be allocated.
   static Handle<WeakFixedArray> Add(
       Handle<Object> maybe_array, Handle<HeapObject> value,
-      SearchForDuplicates search_for_duplicates = kAlwaysAdd);
+      SearchForDuplicates search_for_duplicates = kAlwaysAdd,
+      bool* was_present = NULL);
 
-  void Remove(Handle<HeapObject> value);
+  // Returns true if an entry was found and removed.
+  bool Remove(Handle<HeapObject> value);
 
   void Compact();
 
@@ -5844,6 +5848,9 @@ class DependentCode: public FixedArray {
 };
 
 
+class PrototypeInfo;
+
+
 // All heap objects have a Map that describes their structure.
 //  A Map contains information about:
 //  - Size information about the object
@@ -6051,6 +6058,10 @@ class Map: public HeapObject {
   // [prototype_info]: Per-prototype metadata. Aliased with transitions
   // (which prototype maps don't have).
   DECL_ACCESSORS(prototype_info, Object)
+  // PrototypeInfo is created lazily using this helper (which installs it on
+  // the given prototype's map).
+  static Handle<PrototypeInfo> GetOrCreatePrototypeInfo(
+      Handle<JSObject> prototype, Isolate* isolate);
 
   // [prototype chain validity cell]: Associated with a prototype object,
   // stored in that object's map's PrototypeInfo, indicates that prototype
@@ -6123,9 +6134,6 @@ class Map: public HeapObject {
   static void SetPrototype(
       Handle<Map> map, Handle<Object> prototype,
       PrototypeOptimizationMode proto_mode = FAST_PROTOTYPE);
-  static bool ShouldRegisterAsPrototypeUser(Handle<Map> map,
-                                            Handle<JSObject> prototype);
-  bool CanUseOptimizationsBasedOnPrototypeRegistry();
 
   // [constructor]: points back to the function responsible for this map.
   // The field overlaps with the back pointer. All maps in a transition tree

test/cctest/test-heap.cc

@@ -5367,3 +5367,15 @@ TEST(Regress472513) {
   TestRightTrimFixedTypedArray(i::kExternalUint16Array, 8 - 1, 3);
   TestRightTrimFixedTypedArray(i::kExternalUint32Array, 4, 3);
 }
+
+
+TEST(WeakFixedArray) {
+  CcTest::InitializeVM();
+  v8::HandleScope scope(CcTest::isolate());
+
+  Handle<HeapNumber> number = CcTest::i_isolate()->factory()->NewHeapNumber(1);
+  Handle<WeakFixedArray> array = WeakFixedArray::Add(Handle<Object>(), number);
+  array->Remove(number);
+  array->Compact();
+  WeakFixedArray::Add(array, number);
+}

test/mjsunit/prototype-changes.js

@@ -0,0 +1,56 @@
+// Copyright 2015 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --allow-natives-syntax
+
+function A() {
+  this.a = "a";
+}
+var a = new A();
+
+function B() {
+  this.b = "b";
+}
+B.prototype = a;
+
+function C() {
+  this.c = "c";
+}
+C.prototype = new B();
+
+var c = new C();
+
+function f(expected) {
+  var result = c.z;
+  assertEquals(expected, result);
+}
+f(undefined);
+f(undefined);
+%OptimizeFunctionOnNextCall(f);
+f(undefined);
+a.z = "z";
+f("z");
+f("z");
+
+// Test updating .__proto__ pointers.
+var p1 = {foo: 1.5};
+var p2 = {}; p2.__proto__ = p1;
+var p3 = {}; p3.__proto__ = p2;
+var o = {}; o.__proto__ = p3;
+
+for (var i = 0; i < 2; i++) o.foo;  // Force registration.
+
+var p1a = {foo: 1.7};
+p2.__proto__ = p1a;
+
+function g(o, expected) {
+  var result = o.foo;
+  assertEquals(expected, result);
+}
+
+g(o, 1.7);
+g(o, 1.7);
+g(o, 1.7);
+Object.defineProperty(p1a, "foo", {get: function() { return "foo"}});
+g(o, "foo");