Author: [email protected]
Date: Thu Mar 19 08:06:00 2009
New Revision: 1548
Added:
branches/bleeding_edge/test/mjsunit/bugs/bug-1344252.js
- copied unchanged from r1508,
/branches/bleeding_edge/test/mjsunit/bugs/bug-1344252.js
Removed:
branches/bleeding_edge/test/mjsunit/regress/regress-1344252.js
branches/bleeding_edge/test/mjsunit/regress/regress-92.js
Modified:
branches/bleeding_edge/src/accessors.cc
branches/bleeding_edge/src/heap.cc
branches/bleeding_edge/src/heap.h
branches/bleeding_edge/src/ic.cc
branches/bleeding_edge/src/messages.js
branches/bleeding_edge/src/objects.cc
branches/bleeding_edge/src/objects.h
branches/bleeding_edge/src/regexp-delay.js
branches/bleeding_edge/src/runtime.cc
branches/bleeding_edge/src/v8-counters.h
Log:
Revert change 1509 that flush ICs when adding setters on an object or
when setting a __proto__ to an object that holds a setter.
This seems to cause a major page load regression, so we need to tune
the clearing.
Review URL: http://codereview.chromium.org/50011
Modified: branches/bleeding_edge/src/accessors.cc
==============================================================================
--- branches/bleeding_edge/src/accessors.cc (original)
+++ branches/bleeding_edge/src/accessors.cc Thu Mar 19 08:06:00 2009
@@ -509,17 +509,13 @@
// SpiderMonkey behaves this way.
if (!value->IsJSObject() && !value->IsNull()) return value;
- bool clear_ics = false;
for (Object* pt = value; pt != Heap::null_value(); pt =
pt->GetPrototype()) {
- JSObject *obj = JSObject::cast(pt);
- if (obj == receiver) {
+ if (JSObject::cast(pt) == receiver) {
// Cycle detected.
HandleScope scope;
return Top::Throw(*Factory::NewError("cyclic_proto",
HandleVector<Object>(NULL, 0)));
}
- if (obj->HasLocalPropertyWithType(CALLBACKS))
- clear_ics = true;
}
// Find the first object in the chain whose prototype object is not
@@ -537,10 +533,6 @@
if (new_map->IsFailure()) return new_map;
Map::cast(new_map)->set_prototype(value);
current->set_map(Map::cast(new_map));
-
- // Finally, if the prototype contains a setter we may have broken
- // the assumptions made when creating ics so we have to clear them.
- if (clear_ics) Heap::ClearStoreICs();
// To be consistent with other Set functions, return the value.
return value;
Modified: branches/bleeding_edge/src/heap.cc
==============================================================================
--- branches/bleeding_edge/src/heap.cc (original)
+++ branches/bleeding_edge/src/heap.cc Thu Mar 19 08:06:00 2009
@@ -71,8 +71,6 @@
int Heap::old_gen_exhausted_ = false;
-bool Heap::has_store_ics_ = false;
-
int Heap::amount_of_external_allocated_memory_ = 0;
int Heap::amount_of_external_allocated_memory_at_last_global_gc_ = 0;
@@ -294,14 +292,6 @@
}
-void Heap::ClearStoreICs() {
- if (has_store_ics_) {
- Counters::clear_store_ic.Increment();
- CollectAllGarbage();
- }
-}
-
-
void Heap::CollectAllGarbageIfContextDisposed() {
// If the garbage collector interface is exposed through the global
// gc() function, we avoid being clever about forcing GCs when
@@ -482,7 +472,6 @@
void Heap::MarkCompactEpilogue(bool is_compacting) {
Top::MarkCompactEpilogue(is_compacting);
ThreadManager::MarkCompactEpilogue(is_compacting);
- Heap::has_store_ics_ = false;
}
Modified: branches/bleeding_edge/src/heap.h
==============================================================================
--- branches/bleeding_edge/src/heap.h (original)
+++ branches/bleeding_edge/src/heap.h Thu Mar 19 08:06:00 2009
@@ -606,9 +606,6 @@
// Performs a full garbage collection.
static void CollectAllGarbage();
- // Clears all inline caches by forcing a global garbage collection.
- static void ClearStoreICs();
-
// Performs a full garbage collection if a context has been disposed
// since the last time the check was performed.
static void CollectAllGarbageIfContextDisposed();
@@ -811,14 +808,6 @@
> old_gen_allocation_limit_;
}
- static bool has_store_ics() {
- return has_store_ics_;
- }
-
- static void store_ic_created() {
- has_store_ics_ = true;
- }
-
private:
static int semispace_size_;
static int initial_semispace_size_;
@@ -883,8 +872,6 @@
// Indicates that an allocation has failed in the old generation since
the
// last GC.
static int old_gen_exhausted_;
-
- static bool has_store_ics_;
// Declare all the roots
#define ROOT_DECLARATION(type, name) static type* name##_;
Modified: branches/bleeding_edge/src/ic.cc
==============================================================================
--- branches/bleeding_edge/src/ic.cc (original)
+++ branches/bleeding_edge/src/ic.cc Thu Mar 19 08:06:00 2009
@@ -912,7 +912,6 @@
// Patch the call site depending on the state of the cache.
if (state == UNINITIALIZED || state == MONOMORPHIC_PROTOTYPE_FAILURE) {
- Heap::store_ic_created();
set_target(Code::cast(code));
} else if (state == MONOMORPHIC) {
// Only move to mega morphic if the target changes.
Modified: branches/bleeding_edge/src/messages.js
==============================================================================
--- branches/bleeding_edge/src/messages.js (original)
+++ branches/bleeding_edge/src/messages.js Thu Mar 19 08:06:00 2009
@@ -602,7 +602,7 @@
// Defines accessors for a property that is calculated the first time
// the property is read and then replaces the accessor with the value.
// Also, setting the property causes the accessors to be deleted.
-function DefineOneShotAccessor(obj, name, fun, never_used) {
+function DefineOneShotAccessor(obj, name, fun) {
// Note that the accessors consistently operate on 'obj', not 'this'.
// Since the object may occur in someone else's prototype chain we
// can't rely on 'this' being the same as 'obj'.
@@ -611,10 +611,10 @@
obj[name] = value;
return value;
});
- %DefineAccessor(ToObject(obj), ToString(name), SETTER, function (v) {
+ obj.__defineSetter__(name, function (v) {
delete obj[name];
obj[name] = v;
- }, 0, never_used);
+ });
}
function DefineError(f) {
@@ -648,7 +648,7 @@
if (m === kAddMessageAccessorsMarker) {
DefineOneShotAccessor(this, 'message', function (obj) {
return FormatMessage({type: obj.type, args: obj.arguments});
- }, true);
+ });
} else if (!IS_UNDEFINED(m)) {
this.message = ToString(m);
}
Modified: branches/bleeding_edge/src/objects.cc
==============================================================================
--- branches/bleeding_edge/src/objects.cc (original)
+++ branches/bleeding_edge/src/objects.cc Thu Mar 19 08:06:00 2009
@@ -2483,38 +2483,6 @@
}
-bool JSObject::HasLocalPropertyWithType(PropertyType type) {
- if (IsJSGlobalProxy()) {
- Object* proto = GetPrototype();
- if (proto->IsNull()) return false;
- ASSERT(proto->IsJSGlobalObject());
- return JSObject::cast(proto)->HasLocalPropertyWithType(type);
- }
-
- if (HasFastProperties()) {
- DescriptorArray* descriptors = map()->instance_descriptors();
- int length = descriptors->number_of_descriptors();
- for (int i = 0; i < length; i++) {
- PropertyDetails details(descriptors->GetDetails(i));
- if (details.type() == type)
- return true;
- }
- } else {
- Handle<Dictionary> properties =
Handle<Dictionary>(property_dictionary());
- int capacity = properties->Capacity();
- for (int i = 0; i < capacity; i++) {
- if (properties->IsKey(properties->KeyAt(i))) {
- PropertyDetails details = properties->DetailsAt(i);
- if (details.type() == type)
- return true;
- }
- }
- }
-
- return false;
-}
-
-
void JSObject::Lookup(String* name, LookupResult* result) {
// Ecma-262 3rd 8.6.2.4
for (Object* current = this;
@@ -2642,11 +2610,8 @@
}
-Object* JSObject::DefineAccessor(String* name,
- bool is_getter,
- JSFunction* fun,
- PropertyAttributes attributes,
- bool never_used) {
+Object* JSObject::DefineAccessor(String* name, bool is_getter, JSFunction*
fun,
+ PropertyAttributes attributes) {
// Check access rights if needed.
if (IsAccessCheckNeeded() &&
!Top::MayNamedAccess(this, name, v8::ACCESS_HAS)) {
@@ -2658,22 +2623,13 @@
Object* proto = GetPrototype();
if (proto->IsNull()) return this;
ASSERT(proto->IsJSGlobalObject());
- return JSObject::cast(proto)->DefineAccessor(name,
- is_getter,
- fun,
- attributes,
- never_used);
+ return JSObject::cast(proto)->DefineAccessor(name, is_getter,
+ fun, attributes);
}
Object* array = DefineGetterSetter(name, attributes);
if (array->IsFailure() || array->IsUndefined()) return array;
FixedArray::cast(array)->set(is_getter ? 0 : 1, fun);
-
- // Because setters are nonlocal (they're accessible through the
- // prototype chain) but our inline caches are local we clear them
- // when a new setter is introduced.
- if (!(is_getter || never_used)) Heap::ClearStoreICs();
-
return this;
}
Modified: branches/bleeding_edge/src/objects.h
==============================================================================
--- branches/bleeding_edge/src/objects.h (original)
+++ branches/bleeding_edge/src/objects.h Thu Mar 19 08:06:00 2009
@@ -1200,13 +1200,8 @@
String* name);
PropertyAttributes GetLocalPropertyAttribute(String* name);
- // Defines an accessor. This may violate some of the assumptions we
- // make when setting up ics so unless it is guaranteed that no ics
- // exist for this property we have to clear all ics. Set
the 'never_used'
- // flag to true if you know there can be no ics.
Object* DefineAccessor(String* name, bool is_getter, JSFunction* fun,
- PropertyAttributes attributes, bool never_used);
-
+ PropertyAttributes attributes);
Object* LookupAccessor(String* name, bool is_getter);
// Used from Object::GetProperty().
@@ -1277,8 +1272,6 @@
// objects.
inline bool HasNamedInterceptor();
inline bool HasIndexedInterceptor();
-
- bool HasLocalPropertyWithType(PropertyType type);
// Support functions for v8 api (needed for correct interceptor
behavior).
bool HasRealNamedProperty(String* key);
Modified: branches/bleeding_edge/src/regexp-delay.js
==============================================================================
--- branches/bleeding_edge/src/regexp-delay.js (original)
+++ branches/bleeding_edge/src/regexp-delay.js Thu Mar 19 08:06:00 2009
@@ -356,15 +356,12 @@
LAST_INPUT(lastMatchInfo) = ToString(string);
};
- // All these accessors are set with the 'never_used' flag set to true.
- // This is because at this point there can be no existing ics for setting
- // these properties and so we don't have to bother clearing them.
%DefineAccessor($RegExp, 'input', GETTER, RegExpGetInput, DONT_DELETE);
- %DefineAccessor($RegExp, 'input', SETTER, RegExpSetInput, DONT_DELETE,
true);
+ %DefineAccessor($RegExp, 'input', SETTER, RegExpSetInput, DONT_DELETE);
%DefineAccessor($RegExp, '$_', GETTER, RegExpGetInput, DONT_ENUM |
DONT_DELETE);
- %DefineAccessor($RegExp, '$_', SETTER, RegExpSetInput, DONT_ENUM |
DONT_DELETE, true);
+ %DefineAccessor($RegExp, '$_', SETTER, RegExpSetInput, DONT_ENUM |
DONT_DELETE);
%DefineAccessor($RegExp, '$input', GETTER, RegExpGetInput, DONT_ENUM |
DONT_DELETE);
- %DefineAccessor($RegExp, '$input', SETTER, RegExpSetInput, DONT_ENUM |
DONT_DELETE, true);
+ %DefineAccessor($RegExp, '$input', SETTER, RegExpSetInput, DONT_ENUM |
DONT_DELETE);
// The properties multiline and $* are aliases for each other. When this
// value is set in SpiderMonkey, the value it is set to is coerced to a
@@ -379,9 +376,9 @@
function RegExpSetMultiline(flag) { multiline = flag ? true : false; };
%DefineAccessor($RegExp, 'multiline', GETTER, RegExpGetMultiline,
DONT_DELETE);
- %DefineAccessor($RegExp, 'multiline', SETTER, RegExpSetMultiline,
DONT_DELETE, true);
+ %DefineAccessor($RegExp, 'multiline', SETTER, RegExpSetMultiline,
DONT_DELETE);
%DefineAccessor($RegExp, '$*', GETTER, RegExpGetMultiline, DONT_ENUM |
DONT_DELETE);
- %DefineAccessor($RegExp, '$*', SETTER, RegExpSetMultiline, DONT_ENUM |
DONT_DELETE, true);
+ %DefineAccessor($RegExp, '$*', SETTER, RegExpSetMultiline, DONT_ENUM |
DONT_DELETE);
function NoOpSetter(ignored) {}
@@ -389,25 +386,25 @@
// Static properties set by a successful match.
%DefineAccessor($RegExp, 'lastMatch', GETTER, RegExpGetLastMatch,
DONT_DELETE);
- %DefineAccessor($RegExp, 'lastMatch', SETTER, NoOpSetter, DONT_DELETE,
true);
+ %DefineAccessor($RegExp, 'lastMatch', SETTER, NoOpSetter, DONT_DELETE);
%DefineAccessor($RegExp, '$&', GETTER, RegExpGetLastMatch, DONT_ENUM |
DONT_DELETE);
- %DefineAccessor($RegExp, '$&', SETTER, NoOpSetter, DONT_ENUM |
DONT_DELETE, true);
+ %DefineAccessor($RegExp, '$&', SETTER, NoOpSetter, DONT_ENUM |
DONT_DELETE);
%DefineAccessor($RegExp, 'lastParen', GETTER, RegExpGetLastParen,
DONT_DELETE);
- %DefineAccessor($RegExp, 'lastParen', SETTER, NoOpSetter, DONT_DELETE,
true);
+ %DefineAccessor($RegExp, 'lastParen', SETTER, NoOpSetter, DONT_DELETE);
%DefineAccessor($RegExp, '$+', GETTER, RegExpGetLastParen, DONT_ENUM |
DONT_DELETE);
- %DefineAccessor($RegExp, '$+', SETTER, NoOpSetter, DONT_ENUM |
DONT_DELETE, true);
+ %DefineAccessor($RegExp, '$+', SETTER, NoOpSetter, DONT_ENUM |
DONT_DELETE);
%DefineAccessor($RegExp, 'leftContext', GETTER, RegExpGetLeftContext,
DONT_DELETE);
- %DefineAccessor($RegExp, 'leftContext', SETTER, NoOpSetter, DONT_DELETE,
true);
+ %DefineAccessor($RegExp, 'leftContext', SETTER, NoOpSetter, DONT_DELETE);
%DefineAccessor($RegExp, '$`', GETTER, RegExpGetLeftContext, DONT_ENUM |
DONT_DELETE);
- %DefineAccessor($RegExp, '$`', SETTER, NoOpSetter, DONT_ENUM |
DONT_DELETE, true);
+ %DefineAccessor($RegExp, '$`', SETTER, NoOpSetter, DONT_ENUM |
DONT_DELETE);
%DefineAccessor($RegExp, 'rightContext', GETTER, RegExpGetRightContext,
DONT_DELETE);
- %DefineAccessor($RegExp, 'rightContext', SETTER, NoOpSetter,
DONT_DELETE, true);
+ %DefineAccessor($RegExp, 'rightContext', SETTER, NoOpSetter,
DONT_DELETE);
%DefineAccessor($RegExp, "$'", GETTER, RegExpGetRightContext, DONT_ENUM
| DONT_DELETE);
- %DefineAccessor($RegExp, "$'", SETTER, NoOpSetter, DONT_ENUM |
DONT_DELETE, true);
+ %DefineAccessor($RegExp, "$'", SETTER, NoOpSetter, DONT_ENUM |
DONT_DELETE);
for (var i = 1; i < 10; ++i) {
%DefineAccessor($RegExp, '$' + i, GETTER, RegExpMakeCaptureGetter(i),
DONT_DELETE);
- %DefineAccessor($RegExp, '$' + i, SETTER, NoOpSetter, DONT_DELETE,
true);
+ %DefineAccessor($RegExp, '$' + i, SETTER, NoOpSetter, DONT_DELETE);
}
}
Modified: branches/bleeding_edge/src/runtime.cc
==============================================================================
--- branches/bleeding_edge/src/runtime.cc (original)
+++ branches/bleeding_edge/src/runtime.cc Thu Mar 19 08:06:00 2009
@@ -5074,10 +5074,10 @@
// and setter on the first call to DefineAccessor and ignored on
// subsequent calls.
static Object* Runtime_DefineAccessor(Arguments args) {
- RUNTIME_ASSERT(4 <= args.length() && args.length() <= 6);
+ RUNTIME_ASSERT(args.length() == 4 || args.length() == 5);
// Compute attributes.
PropertyAttributes attributes = NONE;
- if (args.length() >= 5) {
+ if (args.length() == 5) {
CONVERT_CHECKED(Smi, attrs, args[4]);
int value = attrs->value();
// Only attribute bits should be set.
@@ -5085,17 +5085,11 @@
attributes = static_cast<PropertyAttributes>(value);
}
- bool never_used = (args.length() == 6) && (args[5] ==
Heap::true_value());
-
CONVERT_CHECKED(JSObject, obj, args[0]);
CONVERT_CHECKED(String, name, args[1]);
CONVERT_CHECKED(Smi, flag, args[2]);
CONVERT_CHECKED(JSFunction, fun, args[3]);
- return obj->DefineAccessor(name,
- flag->value() == 0,
- fun,
- attributes,
- never_used);
+ return obj->DefineAccessor(name, flag->value() == 0, fun, attributes);
}
Modified: branches/bleeding_edge/src/v8-counters.h
==============================================================================
--- branches/bleeding_edge/src/v8-counters.h (original)
+++ branches/bleeding_edge/src/v8-counters.h Thu Mar 19 08:06:00 2009
@@ -122,8 +122,7 @@
SC(enum_cache_misses, V8.EnumCacheMisses) \
SC(reloc_info_count, V8.RelocInfoCount) \
SC(reloc_info_size, V8.RelocInfoSize) \
- SC(zone_segment_bytes, V8.ZoneSegmentBytes) \
- SC(clear_store_ic, V8.ClearStoreIC)
+ SC(zone_segment_bytes, V8.ZoneSegmentBytes)
// This file contains all the v8 counters that are in use.
--~--~---------~--~----~------------~-------~--~----~
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
-~----------~----~----~----~------~----~------~--~---
