From: jkummerow Date: Mon, 27 Apr 2015 12:59:55 +0000 (-0700) Subject: Reland "Lazily register prototype users..." X-Git-Tag: upstream/4.7.83~2964 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=f6187fb3b52e518ccefd5de1bd1d4aca12bacba2;p=platform%2Fupstream%2Fv8.git 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 968715c653b6337252a05a0224a7a93fab3b0866. Original review: https://codereview.chromium.org/1104813004/ Review URL: https://codereview.chromium.org/1110513002 Cr-Commit-Position: refs/heads/master@{#28076} --- diff --git a/src/objects.cc b/src/objects.cc index 828a0a6..99bf653 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -1911,6 +1911,22 @@ void JSObject::MigrateToMap(Handle object, Handle new_map, // prototype chains involving it. InvalidatePrototypeChains(object->map()); Handle 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 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 object, Handle 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 parent, Handle child, #endif } else { TransitionArray::Insert(parent, name, child, flag); - if (child->prototype()->IsJSObject()) { - Handle 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 array, int index, // static Handle WeakFixedArray::Add( Handle maybe_array, Handle value, - SearchForDuplicates search_for_duplicates) { + SearchForDuplicates search_for_duplicates, bool* was_present) { Handle array = (maybe_array.is_null() || !maybe_array->IsWeakFixedArray()) ? Allocate(value->GetIsolate(), 1, Handle::null()) : Handle::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::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->IsEmptySlot((i))) { - WeakFixedArray::Set(array, i, value); - return array; - } - if (FLAG_trace_weak_arrays) { - PrintF("[WeakFixedArray: searching for free slot]\n"); + if (array->Length() > 0) { + for (int i = first_index;;) { + if (array->IsEmptySlot((i))) { + 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; } - 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 new_array = Allocate(array->GetIsolate(), new_length, array); if (FLAG_trace_weak_arrays) { @@ -8317,7 +8341,8 @@ void WeakFixedArray::Compact() { } -void WeakFixedArray::Remove(Handle value) { +bool WeakFixedArray::Remove(Handle 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 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 object) { // static -void JSObject::RegisterPrototypeUser(Handle prototype, - Handle user) { +void JSObject::LazyRegisterPrototypeUser(Handle user, Isolate* isolate) { DCHECK(FLAG_track_prototype_users); - Isolate* isolate = prototype->GetIsolate(); - if (prototype->IsJSGlobalProxy()) { - PrototypeIterator iter(isolate, prototype); - prototype = Handle::cast(PrototypeIterator::GetCurrent(iter)); - } - Handle proto_info; - Object* maybe_proto_info = prototype->map()->prototype_info(); - if (maybe_proto_info->IsPrototypeInfo()) { - proto_info = handle(PrototypeInfo::cast(maybe_proto_info), isolate); - } else { - proto_info = isolate->factory()->NewPrototypeInfo(); - prototype->map()->set_prototype_info(*proto_info); + // Contract: In line with InvalidatePrototypeChains()'s requirements, + // leaf maps don't need to register as users, only prototypes do. + DCHECK(user->is_prototype_map()); + + Handle current_user = user; + for (PrototypeIterator iter(user); !iter.IsAtEnd(); iter.Advance()) { + Handle maybe_proto = PrototypeIterator::GetCurrent(iter); + if (maybe_proto->IsJSGlobalProxy()) continue; + // Proxies on the prototype chain are not supported. + if (maybe_proto->IsJSProxy()) return; + Handle proto = Handle::cast(maybe_proto); + bool just_registered = + RegisterPrototypeUserIfNotRegistered(proto, current_user, isolate); + // Walk up the prototype chain as far as links haven't been registered yet. + if (!just_registered) break; + current_user = handle(proto->map(), isolate); } +} + + +// Returns true if the user was not yet registered. +// static +bool JSObject::RegisterPrototypeUserIfNotRegistered(Handle prototype, + Handle user, + Isolate* isolate) { + Handle proto_info = + Map::GetOrCreatePrototypeInfo(prototype, isolate); Handle maybe_registry(proto_info->prototype_users(), isolate); - Handle new_array = WeakFixedArray::Add(maybe_registry, user); + bool was_present = false; + Handle 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) { - PrintF("Registering %p as a user of prototype %p.\n", - reinterpret_cast(*user), reinterpret_cast(*prototype)); + if (FLAG_trace_prototype_users && !was_present) { + PrintF("Registering %p as a user of prototype %p (map=%p).\n", + reinterpret_cast(*user), reinterpret_cast(*prototype), + reinterpret_cast(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 -void JSObject::UnregisterPrototypeUser(Handle prototype, +bool JSObject::UnregisterPrototypeUser(Handle prototype, Handle user) { Isolate* isolate = prototype->GetIsolate(); if (prototype->IsJSGlobalProxy()) { @@ -10100,21 +10145,26 @@ void JSObject::UnregisterPrototypeUser(Handle 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 proto_info(PrototypeInfo::cast(maybe_proto_info), isolate); Object* maybe_registry = proto_info->prototype_users(); - if (!maybe_registry->IsWeakFixedArray()) return; - WeakFixedArray::cast(maybe_registry)->Remove(user); - if (FLAG_trace_prototype_users) { + if (!maybe_registry->IsWeakFixedArray()) return false; + bool result = WeakFixedArray::cast(maybe_registry)->Remove(user); + if (FLAG_trace_prototype_users && result) { PrintF("Unregistering %p as a user of prototype %p.\n", reinterpret_cast(*user), reinterpret_cast(*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(map)); + } Object* maybe_proto_info = map->prototype_info(); if (!maybe_proto_info->IsPrototypeInfo()) return; PrototypeInfo* proto_info = PrototypeInfo::cast(maybe_proto_info); @@ -10154,6 +10204,19 @@ void JSObject::InvalidatePrototypeChains(Map* map) { // static +Handle Map::GetOrCreatePrototypeInfo(Handle 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 proto_info = isolate->factory()->NewPrototypeInfo(); + prototype->map()->set_prototype_info(*proto_info); + return proto_info; +} + + +// static Handle Map::GetOrCreatePrototypeChainValidityCell(Handle map, Isolate* isolate) { Handle maybe_prototype(map->prototype(), isolate); @@ -10163,14 +10226,18 @@ Handle Map::GetOrCreatePrototypeChainValidityCell(Handle map, PrototypeIterator iter(isolate, prototype); prototype = Handle::cast(PrototypeIterator::GetCurrent(iter)); } - Handle proto_info( - PrototypeInfo::cast(prototype->map()->prototype_info()), isolate); + // Ensure the prototype is registered with its own prototypes so its cell + // will be invalidated when necessary. + JSObject::LazyRegisterPrototypeUser(handle(prototype->map(), isolate), + isolate); + Handle 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::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 Map::GetOrCreatePrototypeChainValidityCell(Handle map, } +// static void Map::SetPrototype(Handle map, Handle prototype, PrototypeOptimizationMode proto_mode) { - if (map->prototype()->IsJSObject() && FLAG_track_prototype_users) { - Handle old_prototype(JSObject::cast(map->prototype())); - JSObject::UnregisterPrototypeUser(old_prototype, map); - } if (prototype->IsJSObject()) { Handle prototype_jsobj = Handle::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, Handle prototype, } -// static -bool Map::ShouldRegisterAsPrototypeUser(Handle map, - Handle 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 CacheInitialJSArrayMaps( Handle native_context, Handle initial_map) { // Replace all of the cached initial array maps in the native context with diff --git a/src/objects.h b/src/objects.h index 8f8a353..8b0477a 100644 --- a/src/objects.h +++ b/src/objects.h @@ -1841,9 +1841,11 @@ class JSObject: public JSReceiver { static void OptimizeAsPrototype(Handle object, PrototypeOptimizationMode mode); static void ReoptimizeIfPrototype(Handle object); - static void RegisterPrototypeUser(Handle prototype, - Handle user); - static void UnregisterPrototypeUser(Handle prototype, + static void LazyRegisterPrototypeUser(Handle user, Isolate* isolate); + static bool RegisterPrototypeUserIfNotRegistered(Handle prototype, + Handle user, + Isolate* isolate); + static bool UnregisterPrototypeUser(Handle prototype, Handle 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 Add( Handle maybe_array, Handle value, - SearchForDuplicates search_for_duplicates = kAlwaysAdd); + SearchForDuplicates search_for_duplicates = kAlwaysAdd, + bool* was_present = NULL); - void Remove(Handle value); + // Returns true if an entry was found and removed. + bool Remove(Handle 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 GetOrCreatePrototypeInfo( + Handle 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, Handle prototype, PrototypeOptimizationMode proto_mode = FAST_PROTOTYPE); - static bool ShouldRegisterAsPrototypeUser(Handle map, - Handle 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 diff --git a/test/cctest/test-heap.cc b/test/cctest/test-heap.cc index 21fa4ce..b8d7676 100644 --- a/test/cctest/test-heap.cc +++ b/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 number = CcTest::i_isolate()->factory()->NewHeapNumber(1); + Handle array = WeakFixedArray::Add(Handle(), number); + array->Remove(number); + array->Compact(); + WeakFixedArray::Add(array, number); +} diff --git a/test/mjsunit/prototype-changes.js b/test/mjsunit/prototype-changes.js new file mode 100644 index 0000000..e7fcc7e --- /dev/null +++ b/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");