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..8aba36a86156af27c3b59d33f490ffb6cc0d8a95 100644
--- a/runtime/lib/typeddata.cc
+++ b/runtime/lib/typeddata.cc
@@ -78,25 +78,38 @@ 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 dst_element_size_in_bytes = dst_array.ElementSizeInBytes();
+  intptr_t src_element_size_in_bytes = src_array.ElementSizeInBytes();
+  intptr_t dst_offset_in_bytes = dst_start.Value() * dst_element_size_in_bytes;
+  intptr_t src_offset_in_bytes = src_start.Value() * src_element_size_in_bytes;
+  intptr_t length_in_bytes = length.Value() * src_element_size_in_bytes;
+  if (src_element_size_in_bytes != dst_element_size_in_bytes) {

[Vyacheslav Egorov (Google), 2013/04/16 16:23:06]

You need to compare element types.

[kustermann, 2013/04/16 17:26:04]

Done.

+    return Bool::False().raw();
+  }

[siva, 2013/04/16 16:36:48]

I don't believe this check is sufficient, if dst is Uint8ClampedArray and src is
one of Int8Array or Uint8Array you would get all wrong results.

[kustermann, 2013/04/16 17:26:04]

I added now an ElementType function to TypedData/ExternalTypedData (the same way
we have a ElementSizeInBytes function).

+  SetRangeCheck(src_offset_in_bytes,
+                length_in_bytes,
+                src_array.LengthInBytes(),
+                src_element_size_in_bytes);
+  SetRangeCheck(dst_offset_in_bytes,
+                length_in_bytes,
+                dst_array.LengthInBytes(),
+                dst_element_size_in_bytes);
+  {
+    NoGCScope no_gc;
+    if (length_in_bytes > 0) {
+      memmove(dst_array.DataAddr(dst_offset_in_bytes),
+              src_array.DataAddr(src_offset_in_bytes),
+              length_in_bytes);
+    }

[siva, 2013/04/16 16:36:48]

Why does this have to be explicitly listed here instead of calling the Copy
function that is provided in object.cc

We would like to minimize the spread of these NoGCScope regions all over the
code base as far as possible.

I would change this to call the appropriate Copy function ....

[kustermann, 2013/04/16 17:26:04]

The reason for this is that the appropriate Copy functions don't accept
arguments of the other type. So 
ExternalTypedData::Copy(external_typed_array, .., typed_array, ...) doesn't
work:

void ExternalTypedData::Copy(const ExternalTypedData& dst, ...)
void TypedData::Copy(const TypedData& dst, ...)

But we could move the CopyData function to object.cc?

+    return Bool::True().raw();
+  }
+}
 
 DEFINE_NATIVE_ENTRY(TypedData_setRange, 5) {
   GET_NON_NULL_NATIVE_ARGUMENT(Instance, dst, arguments->NativeArgAt(0));
@@ -112,17 +125,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();
 }