Reviewers: Mads Ager,

Message:
Mads,

may you have a look?

Description:
Trim underlying fixed array by one element from the left when doing shift.

For now this trick is only done to objects in new space, see comments
for reasons.

Please review this at http://codereview.chromium.org/1076010

Affected files:
  M src/builtins.cc


Index: src/builtins.cc
diff --git a/src/builtins.cc b/src/builtins.cc
index 91cb1515200ac8555ed181cfb9de15ed926848ff..98f7452b28b81d5578b4c4f5f3308a0adf5f4ead 100644
--- a/src/builtins.cc
+++ b/src/builtins.cc
@@ -443,6 +443,39 @@ BUILTIN(ArrayPop) {
 }


+static FixedArray* LeftTrimFixedArray(FixedArray* elms) {
+  // For now this trick is only applied to fixed arrays in new space.
+  // In large object space object's start must coincide with chunk
+  // and thus the trick is just not applicable.
+  // In old space I'd rather not play rset tricks like
+  // set RSet for fixed array length which is plain int.
+  ASSERT(Heap::new_space()->Contains(elms));
+
+  Object** former_map =
+      HeapObject::RawField(elms, FixedArray::kMapOffset);
+  Object** former_length =
+      HeapObject::RawField(elms, FixedArray::kLengthOffset);
+  Object** former_first =
+      HeapObject::RawField(elms, FixedArray::kHeaderSize);
+  // Check that we don't forget to copy all the bits.
+  STATIC_ASSERT(FixedArray::kMapOffset + 2 * kPointerSize
+      == FixedArray::kHeaderSize);
+
+  int len = elms->length();
+
+  *former_first = reinterpret_cast<Object*>(len - 1);
+  *former_length = Heap::fixed_array_map();
+  // Technically in new space this write might be omitted (except for
+  // debug mode which iterates through the heap), but to play safer
+  // we still do it.
+  *former_map = Heap::raw_unchecked_one_pointer_filler_map();
+
+  ASSERT(elms->address() + kPointerSize
+      == (elms + kPointerSize)->address());
+  return elms + kPointerSize;
+}
+
+
 BUILTIN(ArrayShift) {
   Object* receiver = *args.receiver();
   FixedArray* elms = NULL;
@@ -462,10 +495,14 @@ BUILTIN(ArrayShift) {
     first = Heap::undefined_value();
   }

-  // Shift the elements.
-  AssertNoAllocation no_gc;
-  MoveElements(&no_gc, elms, 0, elms, 1, len - 1);
-  elms->set(len - 1, Heap::the_hole_value());
+  if (Heap::new_space()->Contains(elms)) {
+    array->set_elements(LeftTrimFixedArray(elms));
+  } else {
+    // Shift the elements.
+    AssertNoAllocation no_gc;
+    MoveElements(&no_gc, elms, 0, elms, 1, len - 1);
+    elms->set(len - 1, Heap::the_hole_value());
+  }

   // Set the length.
   array->set_length(Smi::FromInt(len - 1));


--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev

To unsubscribe from this group, send email to v8-dev+unsubscribegooglegroups.com or reply 
to this email with the words "REMOVE ME" as the subject.

Reply via email to