Fast copy between TypedData and ExternalTypedData

runtime/lib/typeddata.cc

Index: runtime/lib/typeddata.cc
diff --git a/runtime/lib/typeddata.cc b/runtime/lib/typeddata.cc
index d9859751826042829d86f41cc4353fe26c166478..9719664a57e5346714247ac65ceca349e7ffd03b 100644
--- a/runtime/lib/typeddata.cc
+++ b/runtime/lib/typeddata.cc
@@ -78,25 +78,37 @@ DEFINE_NATIVE_ENTRY(TypedData_length, 1) {
   return Integer::null();
 }
 
-
-#define COPY_DATA(type, dst, src)                                              \
-  const type& dst_array = type::Cast(dst);                                     \
-  const type& src_array = type::Cast(src);                                     \
-  intptr_t element_size_in_bytes = dst_array.ElementSizeInBytes();             \
-  intptr_t length_in_bytes = length.Value() * element_size_in_bytes;           \
-  intptr_t src_offset_in_bytes = src_start.Value() * element_size_in_bytes;    \
-  intptr_t dst_offset_in_bytes = dst_start.Value() * element_size_in_bytes;    \
-  SetRangeCheck(src_offset_in_bytes,                                           \
-                length_in_bytes,                                               \
-                src_array.LengthInBytes(),                                     \
-                element_size_in_bytes);                                        \
-  SetRangeCheck(dst_offset_in_bytes,                                           \
-                length_in_bytes,                                               \
-                dst_array.LengthInBytes(),                                     \
-                element_size_in_bytes);                                        \
-  type::Copy(dst_array, dst_offset_in_bytes,                                   \
-             src_array, src_offset_in_bytes,                                   \
-             length_in_bytes);
+template <typename DstType, typename SrcType>
+static RawBool* CopyData(const Instance& dst, const Instance& src,
+                         const Smi& dst_start, const Smi& src_start,
+                         const Smi& length) {
+  const DstType& dst_array = DstType::Cast(dst);
+  const SrcType& src_array = SrcType::Cast(src);
+  intptr_t element_size_in_bytes = dst_array.ElementSizeInBytes();
+  intptr_t dst_offset_in_bytes = dst_start.Value() * element_size_in_bytes;
+  intptr_t src_offset_in_bytes = src_start.Value() * element_size_in_bytes;
+  intptr_t length_in_bytes = length.Value() * element_size_in_bytes;
+  if (dst_array.ElementType() == src_array.ElementType()) {

[Anders Johnsen, 2013/04/18 11:00:43]

DBC: !=

[kustermann, 2013/04/18 13:57:57]

That was really stupid of me.

+    return Bool::False().raw();
+  }
+  SetRangeCheck(src_offset_in_bytes,
+                length_in_bytes,
+                src_array.LengthInBytes(),
+                element_size_in_bytes);
+  SetRangeCheck(dst_offset_in_bytes,
+                length_in_bytes,
+                dst_array.LengthInBytes(),
+                element_size_in_bytes);
+  {
+    NoGCScope no_gc;

[Vyacheslav Egorov (Google), 2013/04/17 11:41:56]

If TypedData and ExternalTypedData had a base class that class could have hosted
all this functionality.

I have no strong opinion on this though.

I can see that if we'll move this code object.cc that would lead to tons of
duplication.

Maybe Siva has an idea?

[kustermann, 2013/04/18 13:57:57]

I agree that a base class would solve the code duplication issue. But since I'm
not familiar with the VM code, I'd rather keep my change small.

I moved now this piece of code to another function template in object.h.

+    if (length_in_bytes > 0) {
+      memmove(dst_array.DataAddr(dst_offset_in_bytes),
+              src_array.DataAddr(src_offset_in_bytes),
+              length_in_bytes);
+    }
+    return Bool::True().raw();
+  }
+}
 
 DEFINE_NATIVE_ENTRY(TypedData_setRange, 5) {
   GET_NON_NULL_NATIVE_ARGUMENT(Instance, dst, arguments->NativeArgAt(0));
@@ -112,17 +124,22 @@ DEFINE_NATIVE_ENTRY(TypedData_setRange, 5) {
     args.SetAt(0, error);
     Exceptions::ThrowByType(Exceptions::kArgument, args);
   }
-  if ((dst.IsTypedData() || dst.IsExternalTypedData()) &&
-      (dst.clazz() == src.clazz())) {
-    if (dst.IsTypedData()) {
-      ASSERT(src.IsTypedData());
-      COPY_DATA(TypedData, dst, src);
-    } else {
-      ASSERT(src.IsExternalTypedData());
-      ASSERT(dst.IsExternalTypedData());
-      COPY_DATA(ExternalTypedData, dst, src);
+  if (dst.IsTypedData()) {
+    if (src.IsTypedData()) {
+      return CopyData<TypedData, TypedData>(
+          dst, src, dst_start, src_start, length);
+    } else if (src.IsExternalTypedData()) {
+      return CopyData<TypedData, ExternalTypedData>(
+          dst, src, dst_start, src_start, length);
+    }
+  } else if (dst.IsExternalTypedData()) {
+    if (src.IsTypedData()) {
+      return CopyData<ExternalTypedData, TypedData>(
+          dst, src, dst_start, src_start, length);
+    } else if (src.IsExternalTypedData()) {
+      return CopyData<ExternalTypedData, ExternalTypedData>(
+          dst, src, dst_start, src_start, length);
     }
-    return Bool::True().raw();
   }
   return Bool::False().raw();
 }