From: rossberg@chromium.org <rossberg@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Date: Tue, 13 Nov 2012 15:47:46 +0000 (+0000)
Subject: Restructure JSObject::SetElement for performance.
X-Git-Tag: upstream/4.7.83~15665
X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=dcb6abd460526ebba7b709b59854e262c6256e6b;p=platform%2Fupstream%2Fv8.git

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.

R=verwaest@chromium.org
BUG=

Review URL: https://codereview.chromium.org/11275283

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@12949 ce2b1a6d-e550-0410-aec6-3dcde31c8c00
---

diff --git a/src/objects.cc b/src/objects.cc
index f8d080d..a030546 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;