Remove redudant deleted_count argument from EnqueueSpliceRecord

src/objects.cc

 // Copyright 2013 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 10827 matching lines @@
   PropertyAttributes attributes = object->GetLocalElementAttribute(index);
   ASSERT(attributes != ABSENT);
   if (attributes == DONT_DELETE) return false;
   old_values->Add(object->GetLocalElementAccessorPair(index) == NULL
       ? Object::GetElement(object, index)
       : Handle<Object>::cast(isolate->factory()->the_hole_value()));
   indices->Add(index);
   return true;
 }
 
-
-// TODO(rafaelw): Remove |delete_count| argument and rely on the length of
-// of |deleted|.
 static void EnqueueSpliceRecord(Handle<JSArray> object,
                                 uint32_t index,
                                 Handle<JSArray> deleted,
-                                uint32_t delete_count,
                                 uint32_t add_count) {
   Isolate* isolate = object->GetIsolate();
   HandleScope scope(isolate);
   Handle<Object> index_object = isolate->factory()->NewNumberFromUint(index);
-  Handle<Object> delete_count_object =
-      isolate->factory()->NewNumberFromUint(delete_count);
   Handle<Object> add_count_object =
       isolate->factory()->NewNumberFromUint(add_count);
 
   Handle<Object> args[] =
-      { object, index_object, deleted, delete_count_object, add_count_object };
+      { object, index_object, deleted, add_count_object };
 
   bool threw;
   Execution::Call(Handle<JSFunction>(isolate->observers_enqueue_splice()),
                   isolate->factory()->undefined_value(), ARRAY_SIZE(args), args,
                   &threw);
   ASSERT(!threw);
 }
 
 
 static void BeginPerformSplice(Handle<JSArray> object) {
@@ skipping 83 matching lines @@
   JSObject::EnqueueChangeRecord(
       self, "updated", isolate->factory()->length_string(),
       old_length_handle);
 
   EndPerformSplice(self);
 
   uint32_t index = Min(old_length, new_length);
   uint32_t add_count = new_length > old_length ? new_length - old_length : 0;
   uint32_t delete_count = new_length < old_length ? old_length - new_length : 0;
   Handle<JSArray> deleted = isolate->factory()->NewJSArray(0);
   if (delete_count) {

[rossberg, 2013/06/06 09:12:05]

Nit: can we make this del_cnt > 0?

[rafaelw, 2013/06/06 18:44:41]

Done.

     for (int i = indices.length() - 1; i >= 0; i--) {
       JSObject::SetElement(deleted, indices[i] - index, old_values[i], NONE,
                            kNonStrictMode);
     }
   }
 
-  EnqueueSpliceRecord(self, index, deleted, delete_count, add_count);
+  SetProperty(deleted, isolate->factory()->length_string(),

[adamk, 2013/06/05 17:22:03]

This can go inside the if (delete_count) block.

I also wonder if the thing you actually want is a static, handlified version of
JSArray::SetElementsLength, which will short-circuit the whole property-setting
machinery.

[rafaelw, 2013/06/06 18:44:41]

Done.
Yeah. Let's wait and see if this shows up as a bottle neck.

+              isolate->factory()->NewNumberFromUint(delete_count),
+              NONE, kNonStrictMode);
+  EnqueueSpliceRecord(self, index, deleted, add_count);
 
   return *hresult;
 }
 
 
 Map* Map::GetPrototypeTransition(Object* prototype) {
   FixedArray* cache = GetPrototypeTransitions();
   int number_of_transitions = NumberOfProtoTransitions();
   const int proto_offset =
       kProtoTransitionHeaderSize + kProtoTransitionPrototypeOffset;
@@ skipping 1045 matching lines @@
       uint32_t new_length = 0;
       CHECK(old_length_handle->ToArrayIndex(&old_length));
       CHECK(new_length_handle->ToArrayIndex(&new_length));
 
       BeginPerformSplice(Handle<JSArray>::cast(self));
       EnqueueChangeRecord(self, "new", name, old_value);
       EnqueueChangeRecord(self, "updated", isolate->factory()->length_string(),
                           old_length_handle);
       EndPerformSplice(Handle<JSArray>::cast(self));
       Handle<JSArray> deleted = isolate->factory()->NewJSArray(0);
-      EnqueueSpliceRecord(Handle<JSArray>::cast(self), old_length, deleted, 0,
+      EnqueueSpliceRecord(Handle<JSArray>::cast(self), old_length, deleted,
                           new_length - old_length);
     } else {
       EnqueueChangeRecord(self, "new", name, old_value);
     }
   } else if (old_value->IsTheHole()) {
     EnqueueChangeRecord(self, "reconfigured", name, old_value);
   } else {
     Handle<Object> new_value = Object::GetElement(self, index);
     bool value_changed = !old_value->SameValue(*new_value);
     if (old_attributes != new_attributes) {
@@ skipping 3605 matching lines @@
   set_year(Smi::FromInt(year), SKIP_WRITE_BARRIER);
   set_month(Smi::FromInt(month), SKIP_WRITE_BARRIER);
   set_day(Smi::FromInt(day), SKIP_WRITE_BARRIER);
   set_weekday(Smi::FromInt(weekday), SKIP_WRITE_BARRIER);
   set_hour(Smi::FromInt(hour), SKIP_WRITE_BARRIER);
   set_min(Smi::FromInt(min), SKIP_WRITE_BARRIER);
   set_sec(Smi::FromInt(sec), SKIP_WRITE_BARRIER);
 }
 
 } }  // namespace v8::internal