Revision: 10600
Author: [email protected]
Date: Fri Feb 3 05:37:13 2012
Log: Removed IsTransitionType predicate.
With the upcoming changes to CALLBACKS properties, a predicate on the
transition
type alone doesn't make sense anymore: For CALLBACKS one has to look into
the
property's value to decide, and there is even the possibility of having a an
accessor function *and* a transition in the same property.
I am not completely happy with some parts of this CL, because they contain
redundant code, but given the various representations we currently have for
property type/value pairs, I can see no easy way around that. Perhaps one
can
improve this a bit in a different CL, the current diversity really, really
hurts
productivity...
As a bonus, this CL includes a few minor things:
* CaseClause::RecordTypeFeedback has been cleaned up and it handles the
NULL_DESCRIPTOR case correctly now. Under some (very unlikely)
circumstances,
we previously missed some opportunities for monomorphic calls. In
general, it
is rather unfortunate that NULL_DESCRIPTOR "shines through", it is just a
hack for the inability to remove a descriptor entry during GC, something
callers shouldn't have to be aware of.
* DescriptorArray::CopyInsert has been cleaned up a bit, preparing it for
later
CALLBACKS-related changes.
* LookupResult::Print is now more informative for CONSTANT_TRANSITION.
Review URL: https://chromiumcodereview.appspot.com/9320066
http://code.google.com/p/v8/source/detail?r=10600
Modified:
/branches/bleeding_edge/src/ast.cc
/branches/bleeding_edge/src/objects-inl.h
/branches/bleeding_edge/src/objects.cc
/branches/bleeding_edge/src/objects.h
/branches/bleeding_edge/src/property-details.h
/branches/bleeding_edge/src/property.cc
/branches/bleeding_edge/src/property.h
=======================================
--- /branches/bleeding_edge/src/ast.cc Wed Jan 25 08:31:25 2012
+++ /branches/bleeding_edge/src/ast.cc Fri Feb 3 05:37:13 2012
@@ -730,33 +730,32 @@
bool Call::ComputeTarget(Handle<Map> type, Handle<String> name) {
- // If there is an interceptor, we can't compute the target for
- // a direct call.
+ // If there is an interceptor, we can't compute the target for a direct
call.
if (type->has_named_interceptor()) return false;
if (check_type_ == RECEIVER_MAP_CHECK) {
- // For primitive checks the holder is set up to point to the
- // corresponding prototype object, i.e. one step of the algorithm
- // below has been already performed.
- // For non-primitive checks we clear it to allow computing targets
- // for polymorphic calls.
+ // For primitive checks the holder is set up to point to the
corresponding
+ // prototype object, i.e. one step of the algorithm below has been
already
+ // performed. For non-primitive checks we clear it to allow computing
+ // targets for polymorphic calls.
holder_ = Handle<JSObject>::null();
}
+ LookupResult lookup(type->GetIsolate());
while (true) {
- LookupResult lookup(type->GetIsolate());
type->LookupInDescriptors(NULL, *name, &lookup);
- // If the function wasn't found directly in the map, we start
- // looking upwards through the prototype chain.
- if ((!lookup.IsFound() || IsTransitionType(lookup.type()))
- && type->prototype()->IsJSObject()) {
- holder_ = Handle<JSObject>(JSObject::cast(type->prototype()));
- type = Handle<Map>(holder()->map());
- } else if (lookup.IsFound() && lookup.type() == CONSTANT_FUNCTION) {
- target_ =
Handle<JSFunction>(lookup.GetConstantFunctionFromMap(*type));
- return true;
- } else {
+ // For properties we know the target iff we have a constant function.
+ if (lookup.IsFound() && lookup.IsProperty()) {
+ if (lookup.type() == CONSTANT_FUNCTION) {
+ target_ =
Handle<JSFunction>(lookup.GetConstantFunctionFromMap(*type));
+ return true;
+ }
return false;
}
+ // If we reach the end of the prototype chain, we don't know the
target.
+ if (!type->prototype()->IsJSObject()) return false;
+ // Go up the prototype chain, recording where we are currently.
+ holder_ = Handle<JSObject>(JSObject::cast(type->prototype()));
+ type = Handle<Map>(holder()->map());
}
}
=======================================
--- /branches/bleeding_edge/src/objects-inl.h Fri Jan 27 08:09:20 2012
+++ /branches/bleeding_edge/src/objects-inl.h Fri Feb 3 05:37:13 2012
@@ -1991,8 +1991,28 @@
}
-bool DescriptorArray::IsTransition(int descriptor_number) {
- return IsTransitionType(GetType(descriptor_number));
+bool DescriptorArray::IsTransitionOnly(int descriptor_number) {
+ switch (GetType(descriptor_number)) {
+ case MAP_TRANSITION:
+ case CONSTANT_TRANSITION:
+ case ELEMENTS_TRANSITION:
+ return true;
+ case CALLBACKS: {
+ Object* value = GetValue(descriptor_number);
+ if (!value->IsAccessorPair()) return false;
+ AccessorPair* accessors = AccessorPair::cast(value);
+ return accessors->getter()->IsMap() && accessors->setter()->IsMap();
+ }
+ case NORMAL:
+ case FIELD:
+ case CONSTANT_FUNCTION:
+ case HANDLER:
+ case INTERCEPTOR:
+ case NULL_DESCRIPTOR:
+ return false;
+ }
+ UNREACHABLE(); // Keep the compiler happy.
+ return false;
}
=======================================
--- /branches/bleeding_edge/src/objects.cc Mon Jan 30 06:23:35 2012
+++ /branches/bleeding_edge/src/objects.cc Fri Feb 3 05:37:13 2012
@@ -1823,7 +1823,7 @@
int new_enumeration_index = 0; // 0 means "Use the next available
index."
if (old_index != -1) {
// All calls to ReplaceSlowProperty have had all transitions removed.
- ASSERT(!dictionary->DetailsAt(old_index).IsTransition());
+ ASSERT(!dictionary->ContainsTransition(old_index));
new_enumeration_index = dictionary->DetailsAt(old_index).index();
}
@@ -5729,7 +5729,7 @@
// Conversely, we filter after replacing, so replacing a transition and
// removing all other transitions is not supported.
bool remove_transitions = transition_flag == REMOVE_TRANSITIONS;
- ASSERT(remove_transitions == !descriptor->GetDetails().IsTransition());
+ ASSERT(remove_transitions == !descriptor->ContainsTransition());
ASSERT(descriptor->GetDetails().type() != NULL_DESCRIPTOR);
// Ensure the key is a symbol.
@@ -5738,29 +5738,18 @@
if (!maybe_result->ToObject(&result)) return maybe_result;
}
- int transitions = 0;
- int null_descriptors = 0;
- if (remove_transitions) {
- for (int i = 0; i < number_of_descriptors(); i++) {
- if (IsTransition(i)) transitions++;
- if (IsNullDescriptor(i)) null_descriptors++;
- }
- } else {
- for (int i = 0; i < number_of_descriptors(); i++) {
- if (IsNullDescriptor(i)) null_descriptors++;
- }
- }
- int new_size = number_of_descriptors() - transitions - null_descriptors;
+ int new_size = 0;
+ for (int i = 0; i < number_of_descriptors(); i++) {
+ if (IsNullDescriptor(i)) continue;
+ if (remove_transitions && IsTransitionOnly(i)) continue;
+ new_size++;
+ }
// If key is in descriptor, we replace it in-place when filtering.
// Count a null descriptor for key as inserted, not replaced.
int index = Search(descriptor->GetKey());
- const bool inserting = (index == kNotFound);
- const bool replacing = !inserting;
+ const bool replacing = (index != kNotFound);
bool keep_enumeration_index = false;
- if (inserting) {
- ++new_size;
- }
if (replacing) {
// We are replacing an existing descriptor. We keep the enumeration
// index of a visible property.
@@ -5775,6 +5764,8 @@
// a transition that will be replaced. Adjust count in this case.
++new_size;
}
+ } else {
+ ++new_size;
}
DescriptorArray* new_descriptors;
@@ -5789,7 +5780,7 @@
// Set the enumeration index in the descriptors and set the enumeration
index
// in the result.
int enumeration_index = NextEnumerationIndex();
- if (!descriptor->GetDetails().IsTransition()) {
+ if (!descriptor->ContainsTransition()) {
if (keep_enumeration_index) {
descriptor->SetEnumerationIndex(
PropertyDetails(GetDetails(index)).index());
@@ -5812,7 +5803,7 @@
break;
}
if (IsNullDescriptor(from_index)) continue;
- if (remove_transitions && IsTransition(from_index)) continue;
+ if (remove_transitions && IsTransitionOnly(from_index)) continue;
new_descriptors->CopyFrom(to_index++, this, from_index, witness);
}
@@ -5821,7 +5812,7 @@
for (; from_index < number_of_descriptors(); from_index++) {
if (IsNullDescriptor(from_index)) continue;
- if (remove_transitions && IsTransition(from_index)) continue;
+ if (remove_transitions && IsTransitionOnly(from_index)) continue;
new_descriptors->CopyFrom(to_index++, this, from_index, witness);
}
@@ -11131,6 +11122,31 @@
}
return kNotFound;
}
+
+
+bool StringDictionary::ContainsTransition(int entry) {
+ switch (DetailsAt(entry).type()) {
+ case MAP_TRANSITION:
+ case CONSTANT_TRANSITION:
+ case ELEMENTS_TRANSITION:
+ return true;
+ case CALLBACKS: {
+ Object* value = ValueAt(entry);
+ if (!value->IsAccessorPair()) return false;
+ AccessorPair* accessors = AccessorPair::cast(value);
+ return accessors->getter()->IsMap() || accessors->setter()->IsMap();
+ }
+ case NORMAL:
+ case FIELD:
+ case CONSTANT_FUNCTION:
+ case HANDLER:
+ case INTERCEPTOR:
+ case NULL_DESCRIPTOR:
+ return false;
+ }
+ UNREACHABLE(); // Keep the compiler happy.
+ return false;
+}
template<typename Shape, typename Key>
=======================================
--- /branches/bleeding_edge/src/objects.h Thu Feb 2 05:41:06 2012
+++ /branches/bleeding_edge/src/objects.h Fri Feb 3 05:37:13 2012
@@ -2412,7 +2412,7 @@
inline Object* GetCallbacksObject(int descriptor_number);
inline AccessorDescriptor* GetCallbacks(int descriptor_number);
inline bool IsProperty(int descriptor_number);
- inline bool IsTransition(int descriptor_number);
+ inline bool IsTransitionOnly(int descriptor_number);
inline bool IsNullDescriptor(int descriptor_number);
inline bool IsDontEnum(int descriptor_number);
@@ -3038,6 +3038,8 @@
// Find entry for key, otherwise return kNotFound. Optimized version of
// HashTable::FindEntry.
int FindEntry(String* key);
+
+ bool ContainsTransition(int entry);
};
=======================================
--- /branches/bleeding_edge/src/property-details.h Wed Nov 9 04:47:15 2011
+++ /branches/bleeding_edge/src/property-details.h Fri Feb 3 05:37:13 2012
@@ -1,4 +1,4 @@
-// Copyright 2011 the V8 project authors. All rights reserved.
+// Copyright 2012 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:
@@ -73,26 +73,6 @@
};
-inline bool IsTransitionType(PropertyType type) {
- switch (type) {
- case MAP_TRANSITION:
- case CONSTANT_TRANSITION:
- case ELEMENTS_TRANSITION:
- return true;
- case NORMAL:
- case FIELD:
- case CONSTANT_FUNCTION:
- case CALLBACKS:
- case HANDLER:
- case INTERCEPTOR:
- case NULL_DESCRIPTOR:
- return false;
- }
- UNREACHABLE(); // keep the compiler happy
- return false;
-}
-
-
inline bool IsRealProperty(PropertyType type) {
switch (type) {
case NORMAL:
@@ -138,12 +118,6 @@
inline Smi* AsSmi();
PropertyType type() { return TypeField::decode(value_); }
-
- bool IsTransition() {
- PropertyType t = type();
- ASSERT(t != INTERCEPTOR);
- return IsTransitionType(t);
- }
bool IsProperty() {
return IsRealProperty(type());
=======================================
--- /branches/bleeding_edge/src/property.cc Tue Oct 18 04:18:55 2011
+++ /branches/bleeding_edge/src/property.cc Fri Feb 3 05:37:13 2012
@@ -1,4 +1,4 @@
-// Copyright 2011 the V8 project authors. All rights reserved.
+// Copyright 2012 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:
@@ -91,6 +91,9 @@
break;
case CONSTANT_TRANSITION:
PrintF(out, " -type = constant property transition\n");
+ PrintF(out, " -map:\n");
+ GetTransitionMap()->Print(out);
+ PrintF(out, "\n");
break;
case NULL_DESCRIPTOR:
PrintF(out, " =type = null descriptor\n");
@@ -111,4 +114,28 @@
#endif
+bool Descriptor::ContainsTransition() {
+ switch (details_.type()) {
+ case MAP_TRANSITION:
+ case CONSTANT_TRANSITION:
+ case ELEMENTS_TRANSITION:
+ return true;
+ case CALLBACKS: {
+ if (!value_->IsAccessorPair()) return false;
+ AccessorPair* accessors = AccessorPair::cast(value_);
+ return accessors->getter()->IsMap() || accessors->setter()->IsMap();
+ }
+ case NORMAL:
+ case FIELD:
+ case CONSTANT_FUNCTION:
+ case HANDLER:
+ case INTERCEPTOR:
+ case NULL_DESCRIPTOR:
+ return false;
+ }
+ UNREACHABLE(); // Keep the compiler happy.
+ return false;
+}
+
+
} } // namespace v8::internal
=======================================
--- /branches/bleeding_edge/src/property.h Fri Jan 20 07:10:35 2012
+++ /branches/bleeding_edge/src/property.h Fri Feb 3 05:37:13 2012
@@ -1,4 +1,4 @@
-// Copyright 2011 the V8 project authors. All rights reserved.
+// Copyright 2012 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:
@@ -70,6 +70,8 @@
ASSERT(PropertyDetails::IsValidIndex(index));
details_ = PropertyDetails(details_.attributes(), details_.type(),
index);
}
+
+ bool ContainsTransition();
private:
String* key_;
@@ -290,7 +292,9 @@
Map* GetTransitionMap() {
ASSERT(lookup_type_ == DESCRIPTOR_TYPE);
- ASSERT(IsTransitionType(type()));
+ ASSERT(type() == MAP_TRANSITION ||
+ type() == ELEMENTS_TRANSITION ||
+ type() == CONSTANT_TRANSITION);
return Map::cast(GetValue());
}
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
