Reviewers: mvstanton,

Message:
PTAL

Description:
Unify ElementsAccessor Array Builtins
Repeat the same patterns for future refactoring

BUG=

Please review this at https://codereview.chromium.org/1302413006/

Base URL: https://chromium.googlesource.com/v8/v8.git@2015-08-28_elements_accessor_unshift

Affected files (+29, -40 lines):
  M src/builtins.cc
  M src/elements.h
  M src/elements.cc


Index: src/builtins.cc
diff --git a/src/builtins.cc b/src/builtins.cc
index 5ae9a5e94f5107c35a53861d63ae43b23ab38d46..a84d15015c4e3aa01ea5f7cb2cf780b42ad80249 100644
--- a/src/builtins.cc
+++ b/src/builtins.cc
@@ -371,8 +371,7 @@ BUILTIN(ArrayPush) {
   }
   DCHECK(!array->map()->is_observed());
   ElementsAccessor* accessor = array->GetElementsAccessor();
-  int new_length = accessor->Push(array, elms_obj, &args[1], push_size,
-                                  ElementsAccessor::kDirectionReverse);
+  int new_length = accessor->Push(array, elms_obj, &args, push_size);
   return Smi::FromInt(new_length);
 }

@@ -622,7 +621,7 @@ BUILTIN(ArraySplice) {
   }
   ElementsAccessor* accessor = array->GetElementsAccessor();
   Handle<JSArray> result_array = accessor->Splice(
-      array, elms_obj, actual_start, actual_delete_count, args, add_count);
+ array, elms_obj, actual_start, actual_delete_count, &args, add_count);
   return *result_array;
 }

Index: src/elements.cc
diff --git a/src/elements.cc b/src/elements.cc
index 15515113b90e0d75ffad40ff6a0aca2477e11884..e83e4ec2c42f59defeb7b8c11d7f19e021107ae1 100644
--- a/src/elements.cc
+++ b/src/elements.cc
@@ -602,15 +602,15 @@ class ElementsAccessorBase : public ElementsAccessor {
   }

   virtual uint32_t Push(Handle<JSArray> receiver,
- Handle<FixedArrayBase> backing_store, Object** objects,
-                        uint32_t push_size, int direction) final {
- return ElementsAccessorSubclass::PushImpl(receiver, backing_store, objects,
-                                              push_size, direction);
+ Handle<FixedArrayBase> backing_store, Arguments* args,
+                        uint32_t push_size) final {
+ return ElementsAccessorSubclass::PushImpl(receiver, backing_store, args,
+                                              push_size);
   }

   static uint32_t PushImpl(Handle<JSArray> receiver,
- Handle<FixedArrayBase> elms_obj, Object** objects,
-                           uint32_t push_size, int direction) {
+ Handle<FixedArrayBase> elms_obj, Arguments* args,
+                           uint32_t push_sized) {
     UNREACHABLE();
     return 0;
   }
@@ -646,7 +646,7 @@ class ElementsAccessorBase : public ElementsAccessor {
   virtual Handle<JSArray> Splice(Handle<JSArray> receiver,
                                  Handle<FixedArrayBase> backing_store,
                                  uint32_t start, uint32_t delete_count,
- Arguments args, uint32_t add_count) final { + Arguments* args, uint32_t add_count) final { return ElementsAccessorSubclass::SpliceImpl(receiver, backing_store, start, delete_count, args, add_count);
   }
@@ -654,7 +654,7 @@ class ElementsAccessorBase : public ElementsAccessor {
   static Handle<JSArray> SpliceImpl(Handle<JSArray> receiver,
                                     Handle<FixedArrayBase> backing_store,
                                     uint32_t start, uint32_t delete_count,
-                                    Arguments args, uint32_t add_count) {
+                                    Arguments* args, uint32_t add_count) {
     UNREACHABLE();
     return Handle<JSArray>();
   }
@@ -1276,8 +1276,7 @@ class FastElementsAccessor

   static uint32_t PushImpl(Handle<JSArray> receiver,
                            Handle<FixedArrayBase> backing_store,
-                           Object** objects, uint32_t push_size,
-                           int direction) {
+                           Arguments* args, uint32_t push_size) {
     uint32_t len = Smi::cast(receiver->length())->value();
     if (push_size == 0) {
       return len;
@@ -1287,34 +1286,25 @@ class FastElementsAccessor
     // we should never hit this case.
     DCHECK(push_size <= static_cast<uint32_t>(Smi::kMaxValue - len));
     uint32_t new_length = len + push_size;
-    Handle<FixedArrayBase> new_elms;

     if (new_length > elms_len) {
       // New backing storage is needed.
       uint32_t capacity = new_length + (new_length >> 1) + 16;
-      new_elms = FastElementsAccessorSubclass::ConvertElementsWithCapacity(
+ backing_store = FastElementsAccessorSubclass::ConvertElementsWithCapacity(
           receiver, backing_store, KindTraits::Kind, capacity);
-    } else {
- // push_size is > 0 and new_length <= elms_len, so backing_store cannot be
-      // the empty_fixed_array.
-      new_elms = backing_store;
+      receiver->set_elements(*backing_store);
     }

     // Add the provided values.
     DisallowHeapAllocation no_gc;
-    DCHECK(direction == ElementsAccessor::kDirectionForward ||
-           direction == ElementsAccessor::kDirectionReverse);
-    STATIC_ASSERT(ElementsAccessor::kDirectionForward == 1);
-    STATIC_ASSERT(ElementsAccessor::kDirectionReverse == -1);
+    FixedArrayBase* raw_backing_store = *backing_store;
+    WriteBarrierMode mode = raw_backing_store->GetWriteBarrierMode(no_gc);
     for (uint32_t index = 0; index < push_size; index++) {
-      int offset = direction * index;
-      Object* object = objects[offset];
- FastElementsAccessorSubclass::SetImpl(*new_elms, index + len, object);
-    }
-    if (!new_elms.is_identical_to(backing_store)) {
-      receiver->set_elements(*new_elms);
+      Object* object = (*args)[index + 1];
+      FastElementsAccessorSubclass::SetImpl(raw_backing_store, index + len,
+                                            object, mode);
     }
-    DCHECK(*new_elms == receiver->elements());
+    DCHECK(*backing_store == receiver->elements());
     // Set the length.
     receiver->set_length(Smi::FromInt(new_length));
     return new_length;
@@ -1389,7 +1379,7 @@ class FastElementsAccessor
   static Handle<JSArray> SpliceImpl(Handle<JSArray> receiver,
                                     Handle<FixedArrayBase> backing_store,
                                     uint32_t start, uint32_t delete_count,
-                                    Arguments args, uint32_t add_count) {
+                                    Arguments* args, uint32_t add_count) {
     Isolate* isolate = receiver->GetIsolate();
     Heap* heap = isolate->heap();
     uint32_t len = Smi::cast(receiver->length())->value();
@@ -1425,9 +1415,12 @@ class FastElementsAccessor

     // Copy new Elements from args
     DisallowHeapAllocation no_gc;
-    for (uint32_t index = start; index < start + add_count; index++) {
-      Object* arg = args[3 + index - start];
-      FastElementsAccessorSubclass::SetImpl(*backing_store, index, arg);
+    FixedArrayBase* raw_backing_store = *backing_store;
+    WriteBarrierMode mode = raw_backing_store->GetWriteBarrierMode(no_gc);
+    for (uint32_t index = 0; index < add_count; index++) {
+      Object* object = (*args)[3 + index];
+ FastElementsAccessorSubclass::SetImpl(raw_backing_store, index + start,
+                                            object, mode);
     }

     if (elms_changed) {
Index: src/elements.h
diff --git a/src/elements.h b/src/elements.h
index 540be05060b13bf28761a409d03712999381d8e9..33ce6cb780c31e925b03923ed8469bd3f2091ca8 100644
--- a/src/elements.h
+++ b/src/elements.h
@@ -65,9 +65,6 @@ class ElementsAccessor {
   // destination array with the hole.
   static const int kCopyToEndAndInitializeToHole = -2;

-  static const int kDirectionForward = 1;
-  static const int kDirectionReverse = -1;
-
// Copy elements from one backing store to another. Typically, callers specify // the source JSObject or JSArray in source_holder. If the holder's backing
   // store is available, it can be passed in source and source_holder is
@@ -133,8 +130,8 @@ class ElementsAccessor {
// TODO(cbruni): Consider passing Arguments* instead of Object** depending on
   // the requirements of future callers.
   virtual uint32_t Push(Handle<JSArray> receiver,
- Handle<FixedArrayBase> backing_store, Object** objects,
-                        uint32_t start, int direction) = 0;
+ Handle<FixedArrayBase> backing_store, Arguments* args,
+                        uint32_t push_size) = 0;

   virtual uint32_t Unshift(Handle<JSArray> receiver,
                            Handle<FixedArrayBase> backing_store,
@@ -147,7 +144,7 @@ class ElementsAccessor {
   virtual Handle<JSArray> Splice(Handle<JSArray> receiver,
                                  Handle<FixedArrayBase> backing_store,
                                  uint32_t start, uint32_t delete_count,
-                                 Arguments args, uint32_t add_count) = 0;
+                                 Arguments* args, uint32_t add_count) = 0;

   virtual Handle<Object> Pop(Handle<JSArray> receiver,
                              Handle<FixedArrayBase> backing_store) = 0;


--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to