Revision: 5254
Author: [email protected]
Date: Thu Aug 12 07:51:59 2010
Log: Preserve constant function transition when adding the same function.
This should help in cases like:
function Constructor() {
this.foo = constFunction;
this.bar = "baz";
}
for (...) {
o = new Constructor();
// Constant call IC will work.
o.foo();
// Inlined property load will see the same map.
use(o.bar);
}
This change also fixes a latent bug in custom call IC-s for strings
exposed by string-charcodeat.js.
Review URL: http://codereview.chromium.org/3160006
http://code.google.com/p/v8/source/detail?r=5254
Modified:
/branches/bleeding_edge/src/ia32/stub-cache-ia32.cc
/branches/bleeding_edge/src/mark-compact.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.h
=======================================
--- /branches/bleeding_edge/src/ia32/stub-cache-ia32.cc Wed Aug 11 06:48:58
2010
+++ /branches/bleeding_edge/src/ia32/stub-cache-ia32.cc Thu Aug 12 07:51:59
2010
@@ -1571,6 +1571,9 @@
// -- esp[(argc + 1) * 4] : receiver
// -----------------------------------
+ // If object is not a string, bail out to regular call.
+ if (!object->IsString()) return Heap::undefined_value();
+
const int argc = arguments().immediate();
Label miss;
@@ -1581,6 +1584,7 @@
GenerateDirectLoadGlobalFunctionPrototype(masm(),
Context::STRING_FUNCTION_INDEX,
eax);
+ ASSERT(object != holder);
CheckPrototypes(JSObject::cast(object->GetPrototype()), eax, holder,
ebx, edx, edi, name, &miss);
@@ -1635,6 +1639,9 @@
// -- esp[(argc + 1) * 4] : receiver
// -----------------------------------
+ // If object is not a string, bail out to regular call.
+ if (!object->IsString()) return Heap::undefined_value();
+
const int argc = arguments().immediate();
Label miss;
@@ -1646,6 +1653,7 @@
GenerateDirectLoadGlobalFunctionPrototype(masm(),
Context::STRING_FUNCTION_INDEX,
eax);
+ ASSERT(object != holder);
CheckPrototypes(JSObject::cast(object->GetPrototype()), eax, holder,
ebx, edx, edi, name, &miss);
=======================================
--- /branches/bleeding_edge/src/mark-compact.cc Wed Aug 11 07:30:14 2010
+++ /branches/bleeding_edge/src/mark-compact.cc Thu Aug 12 07:51:59 2010
@@ -554,10 +554,11 @@
ASSERT(contents->IsFixedArray());
ASSERT(contents->length() >= 2);
SetMark(contents);
- // Contents contains (value, details) pairs. If the details say
- // that the type of descriptor is MAP_TRANSITION, CONSTANT_TRANSITION,
- // or NULL_DESCRIPTOR, we don't mark the value as live. Only for
- // type MAP_TRANSITION is the value a Object* (a Map*).
+ // Contents contains (value, details) pairs. If the details say that
+ // the type of descriptor is MAP_TRANSITION, CONSTANT_TRANSITION, or
+ // NULL_DESCRIPTOR, we don't mark the value as live. Only for
+ // MAP_TRANSITION and CONSTANT_TRANSITION is the value an Object* (a
+ // Map*).
for (int i = 0; i < contents->length(); i += 2) {
// If the pair (value, details) at index i, i+1 is not
// a transition or null descriptor, mark the value.
=======================================
--- /branches/bleeding_edge/src/objects-inl.h Wed Aug 11 07:30:14 2010
+++ /branches/bleeding_edge/src/objects-inl.h Thu Aug 12 07:51:59 2010
@@ -1491,6 +1491,16 @@
// Slow case: perform binary search.
return BinarySearch(name, 0, nof - 1);
}
+
+
+int DescriptorArray::SearchWithCache(String* name) {
+ int number = DescriptorLookupCache::Lookup(this, name);
+ if (number == DescriptorLookupCache::kAbsent) {
+ number = Search(name);
+ DescriptorLookupCache::Update(this, name, number);
+ }
+ return number;
+}
String* DescriptorArray::GetKey(int descriptor_number) {
=======================================
--- /branches/bleeding_edge/src/objects.cc Wed Aug 11 07:30:14 2010
+++ /branches/bleeding_edge/src/objects.cc Thu Aug 12 07:51:59 2010
@@ -1330,7 +1330,7 @@
if (attributes != NONE) {
return function;
}
- ConstTransitionDescriptor mark(name);
+ ConstTransitionDescriptor mark(name, Map::cast(new_map));
new_descriptors =
old_map->instance_descriptors()->CopyInsert(&mark, KEEP_TRANSITIONS);
if (new_descriptors->IsFailure()) {
@@ -1688,11 +1688,7 @@
void JSObject::LookupInDescriptor(String* name, LookupResult* result) {
DescriptorArray* descriptors = map()->instance_descriptors();
- int number = DescriptorLookupCache::Lookup(descriptors, name);
- if (number == DescriptorLookupCache::kAbsent) {
- number = descriptors->Search(name);
- DescriptorLookupCache::Update(descriptors, name, number);
- }
+ int number = descriptors->SearchWithCache(name);
if (number != DescriptorArray::kNotFound) {
result->DescriptorResult(this, descriptors->GetDetails(number),
number);
} else {
@@ -1889,10 +1885,25 @@
result->holder());
case INTERCEPTOR:
return SetPropertyWithInterceptor(name, value, attributes);
- case CONSTANT_TRANSITION:
- // Replace with a MAP_TRANSITION to a new map with a FIELD, even
- // if the value is a function.
+ case CONSTANT_TRANSITION: {
+ // If the same constant function is being added we can simply
+ // transition to the target map.
+ Map* target_map = result->GetTransitionMap();
+ DescriptorArray* target_descriptors =
target_map->instance_descriptors();
+ int number = target_descriptors->SearchWithCache(name);
+ ASSERT(number != DescriptorArray::kNotFound);
+ ASSERT(target_descriptors->GetType(number) == CONSTANT_FUNCTION);
+ JSFunction* function =
+ JSFunction::cast(target_descriptors->GetValue(number));
+ ASSERT(!Heap::InNewSpace(function));
+ if (value == function) {
+ set_map(target_map);
+ return value;
+ }
+ // Otherwise, replace with a MAP_TRANSITION to a new map with a
+ // FIELD, even if the value is a constant function.
return ConvertDescriptorToFieldAndMapTransition(name, value,
attributes);
+ }
case NULL_DESCRIPTOR:
return ConvertDescriptorToFieldAndMapTransition(name, value,
attributes);
default:
@@ -4936,7 +4947,8 @@
void Map::CreateBackPointers() {
DescriptorArray* descriptors = instance_descriptors();
for (int i = 0; i < descriptors->number_of_descriptors(); i++) {
- if (descriptors->GetType(i) == MAP_TRANSITION) {
+ if (descriptors->GetType(i) == MAP_TRANSITION ||
+ descriptors->GetType(i) == CONSTANT_TRANSITION) {
// Get target.
Map* target = Map::cast(descriptors->GetValue(i));
#ifdef DEBUG
@@ -4977,7 +4989,8 @@
// map is not reached again by following a back pointer from a
// non-live object.
PropertyDetails details(Smi::cast(contents->get(i + 1)));
- if (details.type() == MAP_TRANSITION) {
+ if (details.type() == MAP_TRANSITION ||
+ details.type() == CONSTANT_TRANSITION) {
Map* target = reinterpret_cast<Map*>(contents->get(i));
ASSERT(target->IsHeapObject());
if (!target->IsMarked()) {
=======================================
--- /branches/bleeding_edge/src/objects.h Thu Aug 12 06:43:08 2010
+++ /branches/bleeding_edge/src/objects.h Thu Aug 12 07:51:59 2010
@@ -1865,6 +1865,10 @@
// Search the instance descriptors for given name.
inline int Search(String* name);
+ // As the above, but uses DescriptorLookupCache and updates it when
+ // necessary.
+ inline int SearchWithCache(String* name);
+
// Tells whether the name is present int the array.
bool Contains(String* name) { return kNotFound != Search(name); }
=======================================
--- /branches/bleeding_edge/src/property.h Tue Mar 23 04:40:38 2010
+++ /branches/bleeding_edge/src/property.h Thu Aug 12 07:51:59 2010
@@ -115,8 +115,8 @@
// the same CONSTANT_FUNCTION field.
class ConstTransitionDescriptor: public Descriptor {
public:
- explicit ConstTransitionDescriptor(String* key)
- : Descriptor(key, Smi::FromInt(0), NONE, CONSTANT_TRANSITION) { }
+ explicit ConstTransitionDescriptor(String* key, Map* map)
+ : Descriptor(key, map, NONE, CONSTANT_TRANSITION) { }
};
@@ -260,7 +260,7 @@
Map* GetTransitionMap() {
ASSERT(lookup_type_ == DESCRIPTOR_TYPE);
- ASSERT(type() == MAP_TRANSITION);
+ ASSERT(type() == MAP_TRANSITION || type() == CONSTANT_TRANSITION);
return Map::cast(GetValue());
}
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
