Revision: 23210
Author: [email protected]
Date: Tue Aug 19 17:02:04 2014 UTC
Log: Use LookupIterator to transition to accessors
BUG=
[email protected]
Review URL: https://codereview.chromium.org/490533002
http://code.google.com/p/v8/source/detail?r=23210
Added:
/branches/bleeding_edge/test/mjsunit/deopt-global-accessor.js
Modified:
/branches/bleeding_edge/src/lookup-inl.h
/branches/bleeding_edge/src/lookup.cc
/branches/bleeding_edge/src/lookup.h
/branches/bleeding_edge/src/objects.cc
/branches/bleeding_edge/src/objects.h
/branches/bleeding_edge/src/runtime.cc
/branches/bleeding_edge/src/stub-cache.cc
/branches/bleeding_edge/src/stub-cache.h
/branches/bleeding_edge/src/v8natives.js
=======================================
--- /dev/null
+++ /branches/bleeding_edge/test/mjsunit/deopt-global-accessor.js Tue Aug
19 17:02:04 2014 UTC
@@ -0,0 +1,23 @@
+// 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: --allow-natives-syntax
+
+x = 1;
+x = 2;
+x = 3;
+
+function f() {
+ return x;
+}
+
+f();
+f();
+f();
+%OptimizeFunctionOnNextCall(f);
+f();
+
+Object.defineProperty(this, "x", {get:function() { return 100; }});
+
+assertEquals(100, f());
=======================================
--- /branches/bleeding_edge/src/lookup-inl.h Tue Aug 5 09:32:55 2014 UTC
+++ /branches/bleeding_edge/src/lookup-inl.h Tue Aug 19 17:02:04 2014 UTC
@@ -29,6 +29,7 @@
LookupIterator::State LookupIterator::LookupInHolder(Map* map) {
+ STATIC_ASSERT(INTERCEPTOR == BEFORE_PROPERTY);
DisallowHeapAllocation no_gc;
switch (state_) {
case NOT_FOUND:
=======================================
--- /branches/bleeding_edge/src/lookup.cc Mon Aug 18 14:59:04 2014 UTC
+++ /branches/bleeding_edge/src/lookup.cc Tue Aug 19 17:02:04 2014 UTC
@@ -5,6 +5,7 @@
#include "src/v8.h"
#include "src/bootstrapper.h"
+#include "src/deoptimizer.h"
#include "src/lookup.h"
#include "src/lookup-inl.h"
@@ -107,6 +108,14 @@
has_property_ = true;
return true;
}
+
+
+void LookupIterator::ReloadPropertyInformation() {
+ state_ = BEFORE_PROPERTY;
+ state_ = LookupInHolder(*holder_map_);
+ DCHECK(IsFound());
+ HasProperty();
+}
void LookupIterator::PrepareForDataProperty(Handle<Object> value) {
@@ -116,13 +125,7 @@
holder_map_ =
Map::PrepareForDataProperty(holder_map_, descriptor_number(), value);
JSObject::MigrateToMap(GetHolder<JSObject>(), holder_map_);
- // Reload property information.
- if (holder_map_->is_dictionary_map()) {
- property_encoding_ = DICTIONARY;
- } else {
- property_encoding_ = DESCRIPTOR;
- }
- CHECK(HasProperty());
+ ReloadPropertyInformation();
}
@@ -137,17 +140,12 @@
JSObject::MigrateToMap(holder, holder_map_);
}
- // Reload property information and update the descriptor if in dictionary
- // mode.
if (holder_map_->is_dictionary_map()) {
- property_encoding_ = DICTIONARY;
PropertyDetails details(attributes, NORMAL, 0);
JSObject::SetNormalizedProperty(holder, name(), value, details);
- } else {
- property_encoding_ = DESCRIPTOR;
}
- CHECK(HasProperty());
+ ReloadPropertyInformation();
}
@@ -172,12 +170,62 @@
value, attributes,
store_mode);
JSObject::MigrateToMap(receiver, holder_map_);
- // Reload the information.
- state_ = NOT_FOUND;
- configuration_ = CHECK_PROPERTY;
- state_ = LookupInHolder(*holder_map_);
- DCHECK(IsFound());
- HasProperty();
+ ReloadPropertyInformation();
+}
+
+
+void LookupIterator::TransitionToAccessorProperty(
+ AccessorComponent component, Handle<Object> accessor,
+ PropertyAttributes attributes) {
+ DCHECK(!accessor->IsNull());
+ // Can only be called when the receiver is a JSObject. JSProxy has to be
+ // handled via a trap. Adding properties to primitive values is not
+ // observable.
+ Handle<JSObject> receiver = Handle<JSObject>::cast(GetReceiver());
+
+ if (receiver->IsJSGlobalProxy()) {
+ PrototypeIterator iter(isolate(), receiver);
+ receiver =
+ Handle<JSGlobalObject>::cast(PrototypeIterator::GetCurrent(iter));
+ }
+
+ maybe_holder_ = receiver;
+ holder_map_ = Map::TransitionToAccessorProperty(
+ handle(receiver->map()), name_, component, accessor, attributes);
+ JSObject::MigrateToMap(receiver, holder_map_);
+
+ ReloadPropertyInformation();
+
+ if (!holder_map_->is_dictionary_map()) return;
+
+ // We have to deoptimize since accesses to data properties may have been
+ // inlined without a corresponding map-check.
+ if (holder_map_->IsGlobalObjectMap()) {
+ Deoptimizer::DeoptimizeGlobalObject(*receiver);
+ }
+
+ // Install the accessor into the dictionary-mode object.
+ PropertyDetails details(attributes, CALLBACKS, 0);
+ Handle<AccessorPair> pair;
+ if (IsFound() && HasProperty() && property_kind() == ACCESSOR &&
+ GetAccessors()->IsAccessorPair()) {
+ pair = Handle<AccessorPair>::cast(GetAccessors());
+ // If the component and attributes are identical, nothing has to be
done.
+ if (pair->get(component) == *accessor) {
+ if (property_details().attributes() == attributes) return;
+ } else {
+ pair = AccessorPair::Copy(pair);
+ pair->set(component, *accessor);
+ }
+ } else {
+ pair = isolate()->factory()->NewAccessorPair();
+ pair->set(component, *accessor);
+ }
+ JSObject::SetNormalizedProperty(receiver, name_, pair, details);
+
+ JSObject::ReoptimizeIfPrototype(receiver);
+ holder_map_ = handle(receiver->map());
+ ReloadPropertyInformation();
}
=======================================
--- /branches/bleeding_edge/src/lookup.h Mon Aug 18 15:08:14 2014 UTC
+++ /branches/bleeding_edge/src/lookup.h Tue Aug 19 17:02:04 2014 UTC
@@ -31,11 +31,14 @@
};
enum State {
+ ACCESS_CHECK,
+ INTERCEPTOR,
+ JSPROXY,
NOT_FOUND,
PROPERTY,
- INTERCEPTOR,
- ACCESS_CHECK,
- JSPROXY
+ // Set state_ to BEFORE_PROPERTY to ensure that the next lookup will
be a
+ // PROPERTY lookup.
+ BEFORE_PROPERTY = INTERCEPTOR
};
enum PropertyKind {
@@ -100,6 +103,10 @@
bool IsFound() const { return state_ != NOT_FOUND; }
void Next();
+ void NotFound() {
+ has_property_ = false;
+ state_ = NOT_FOUND;
+ }
Heap* heap() const { return isolate_->heap(); }
Factory* factory() const { return isolate_->factory(); }
@@ -130,6 +137,9 @@
Object::StoreFromKeyed store_mode);
void ReconfigureDataProperty(Handle<Object> value,
PropertyAttributes attributes);
+ void TransitionToAccessorProperty(AccessorComponent component,
+ Handle<Object> accessor,
+ PropertyAttributes attributes);
PropertyKind property_kind() const {
DCHECK(has_property_);
return property_kind_;
@@ -162,6 +172,7 @@
MUST_USE_RESULT inline JSReceiver* NextHolder(Map* map);
inline State LookupInHolder(Map* map);
Handle<Object> FetchValue() const;
+ void ReloadPropertyInformation();
bool IsBootstrapping() const;
=======================================
--- /branches/bleeding_edge/src/objects.cc Tue Aug 19 14:58:41 2014 UTC
+++ /branches/bleeding_edge/src/objects.cc Tue Aug 19 17:02:04 2014 UTC
@@ -6100,51 +6100,6 @@
SetElementCallback(object, index, accessors, attributes);
}
-
-
-Handle<AccessorPair> JSObject::CreateAccessorPairFor(Handle<JSObject>
object,
- Handle<Name> name) {
- Isolate* isolate = object->GetIsolate();
- LookupResult result(isolate);
- object->LookupOwnRealNamedProperty(name, &result);
- if (result.IsPropertyCallbacks()) {
- // Note that the result can actually have IsDontDelete() == true when
we
- // e.g. have to fall back to the slow case while adding a setter after
- // successfully reusing a map transition for a getter. Nevertheless,
this is
- // OK, because the assertion only holds for the whole addition of both
- // accessors, not for the addition of each part. See first comment in
- // DefinePropertyAccessor below.
- Object* obj = result.GetCallbackObject();
- if (obj->IsAccessorPair()) {
- return AccessorPair::Copy(handle(AccessorPair::cast(obj), isolate));
- }
- }
- return isolate->factory()->NewAccessorPair();
-}
-
-
-void JSObject::DefinePropertyAccessor(Handle<JSObject> object,
- Handle<Name> name,
- Handle<Object> getter,
- Handle<Object> setter,
- PropertyAttributes attributes) {
- // We could assert that the property is configurable here, but we would
need
- // to do a lookup, which seems to be a bit of overkill.
- bool only_attribute_changes = getter->IsNull() && setter->IsNull();
- if (object->HasFastProperties() && !only_attribute_changes &&
- (object->map()->NumberOfOwnDescriptors() <=
kMaxNumberOfDescriptors)) {
- bool getterOk = getter->IsNull() ||
- DefineFastAccessor(object, name, ACCESSOR_GETTER, getter,
attributes);
- bool setterOk = !getterOk || setter->IsNull() ||
- DefineFastAccessor(object, name, ACCESSOR_SETTER, setter,
attributes);
- if (getterOk && setterOk) return;
- }
-
- Handle<AccessorPair> accessors = CreateAccessorPairFor(object, name);
- accessors->SetComponents(*getter, *setter);
-
- SetPropertyCallback(object, name, accessors, attributes);
-}
bool Map::DictionaryElementsInPrototypeChainOnly() {
@@ -6305,7 +6260,19 @@
if (is_element) {
DefineElementAccessor(object, index, getter, setter, attributes);
} else {
- DefinePropertyAccessor(object, name, getter, setter, attributes);
+ DCHECK(getter->IsSpecFunction() || getter->IsUndefined() ||
+ getter->IsNull());
+ DCHECK(setter->IsSpecFunction() || setter->IsUndefined() ||
+ setter->IsNull());
+ // At least one of the accessors needs to be a new value.
+ DCHECK(!getter->IsNull() || !setter->IsNull());
+ LookupIterator it(object, name, LookupIterator::CHECK_PROPERTY);
+ if (!getter->IsNull()) {
+ it.TransitionToAccessorProperty(ACCESSOR_GETTER, getter, attributes);
+ }
+ if (!setter->IsNull()) {
+ it.TransitionToAccessorProperty(ACCESSOR_SETTER, setter, attributes);
+ }
}
if (is_observed) {
@@ -6315,111 +6282,6 @@
return isolate->factory()->undefined_value();
}
-
-
-static bool TryAccessorTransition(Handle<JSObject> self,
- Handle<Map> transitioned_map,
- int target_descriptor,
- AccessorComponent component,
- Handle<Object> accessor,
- PropertyAttributes attributes) {
- DescriptorArray* descs = transitioned_map->instance_descriptors();
- PropertyDetails details = descs->GetDetails(target_descriptor);
-
- // If the transition target was not callbacks, fall back to the slow
case.
- if (details.type() != CALLBACKS) return false;
- Object* descriptor = descs->GetCallbacksObject(target_descriptor);
- if (!descriptor->IsAccessorPair()) return false;
-
- Object* target_accessor = AccessorPair::cast(descriptor)->get(component);
- PropertyAttributes target_attributes = details.attributes();
-
- // Reuse transition if adding same accessor with same attributes.
- if (target_accessor == *accessor && target_attributes == attributes) {
- JSObject::MigrateToMap(self, transitioned_map);
- return true;
- }
-
- // If either not the same accessor, or not the same attributes, fall
back to
- // the slow case.
- return false;
-}
-
-
-bool JSObject::DefineFastAccessor(Handle<JSObject> object,
- Handle<Name> name,
- AccessorComponent component,
- Handle<Object> accessor,
- PropertyAttributes attributes) {
- DCHECK(accessor->IsSpecFunction() || accessor->IsUndefined());
- Isolate* isolate = object->GetIsolate();
- LookupResult result(isolate);
- object->LookupOwn(name, &result);
-
- if (result.IsFound() && !result.IsPropertyCallbacks()) {
- return false;
- }
-
- // Return success if the same accessor with the same attributes already
exist.
- AccessorPair* source_accessors = NULL;
- if (result.IsPropertyCallbacks()) {
- Object* callback_value = result.GetCallbackObject();
- if (callback_value->IsAccessorPair()) {
- source_accessors = AccessorPair::cast(callback_value);
- Object* entry = source_accessors->get(component);
- if (entry == *accessor && result.GetAttributes() == attributes) {
- return true;
- }
- } else {
- return false;
- }
-
- int descriptor_number = result.GetDescriptorIndex();
-
- object->map()->LookupTransition(*object, *name, &result);
-
- if (result.IsFound()) {
- Handle<Map> target(result.GetTransitionTarget());
- DCHECK(target->NumberOfOwnDescriptors() ==
- object->map()->NumberOfOwnDescriptors());
- // This works since descriptors are sorted in order of addition.
- DCHECK(Name::Equals(
- handle(object->map()->instance_descriptors()->GetKey(
- descriptor_number)),
- name));
- return TryAccessorTransition(object, target, descriptor_number,
- component, accessor, attributes);
- }
- } else {
- // If not, lookup a transition.
- object->map()->LookupTransition(*object, *name, &result);
-
- // If there is a transition, try to follow it.
- if (result.IsFound()) {
- Handle<Map> target(result.GetTransitionTarget());
- int descriptor_number = target->LastAdded();
- DCHECK(Name::Equals(name,
-
handle(target->instance_descriptors()->GetKey(descriptor_number))));
- return TryAccessorTransition(object, target, descriptor_number,
- component, accessor, attributes);
- }
- }
-
- // If there is no transition yet, add a transition to the a new accessor
pair
- // containing the accessor. Allocate a new pair if there were no source
- // accessors. Otherwise, copy the pair and modify the accessor.
- Handle<AccessorPair> accessors = source_accessors != NULL
- ? AccessorPair::Copy(Handle<AccessorPair>(source_accessors))
- : isolate->factory()->NewAccessorPair();
- accessors->set(component, *accessor);
-
- CallbacksDescriptor new_accessors_desc(name, accessors, attributes);
- Handle<Map> new_map = Map::CopyInsertDescriptor(
- handle(object->map()), &new_accessors_desc, INSERT_TRANSITION);
-
- JSObject::MigrateToMap(object, new_map);
- return true;
-}
MaybeHandle<Object> JSObject::SetAccessor(Handle<JSObject> object,
@@ -7060,6 +6922,101 @@
return CopyGeneralizeAllRepresentations(map, descriptor, FORCE_FIELD,
attributes, "attributes
mismatch");
}
+
+
+Handle<Map> Map::TransitionToAccessorProperty(Handle<Map> map,
+ Handle<Name> name,
+ AccessorComponent component,
+ Handle<Object> accessor,
+ PropertyAttributes
attributes) {
+ Isolate* isolate = name->GetIsolate();
+
+ // Dictionary maps can always have additional data properties.
+ if (map->is_dictionary_map()) {
+ // For global objects, property cells are inlined. We need to change
the
+ // map.
+ if (map->IsGlobalObjectMap()) return Copy(map);
+ return map;
+ }
+
+ // Migrate to the newest map before transitioning to the new property.
+ if (map->is_deprecated()) map = Update(map);
+
+ PropertyNormalizationMode mode = map->is_prototype_map()
+ ? KEEP_INOBJECT_PROPERTIES
+ : CLEAR_INOBJECT_PROPERTIES;
+
+ int index = map->SearchTransition(*name);
+ if (index != TransitionArray::kNotFound) {
+ Handle<Map> transition(map->GetTransition(index));
+ DescriptorArray* descriptors = transition->instance_descriptors();
+ // Fast path, assume that we're modifying the last added descriptor.
+ int descriptor = transition->LastAdded();
+ if (descriptors->GetKey(descriptor) != *name) {
+ // If not, search for the descriptor.
+ descriptor = descriptors->SearchWithCache(*name, *transition);
+ }
+
+ if (descriptors->GetDetails(descriptor).type() != CALLBACKS) {
+ return Map::Normalize(map, mode);
+ }
+
+ // TODO(verwaest): Handle attributes better.
+ if (descriptors->GetDetails(descriptor).attributes() != attributes) {
+ return Map::Normalize(map, mode);
+ }
+
+ Handle<Object> maybe_pair(descriptors->GetValue(descriptor), isolate);
+ if (!maybe_pair->IsAccessorPair()) {
+ return Map::Normalize(map, mode);
+ }
+
+ Handle<AccessorPair> pair = Handle<AccessorPair>::cast(maybe_pair);
+ if (pair->get(component) != *accessor) {
+ return Map::Normalize(map, mode);
+ }
+
+ return transition;
+ }
+
+ Handle<AccessorPair> pair;
+ DescriptorArray* old_descriptors = map->instance_descriptors();
+ int descriptor = old_descriptors->SearchWithCache(*name, *map);
+ if (descriptor != DescriptorArray::kNotFound) {
+ PropertyDetails old_details = old_descriptors->GetDetails(descriptor);
+ if (old_details.type() != CALLBACKS) {
+ return Map::Normalize(map, mode);
+ }
+
+ if (old_details.attributes() != attributes) {
+ return Map::Normalize(map, mode);
+ }
+
+ Handle<Object> maybe_pair(old_descriptors->GetValue(descriptor),
isolate);
+ if (!maybe_pair->IsAccessorPair()) {
+ return Map::Normalize(map, mode);
+ }
+
+ Object* current =
Handle<AccessorPair>::cast(maybe_pair)->get(component);
+ if (current == *accessor) return map;
+
+ if (!current->IsTheHole()) {
+ return Map::Normalize(map, mode);
+ }
+
+ pair = AccessorPair::Copy(Handle<AccessorPair>::cast(maybe_pair));
+ } else if (map->NumberOfOwnDescriptors() >= kMaxNumberOfDescriptors ||
+ map->TooManyFastProperties(CERTAINLY_NOT_STORE_FROM_KEYED)) {
+ return Map::Normalize(map, CLEAR_INOBJECT_PROPERTIES);
+ } else {
+ pair = isolate->factory()->NewAccessorPair();
+ }
+
+ pair->set(component, *accessor);
+ TransitionFlag flag = INSERT_TRANSITION;
+ CallbacksDescriptor new_desc(name, pair, attributes);
+ return Map::CopyInsertDescriptor(map, &new_desc, flag);
+}
Handle<Map> Map::CopyAddDescriptor(Handle<Map> map,
=======================================
--- /branches/bleeding_edge/src/objects.h Tue Aug 19 14:58:41 2014 UTC
+++ /branches/bleeding_edge/src/objects.h Tue Aug 19 17:02:04 2014 UTC
@@ -2724,22 +2724,6 @@
Handle<Object> getter,
Handle<Object> setter,
PropertyAttributes attributes);
- static Handle<AccessorPair> CreateAccessorPairFor(Handle<JSObject>
object,
- Handle<Name> name);
- static void DefinePropertyAccessor(Handle<JSObject> object,
- Handle<Name> name,
- Handle<Object> getter,
- Handle<Object> setter,
- PropertyAttributes attributes);
-
- // Try to define a single accessor paying attention to map transitions.
- // Returns false if this was not possible and we have to use the slow
case.
- static bool DefineFastAccessor(Handle<JSObject> object,
- Handle<Name> name,
- AccessorComponent component,
- Handle<Object> accessor,
- PropertyAttributes attributes);
-
// Return the hash table backing store or the inline stored identity
hash,
// whatever is found.
@@ -6462,6 +6446,9 @@
Handle<Object> value,
PropertyAttributes
attributes,
StoreFromKeyed store_mode);
+ static Handle<Map> TransitionToAccessorProperty(
+ Handle<Map> map, Handle<Name> name, AccessorComponent component,
+ Handle<Object> accessor, PropertyAttributes attributes);
static Handle<Map> ReconfigureDataProperty(Handle<Map> map, int
descriptor,
PropertyAttributes
attributes);
=======================================
--- /branches/bleeding_edge/src/runtime.cc Tue Aug 19 14:55:47 2014 UTC
+++ /branches/bleeding_edge/src/runtime.cc Tue Aug 19 17:02:04 2014 UTC
@@ -4948,7 +4948,7 @@
// Transform getter or setter into something DefineAccessor can handle.
static Handle<Object> InstantiateAccessorComponent(Isolate* isolate,
Handle<Object>
component) {
- if (component->IsUndefined()) return isolate->factory()->null_value();
+ if (component->IsUndefined()) return
isolate->factory()->undefined_value();
Handle<FunctionTemplateInfo> info =
Handle<FunctionTemplateInfo>::cast(component);
return Utils::OpenHandle(*Utils::ToLocal(info)->GetFunction());
=======================================
--- /branches/bleeding_edge/src/stub-cache.cc Mon Aug 18 15:03:13 2014 UTC
+++ /branches/bleeding_edge/src/stub-cache.cc Tue Aug 19 17:02:04 2014 UTC
@@ -1191,18 +1191,6 @@
MacroAssembler* masm) {
KeyedStoreIC::GenerateSlow(masm);
}
-
-
-CallOptimization::CallOptimization(LookupResult* lookup) {
- if (lookup->IsFound() &&
- lookup->IsCacheable() &&
- lookup->IsConstantFunction()) {
- // We only optimize constant function calls.
- Initialize(Handle<JSFunction>(lookup->GetConstantFunction()));
- } else {
- Initialize(Handle<JSFunction>::null());
- }
-}
CallOptimization::CallOptimization(Handle<JSFunction> function) {
=======================================
--- /branches/bleeding_edge/src/stub-cache.h Mon Aug 18 15:03:13 2014 UTC
+++ /branches/bleeding_edge/src/stub-cache.h Tue Aug 19 17:02:04 2014 UTC
@@ -626,8 +626,6 @@
// Holds information about possible function call optimizations.
class CallOptimization BASE_EMBEDDED {
public:
- explicit CallOptimization(LookupResult* lookup);
-
explicit CallOptimization(Handle<JSFunction> function);
bool is_constant_call() const {
=======================================
--- /branches/bleeding_edge/src/v8natives.js Mon Aug 11 19:24:05 2014 UTC
+++ /branches/bleeding_edge/src/v8natives.js Tue Aug 19 17:02:04 2014 UTC
@@ -840,8 +840,18 @@
// property.
// Step 12 - updating an existing accessor property with an accessor
// descriptor.
- var getter = desc.hasGetter() ? desc.getGet() : null;
- var setter = desc.hasSetter() ? desc.getSet() : null;
+ var getter = null;
+ if (desc.hasGetter()) {
+ getter = desc.getGet();
+ } else if (IsAccessorDescriptor(current) && current.hasGetter()) {
+ getter = current.getGet();
+ }
+ var setter = null;
+ if (desc.hasSetter()) {
+ setter = desc.getSet();
+ } else if (IsAccessorDescriptor(current) && current.hasSetter()) {
+ setter = current.getSet();
+ }
%DefineAccessorPropertyUnchecked(obj, p, getter, setter, flag);
}
return true;
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
