Reviewers: Kevin Millikin,
Description:
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.
Please review this at http://codereview.chromium.org/8506004/
SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files:
M src/factory.cc
M src/mark-compact.cc
M src/objects-inl.h
M src/profile-generator.cc
M src/property-details.h
M src/property.h
M src/string-stream.cc
Index: src/factory.cc
diff --git a/src/factory.cc b/src/factory.cc
index
88684a4fb87ab3907ed501fdbcd5e2d1d16e06f4..8344355a26cae6da87cd8ac032ecbec5124d7597
100644
--- a/src/factory.cc
+++ b/src/factory.cc
@@ -844,7 +844,7 @@ Handle<DescriptorArray>
Factory::CopyAppendCallbackDescriptors(
// 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);
}
}
Index: src/mark-compact.cc
diff --git a/src/mark-compact.cc b/src/mark-compact.cc
index
94e65fa3a3bdaf13681fca8a89ef1be6327561f7..c5364839892d32ada96e22b09aa80a0c5fc0835f
100644
--- a/src/mark-compact.cc
+++ b/src/mark-compact.cc
@@ -1635,15 +1635,14 @@ void MarkCompactCollector::MarkDescriptorArray(
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.
Index: src/objects-inl.h
diff --git a/src/objects-inl.h b/src/objects-inl.h
index
3b5f58ab2ecf2d61a07ea2b5fdcd0b64402a435a..4060969f1f0b991c5b01125136e2d8d536bca691
100644
--- a/src/objects-inl.h
+++ b/src/objects-inl.h
@@ -1860,7 +1860,7 @@ AccessorDescriptor* DescriptorArray::GetCallbacks(int
descriptor_number) {
bool DescriptorArray::IsProperty(int descriptor_number) {
- return GetType(descriptor_number) < FIRST_PHANTOM_PROPERTY_TYPE;
+ return IsRealProperty(GetType(descriptor_number));
}
Index: src/profile-generator.cc
diff --git a/src/profile-generator.cc b/src/profile-generator.cc
index
7760d8f90e46feb65b8f2ab3c7a615c12bb93aa1..97da3567dfdc7129ce85205417594c1e27cd894c
100644
--- a/src/profile-generator.cc
+++ b/src/profile-generator.cc
@@ -2112,7 +2112,17 @@ void
V8HeapExplorer::ExtractPropertyReferences(JSObject* js_obj,
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 {
Index: src/property-details.h
diff --git a/src/property-details.h b/src/property-details.h
index
9df99880846d6df101827163fe738abce670679d..135c2ca01321fb67fbadba602f7f47b6b6fb792e
100644
--- a/src/property-details.h
+++ b/src/property-details.h
@@ -61,12 +61,11 @@ enum PropertyType {
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.
@@ -94,6 +93,26 @@ inline bool IsTransitionType(PropertyType type) {
}
+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.
// They are used both in property dictionaries and instance descriptors.
class PropertyDetails BASE_EMBEDDED {
@@ -127,7 +146,7 @@ class PropertyDetails BASE_EMBEDDED {
}
bool IsProperty() {
- return type() < FIRST_PHANTOM_PROPERTY_TYPE;
+ return IsRealProperty(type());
}
PropertyAttributes attributes() { return
AttributesField::decode(value_); }
Index: src/property.h
diff --git a/src/property.h b/src/property.h
index
133abc1b42ef0bd50e307485f5e0f7cc3fb7a300..3203dd1120a4168102f72e1e75a3d47d5429fc42
100644
--- a/src/property.h
+++ b/src/property.h
@@ -262,7 +262,7 @@ class LookupResult BASE_EMBEDDED {
// 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?
Index: src/string-stream.cc
diff --git a/src/string-stream.cc b/src/string-stream.cc
index
8086cf9515e24f08ec6be2f5954789c84b69c956..35f7be5416019770a72505bbc3acf1aa071d422b
100644
--- a/src/string-stream.cc
+++ b/src/string-stream.cc
@@ -350,29 +350,24 @@ void StringStream::PrintUsingMap(JSObject* js_object)
{
}
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);
+ 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);
}
- break;
- default:
- break;
}
}
}
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
