Fast copy between TypedData and ExternalTypedData

runtime/lib/typeddata.cc

 // Copyright (c) 2013, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 #include "vm/bootstrap_natives.h"
 
 #include "include/dart_api.h"
 
 #include "vm/bigint_operations.h"
 #include "vm/exceptions.h"
@@ skipping 60 matching lines @@
     return Smi::New(array.Length());
   }
   const String& error = String::Handle(String::NewFormatted(
       "Expected a TypedData object but found %s", instance.ToCString()));
   const Array& args = Array::Handle(Array::New(1));
   args.SetAt(0, error);
   Exceptions::ThrowByType(Exceptions::kArgument, args);
   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));
   GET_NON_NULL_NATIVE_ARGUMENT(Smi, dst_start, arguments->NativeArgAt(1));
   GET_NON_NULL_NATIVE_ARGUMENT(Smi, length, arguments->NativeArgAt(2));
   GET_NON_NULL_NATIVE_ARGUMENT(Instance, src, arguments->NativeArgAt(3));
   GET_NON_NULL_NATIVE_ARGUMENT(Smi, src_start, arguments->NativeArgAt(4));
 
   if (length.Value() < 0) {
     const String& error = String::Handle(String::NewFormatted(
         "length (%"Pd") must be non-negative", length.Value()));
     const Array& args = Array::Handle(Array::New(1));
     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);
     }
-    return Bool::True().raw();
+  } 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::False().raw();
 }
 
 
 // We check the length parameter against a possible maximum length for the
 // array based on available physical addressable memory on the system. The
 // maximum possible length is a scaled value of kSmiMax which is set up based
 // on whether the underlying architecture is 32-bit or 64-bit.
 #define TYPED_DATA_NEW(name)                                                   \
@@ skipping 165 matching lines @@
 TYPED_DATA_NATIVES(Uint16Array, GetUint16, SetUint16, Smi, Value)
 TYPED_DATA_NATIVES(Int32Array, GetInt32, SetInt32, Integer, AsInt64Value)
 TYPED_DATA_NATIVES(Uint32Array, GetUint32, SetUint32, Integer, AsInt64Value)
 TYPED_DATA_NATIVES(Int64Array, GetInt64, SetInt64, Integer, AsInt64Value)
 TYPED_DATA_UINT64_NATIVES(Uint64Array, GetUint64, SetUint64, Integer)
 TYPED_DATA_NATIVES(Float32Array, GetFloat32, SetFloat32, Double, value)
 TYPED_DATA_NATIVES(Float64Array, GetFloat64, SetFloat64, Double, value)
 TYPED_DATA_NATIVES(Float32x4Array, GetFloat32x4, SetFloat32x4, Float32x4, value)
 
 }  // namespace dart