From 60c08a8bf25be88167c4467f92b7b878add24eb2 Mon Sep 17 00:00:00 2001 From: "verwaest@chromium.org" Date: Tue, 18 Feb 2014 12:19:32 +0000 Subject: [PATCH] Directly store the transition target on LookupResult in TransitionResult. BUG=chromium:343964 LOG=N R=jkummerow@chromium.org Review URL: https://codereview.chromium.org/170343003 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@19440 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/hydrogen-instructions.cc | 4 +- src/hydrogen.cc | 3 +- src/hydrogen.h | 3 +- src/ic.cc | 13 ++---- src/objects-inl.h | 8 +++- src/objects.h | 2 + src/property.cc | 5 ++- src/property.h | 52 ++++++++--------------- src/transitions-inl.h | 4 +- test/mjsunit/regress/regress-lookup-transition.js | 14 ++++++ 10 files changed, 52 insertions(+), 56 deletions(-) create mode 100644 test/mjsunit/regress/regress-lookup-transition.js diff --git a/src/hydrogen-instructions.cc b/src/hydrogen-instructions.cc index 3cfce32..5795385 100644 --- a/src/hydrogen-instructions.cc +++ b/src/hydrogen-instructions.cc @@ -4376,14 +4376,14 @@ HObjectAccess HObjectAccess::ForBackingStoreOffset(int offset, HObjectAccess HObjectAccess::ForField(Handle map, LookupResult* lookup, Handle name) { - ASSERT(lookup->IsField() || lookup->IsTransitionToField(*map)); + ASSERT(lookup->IsField() || lookup->IsTransitionToField()); int index; Representation representation; if (lookup->IsField()) { index = lookup->GetLocalFieldIndexFromMap(*map); representation = lookup->representation(); } else { - Map* transition = lookup->GetTransitionMapFromMap(*map); + Map* transition = lookup->GetTransitionTarget(); int descriptor = transition->LastAdded(); index = transition->instance_descriptors()->GetFieldIndex(descriptor) - map->inobject_properties(); diff --git a/src/hydrogen.cc b/src/hydrogen.cc index 2975758..b471faa 100644 --- a/src/hydrogen.cc +++ b/src/hydrogen.cc @@ -5555,8 +5555,7 @@ bool HOptimizedGraphBuilder::PropertyAccessInfo::CanAccessMonomorphic() { if (lookup_.IsPropertyCallbacks()) return true; Handle map = this->map(); map->LookupTransition(NULL, *name_, &lookup_); - if (lookup_.IsTransitionToField(*map) && map->unused_property_fields() > 0) { - transition_ = handle(lookup_.GetTransitionMapFromMap(*map)); + if (lookup_.IsTransitionToField() && map->unused_property_fields() > 0) { return true; } return false; diff --git a/src/hydrogen.h b/src/hydrogen.h index cf0b484..b9d53be 100644 --- a/src/hydrogen.h +++ b/src/hydrogen.h @@ -2408,7 +2408,7 @@ class HOptimizedGraphBuilder : public HGraphBuilder, public AstVisitor { Handle holder() { return holder_; } Handle accessor() { return accessor_; } Handle constant() { return constant_; } - Handle transition() { return transition_; } + Handle transition() { return handle(lookup_.GetTransitionTarget()); } HObjectAccess access() { return access_; } private: @@ -2435,7 +2435,6 @@ class HOptimizedGraphBuilder : public HGraphBuilder, public AstVisitor { Handle accessor_; Handle api_holder_; Handle constant_; - Handle transition_; HObjectAccess access_; }; diff --git a/src/ic.cc b/src/ic.cc index 517ed18..0ca70d3 100644 --- a/src/ic.cc +++ b/src/ic.cc @@ -1134,8 +1134,7 @@ static bool LookupForWrite(Handle receiver, // receiver when trying to fetch extra information from the transition. receiver->map()->LookupTransition(*holder, *name, lookup); if (!lookup->IsTransition()) return false; - PropertyDetails target_details = - lookup->GetTransitionDetails(receiver->map()); + 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 @@ -1146,7 +1145,7 @@ static bool LookupForWrite(Handle receiver, // transition target. ASSERT(!receiver->map()->is_deprecated()); if (!value->FitsRepresentation(target_details.representation())) { - Handle target(lookup->GetTransitionMapFromMap(receiver->map())); + Handle target(lookup->GetTransitionTarget()); Map::GeneralizeRepresentation( target, target->LastAdded(), value->OptimalRepresentation(), FORCE_FIELD); @@ -1319,12 +1318,8 @@ Handle StoreIC::CompileHandler(LookupResult* lookup, case TRANSITION: { // Explicitly pass in the receiver map since LookupForWrite may have // stored something else than the receiver in the holder. - Handle transition( - lookup->GetTransitionTarget(receiver->map()), isolate()); - int descriptor = transition->LastAdded(); - - DescriptorArray* target_descriptors = transition->instance_descriptors(); - PropertyDetails details = target_descriptors->GetDetails(descriptor); + Handle transition(lookup->GetTransitionTarget()); + PropertyDetails details = transition->GetLastDescriptorDetails(); if (details.type() == CALLBACKS || details.attributes() != NONE) break; diff --git a/src/objects-inl.h b/src/objects-inl.h index ea7deda..4f16806 100644 --- a/src/objects-inl.h +++ b/src/objects-inl.h @@ -2497,6 +2497,11 @@ int DescriptorArray::SearchWithCache(Name* name, Map* map) { } +PropertyDetails Map::GetLastDescriptorDetails() { + return instance_descriptors()->GetDetails(LastAdded()); +} + + void Map::LookupDescriptor(JSObject* holder, Name* name, LookupResult* result) { @@ -2514,7 +2519,8 @@ void Map::LookupTransition(JSObject* holder, TransitionArray* transition_array = transitions(); int number = transition_array->Search(name); if (number != TransitionArray::kNotFound) { - return result->TransitionResult(holder, number); + return result->TransitionResult( + holder, transition_array->GetTarget(number)); } } result->NotFound(); diff --git a/src/objects.h b/src/objects.h index 710d2ba..acd2c11 100644 --- a/src/objects.h +++ b/src/objects.h @@ -6088,6 +6088,8 @@ class Map: public HeapObject { Name* name, LookupResult* result); + inline PropertyDetails GetLastDescriptorDetails(); + // The size of transition arrays are limited so they do not end up in large // object space. Otherwise ClearNonLiveTransitions would leak memory while // applying in-place right trimming. diff --git a/src/property.cc b/src/property.cc index 83a6a36..2f72eec 100644 --- a/src/property.cc +++ b/src/property.cc @@ -35,6 +35,7 @@ void LookupResult::Iterate(ObjectVisitor* visitor) { LookupResult* current = this; // Could be NULL. while (current != NULL) { visitor->VisitPointer(BitCast(¤t->holder_)); + visitor->VisitPointer(BitCast(¤t->transition_)); current = current->next_; } } @@ -82,13 +83,13 @@ void LookupResult::Print(FILE* out) { case FIELD: PrintF(out, " -type = map transition\n"); PrintF(out, " -map:\n"); - GetTransitionMap()->Print(out); + GetTransitionTarget()->Print(out); PrintF(out, "\n"); return; case CONSTANT: PrintF(out, " -type = constant property transition\n"); PrintF(out, " -map:\n"); - GetTransitionMap()->Print(out); + GetTransitionTarget()->Print(out); PrintF(out, "\n"); return; case CALLBACKS: diff --git a/src/property.h b/src/property.h index 03dd66e..baa5a0f 100644 --- a/src/property.h +++ b/src/property.h @@ -184,6 +184,7 @@ class LookupResult BASE_EMBEDDED { next_(isolate->top_lookup_result()), lookup_type_(NOT_FOUND), holder_(NULL), + transition_(NULL), cacheable_(true), details_(NONE, NONEXISTENT, Representation::None()) { isolate->set_top_lookup_result(this); @@ -199,6 +200,7 @@ class LookupResult BASE_EMBEDDED { void DescriptorResult(JSObject* holder, PropertyDetails details, int number) { lookup_type_ = DESCRIPTOR_TYPE; holder_ = holder; + transition_ = NULL; details_ = details; number_ = number; } @@ -209,16 +211,18 @@ class LookupResult BASE_EMBEDDED { return value->FitsRepresentation(details_.representation()); } - void TransitionResult(JSObject* holder, int number) { + void TransitionResult(JSObject* holder, Map* target) { lookup_type_ = TRANSITION_TYPE; details_ = PropertyDetails(NONE, TRANSITION, Representation::None()); holder_ = holder; - number_ = number; + transition_ = target; + number_ = 0xAAAA; } void DictionaryResult(JSObject* holder, int entry) { lookup_type_ = DICTIONARY_TYPE; holder_ = holder; + transition_ = NULL; details_ = holder->property_dictionary()->DetailsAt(entry); number_ = entry; } @@ -226,6 +230,7 @@ class LookupResult BASE_EMBEDDED { void HandlerResult(JSProxy* proxy) { lookup_type_ = HANDLER_TYPE; holder_ = proxy; + transition_ = NULL; details_ = PropertyDetails(NONE, HANDLER, Representation::Tagged()); cacheable_ = false; } @@ -233,6 +238,7 @@ class LookupResult BASE_EMBEDDED { void InterceptorResult(JSObject* holder) { lookup_type_ = INTERCEPTOR_TYPE; holder_ = holder; + transition_ = NULL; details_ = PropertyDetails(NONE, INTERCEPTOR, Representation::Tagged()); } @@ -240,6 +246,7 @@ class LookupResult BASE_EMBEDDED { lookup_type_ = NOT_FOUND; details_ = PropertyDetails(NONE, NONEXISTENT, Representation::None()); holder_ = NULL; + transition_ = NULL; } JSObject* holder() const { @@ -248,7 +255,7 @@ class LookupResult BASE_EMBEDDED { } JSProxy* proxy() const { - ASSERT(IsFound()); + ASSERT(IsHandler()); return JSProxy::cast(holder_); } @@ -373,47 +380,21 @@ class LookupResult BASE_EMBEDDED { return NULL; } - Map* GetTransitionTarget(Map* map) const { - ASSERT(IsTransition()); - TransitionArray* transitions = map->transitions(); - return transitions->GetTarget(number_); - } - Map* GetTransitionTarget() const { - return GetTransitionTarget(holder()->map()); - } - - PropertyDetails GetTransitionDetails(Map* map) const { - ASSERT(IsTransition()); - TransitionArray* transitions = map->transitions(); - return transitions->GetTargetDetails(number_); + return transition_; } PropertyDetails GetTransitionDetails() const { - return GetTransitionDetails(holder()->map()); - } - - bool IsTransitionToField(Map* map) const { - return IsTransition() && GetTransitionDetails(map).type() == FIELD; - } - - bool IsTransitionToConstant(Map* map) const { - return IsTransition() && GetTransitionDetails(map).type() == CONSTANT; - } - - Map* GetTransitionMap() const { ASSERT(IsTransition()); - return Map::cast(GetValue()); + return transition_->GetLastDescriptorDetails(); } - Map* GetTransitionMapFromMap(Map* map) const { - ASSERT(IsTransition()); - return map->transitions()->GetTarget(number_); + bool IsTransitionToField() const { + return IsTransition() && GetTransitionDetails().type() == FIELD; } - int GetTransitionIndex() const { - ASSERT(IsTransition()); - return number_; + bool IsTransitionToConstant() const { + return IsTransition() && GetTransitionDetails().type() == CONSTANT; } int GetDescriptorIndex() const { @@ -501,6 +482,7 @@ class LookupResult BASE_EMBEDDED { } lookup_type_; JSReceiver* holder_; + Map* transition_; int number_; bool cacheable_; PropertyDetails details_; diff --git a/src/transitions-inl.h b/src/transitions-inl.h index 7d8608b..7895117 100644 --- a/src/transitions-inl.h +++ b/src/transitions-inl.h @@ -160,9 +160,7 @@ void TransitionArray::SetTarget(int transition_number, Map* value) { PropertyDetails TransitionArray::GetTargetDetails(int transition_number) { Map* map = GetTarget(transition_number); - DescriptorArray* descriptors = map->instance_descriptors(); - int descriptor = map->LastAdded(); - return descriptors->GetDetails(descriptor); + return map->GetLastDescriptorDetails(); } diff --git a/test/mjsunit/regress/regress-lookup-transition.js b/test/mjsunit/regress/regress-lookup-transition.js new file mode 100644 index 0000000..9b32939 --- /dev/null +++ b/test/mjsunit/regress/regress-lookup-transition.js @@ -0,0 +1,14 @@ +// Copyright 2014 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: --harmony-proxies --expose-gc + +var proxy = Proxy.create({ getPropertyDescriptor:function(key) { + gc(); +}}); + +function f() { this.x = 23; } +f.prototype = proxy; +new f(); +new f(); -- 2.7.4