Reviewers: Toon Verwaest,
Description:
Restructure JSObject::SetElement for performance.
Wins back ~1500 points on Octane/Gameboy that we lost with
https://codereview.chromium.org/11365111 (CL 12900), presumably
by lowering register pressure and/or handlification overhead.
Hopefully benefits other regressions as well.
[email protected]
BUG=
Please review this at https://codereview.chromium.org/11275283/
SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files:
M src/objects.cc
Index: src/objects.cc
diff --git a/src/objects.cc b/src/objects.cc
index
f8d080d676399d0eea6aebff3fc8ef0f0744d8ae..a03054653aa16cf014d8ffd83783a7ccd4d7c223
100644
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -10267,25 +10267,21 @@ MaybeObject* JSObject::SetElement(uint32_t index,
bool check_prototype,
SetPropertyMode set_mode) {
Isolate* isolate = GetIsolate();
- HandleScope scope(isolate);
- Handle<JSObject> self(this);
- Handle<Object> value(value_raw);
// Check access rights if needed.
if (IsAccessCheckNeeded()) {
- Heap* heap = GetHeap();
- if (!heap->isolate()->MayIndexedAccess(*self, index, v8::ACCESS_SET)) {
- heap->isolate()->ReportFailedAccessCheck(*self, v8::ACCESS_SET);
- return *value;
+ if (!isolate->MayIndexedAccess(this, index, v8::ACCESS_SET)) {
+ isolate->ReportFailedAccessCheck(this, v8::ACCESS_SET);
+ return value_raw;
}
}
if (IsJSGlobalProxy()) {
Object* proto = GetPrototype();
- if (proto->IsNull()) return *value;
+ if (proto->IsNull()) return value_raw;
ASSERT(proto->IsJSGlobalObject());
return JSObject::cast(proto)->SetElement(index,
- *value,
+ value_raw,
attributes,
strict_mode,
check_prototype,
@@ -10295,7 +10291,7 @@ MaybeObject* JSObject::SetElement(uint32_t index,
// Don't allow element properties to be redefined for external arrays.
if (HasExternalArrayElements() && set_mode == DEFINE_PROPERTY) {
Handle<Object> number = isolate->factory()->NewNumberFromUint(index);
- Handle<Object> args[] = { self, number };
+ Handle<Object> args[] = { handle(this), number };
Handle<Object> error = isolate->factory()->NewTypeError(
"redef_external_array_element", HandleVector(args,
ARRAY_SIZE(args)));
return isolate->Throw(*error);
@@ -10310,23 +10306,27 @@ MaybeObject* JSObject::SetElement(uint32_t index,
dictionary->set_requires_slow_elements();
}
+ if (!(FLAG_harmony_observation && map()->is_observed())) {
+ return HasIndexedInterceptor()
+ ? SetElementWithInterceptor(
+ index, value_raw, attributes, strict_mode, check_prototype,
set_mode)
+ : SetElementWithoutInterceptor(
+ index, value_raw, attributes, strict_mode, check_prototype,
set_mode);
+ }
+
// From here on, everything has to be handlified.
- Handle<String> name;
- Handle<Object> old_value(isolate->heap()->the_hole_value());
- Handle<Object> old_array_length;
- PropertyAttributes old_attributes = ABSENT;
- bool preexists = false;
- if (FLAG_harmony_observation && map()->is_observed()) {
- name = isolate->factory()->Uint32ToString(index);
- preexists = self->HasLocalElement(index);
- if (preexists) {
- old_attributes = self->GetLocalPropertyAttribute(*name);
- // TODO(observe): only read & set old_value if we have a data
property
- old_value = Object::GetElement(self, index);
- } else if (self->IsJSArray()) {
- // Store old array length in case adding an element grows the array.
- old_array_length = handle(Handle<JSArray>::cast(self)->length());
- }
+ Handle<JSObject> self(this);
+ Handle<Object> value(value_raw);
+ PropertyAttributes old_attributes =
self->GetLocalElementAttribute(index);
+ Handle<Object> old_value = isolate->factory()->the_hole_value();
+ Handle<Object> old_length;
+
+ if (old_attributes != ABSENT) {
+ // TODO(observe): only read & set old_value if we have a data property
+ old_value = Object::GetElement(self, index);
+ } else if (self->IsJSArray()) {
+ // Store old array length in case adding an element grows the array.
+ old_length = handle(Handle<JSArray>::cast(self)->length());
}
// Check for lookup interceptor
@@ -10339,23 +10339,19 @@ MaybeObject* JSObject::SetElement(uint32_t index,
Handle<Object> hresult;
if (!result->ToHandle(&hresult)) return result;
- if (FLAG_harmony_observation && map()->is_observed()) {
- PropertyAttributes new_attributes =
self->GetLocalPropertyAttribute(*name);
- if (!preexists) {
- EnqueueChangeRecord(self, "new", name, old_value);
- if (self->IsJSArray() &&
- !old_array_length->SameValue(Handle<JSArray>::cast(self)->length()))
{
- EnqueueChangeRecord(self, "updated",
- isolate->factory()->length_symbol(),
- old_array_length);
- }
- } else if (new_attributes != old_attributes || old_value->IsTheHole())
{
- EnqueueChangeRecord(self, "reconfigured", name, old_value);
- } else {
- Handle<Object> new_value = Object::GetElement(self, index);
- if (!new_value->SameValue(*old_value))
- EnqueueChangeRecord(self, "updated", name, old_value);
- }
+ Handle<String> name = isolate->factory()->Uint32ToString(index);
+ PropertyAttributes new_attributes =
self->GetLocalElementAttribute(index);
+ if (old_attributes == ABSENT) {
+ EnqueueChangeRecord(self, "new", name, old_value);
+ if (self->IsJSArray() &&
+ !old_length->SameValue(Handle<JSArray>::cast(self)->length())) {
+ EnqueueChangeRecord(
+ self, "updated", isolate->factory()->length_symbol(),
old_length);
+ }
+ } else if (old_attributes != new_attributes || old_value->IsTheHole()) {
+ EnqueueChangeRecord(self, "reconfigured", name, old_value);
+ } else if (!old_value->SameValue(*Object::GetElement(self, index))) {
+ EnqueueChangeRecord(self, "updated", name, old_value);
}
return *hresult;
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
