Revision: 9930
Author: [email protected]
Date: Wed Nov 9 04:47:15 2011
Log: Made PropertyType handling even more explicit.
Replaced FIRST_PHANTOM_PROPERTY_TYPE by a predicate. Removed the (hopefully)
last default cases for switches on PropertyType. Benchmarks show that both
changes are performace-neutral.
Now every value of PropertyType should either be handled by an explicit
case in
a switch or by an equality operator. Therefore, the C++ compiler should
finally
be able to tell us which places to touch when changing PropertyType.
Review URL: http://codereview.chromium.org/8506004
http://code.google.com/p/v8/source/detail?r=9930
Modified:
/branches/bleeding_edge/src/factory.cc
/branches/bleeding_edge/src/mark-compact.cc
/branches/bleeding_edge/src/objects-inl.h
/branches/bleeding_edge/src/profile-generator.cc
/branches/bleeding_edge/src/property-details.h
/branches/bleeding_edge/src/property.h
/branches/bleeding_edge/src/string-stream.cc
=======================================
--- /branches/bleeding_edge/src/factory.cc Thu Nov 3 03:36:55 2011
+++ /branches/bleeding_edge/src/factory.cc Wed Nov 9 04:47:15 2011
@@ -844,7 +844,7 @@
// Copy the descriptors from the array.
for (int i = 0; i < array->number_of_descriptors(); i++) {
- if (array->GetType(i) != NULL_DESCRIPTOR) {
+ if (!array->IsNullDescriptor(i)) {
result->CopyFrom(descriptor_count++, *array, i, witness);
}
}
=======================================
--- /branches/bleeding_edge/src/mark-compact.cc Thu Nov 3 07:17:05 2011
+++ /branches/bleeding_edge/src/mark-compact.cc Wed Nov 9 04:47:15 2011
@@ -1635,15 +1635,14 @@
RecordSlot(slot, slot, *slot);
- PropertyType type = details.type();
- if (type < FIRST_PHANTOM_PROPERTY_TYPE) {
+ if (details.IsProperty()) {
HeapObject* object = HeapObject::cast(value);
MarkBit mark = Marking::MarkBitFrom(HeapObject::cast(object));
if (!mark.Get()) {
SetMark(HeapObject::cast(object), mark);
marking_deque_.PushBlack(object);
}
- } else if (type == ELEMENTS_TRANSITION && value->IsFixedArray()) {
+ } else if (details.type() == ELEMENTS_TRANSITION &&
value->IsFixedArray()) {
// For maps with multiple elements transitions, the transition maps
are
// stored in a FixedArray. Keep the fixed array alive but not the
maps
// that it refers to.
=======================================
--- /branches/bleeding_edge/src/objects-inl.h Tue Nov 8 00:42:13 2011
+++ /branches/bleeding_edge/src/objects-inl.h Wed Nov 9 04:47:15 2011
@@ -1860,7 +1860,7 @@
bool DescriptorArray::IsProperty(int descriptor_number) {
- return GetType(descriptor_number) < FIRST_PHANTOM_PROPERTY_TYPE;
+ return IsRealProperty(GetType(descriptor_number));
}
=======================================
--- /branches/bleeding_edge/src/profile-generator.cc Wed Nov 9 04:15:35
2011
+++ /branches/bleeding_edge/src/profile-generator.cc Wed Nov 9 04:47:15
2011
@@ -2114,7 +2114,17 @@
js_obj, entry,
descs->GetKey(i), descs->GetConstantFunction(i));
break;
- default: ;
+ case NORMAL: // only in slow mode
+ case HANDLER: // only in lookup results, not in descriptors
+ case INTERCEPTOR: // only in lookup results, not in descriptors
+ case MAP_TRANSITION: // we do not care about transitions here...
+ case ELEMENTS_TRANSITION:
+ case CONSTANT_TRANSITION:
+ case NULL_DESCRIPTOR: // ... and not about "holes"
+ break;
+ // TODO(svenpanne): Should we really ignore accessors here?
+ case CALLBACKS:
+ break;
}
}
} else {
=======================================
--- /branches/bleeding_edge/src/property-details.h Tue Nov 8 02:43:25 2011
+++ /branches/bleeding_edge/src/property-details.h Wed Nov 9 04:47:15 2011
@@ -61,12 +61,11 @@
CALLBACKS = 3,
HANDLER = 4, // only in lookup results, not in
descriptors
INTERCEPTOR = 5, // only in lookup results, not in
descriptors
+ // All properties before MAP_TRANSITION are real.
MAP_TRANSITION = 6, // only in fast mode
ELEMENTS_TRANSITION = 7,
CONSTANT_TRANSITION = 8, // only in fast mode
NULL_DESCRIPTOR = 9, // only in fast mode
- // All properties before MAP_TRANSITION are real.
- FIRST_PHANTOM_PROPERTY_TYPE = MAP_TRANSITION,
// There are no IC stubs for NULL_DESCRIPTORS. Therefore,
// NULL_DESCRIPTOR can be used as the type flag for IC stubs for
// nonexistent properties.
@@ -92,6 +91,26 @@
UNREACHABLE(); // keep the compiler happy
return false;
}
+
+
+inline bool IsRealProperty(PropertyType type) {
+ switch (type) {
+ case NORMAL:
+ case FIELD:
+ case CONSTANT_FUNCTION:
+ case CALLBACKS:
+ case HANDLER:
+ case INTERCEPTOR:
+ return true;
+ case MAP_TRANSITION:
+ case ELEMENTS_TRANSITION:
+ case CONSTANT_TRANSITION:
+ case NULL_DESCRIPTOR:
+ return false;
+ }
+ UNREACHABLE(); // keep the compiler happy
+ return false;
+}
// PropertyDetails captures type and attributes for a property.
@@ -127,7 +146,7 @@
}
bool IsProperty() {
- return type() < FIRST_PHANTOM_PROPERTY_TYPE;
+ return IsRealProperty(type());
}
PropertyAttributes attributes() { return
AttributesField::decode(value_); }
=======================================
--- /branches/bleeding_edge/src/property.h Tue Nov 8 00:42:13 2011
+++ /branches/bleeding_edge/src/property.h Wed Nov 9 04:47:15 2011
@@ -262,7 +262,7 @@
// Is the result is a property excluding transitions and the null
// descriptor?
bool IsProperty() {
- return IsFound() && (type() < FIRST_PHANTOM_PROPERTY_TYPE);
+ return IsFound() && GetPropertyDetails().IsProperty();
}
// Is the result a property or a transition?
=======================================
--- /branches/bleeding_edge/src/string-stream.cc Fri Sep 9 15:39:47 2011
+++ /branches/bleeding_edge/src/string-stream.cc Wed Nov 9 04:47:15 2011
@@ -350,29 +350,24 @@
}
DescriptorArray* descs = map->instance_descriptors();
for (int i = 0; i < descs->number_of_descriptors(); i++) {
- switch (descs->GetType(i)) {
- case FIELD: {
- Object* key = descs->GetKey(i);
- if (key->IsString() || key->IsNumber()) {
- int len = 3;
- if (key->IsString()) {
- len = String::cast(key)->length();
- }
- for (; len < 18; len++)
- Put(' ');
- if (key->IsString()) {
- Put(String::cast(key));
- } else {
- key->ShortPrint();
- }
- Add(": ");
- Object* value =
js_object->FastPropertyAt(descs->GetFieldIndex(i));
- Add("%o\n", value);
- }
- }
- break;
- default:
- break;
+ if (descs->GetType(i) == FIELD) {
+ Object* key = descs->GetKey(i);
+ if (key->IsString() || key->IsNumber()) {
+ int len = 3;
+ if (key->IsString()) {
+ len = String::cast(key)->length();
+ }
+ for (; len < 18; len++)
+ Put(' ');
+ if (key->IsString()) {
+ Put(String::cast(key));
+ } else {
+ key->ShortPrint();
+ }
+ Add(": ");
+ Object* value = js_object->FastPropertyAt(descs->GetFieldIndex(i));
+ Add("%o\n", value);
+ }
}
}
}
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
