Improve performance of Dart_ListGetAsBytes.

runtime/vm/dart_api_impl.cc

Index: runtime/vm/dart_api_impl.cc
===================================================================
--- runtime/vm/dart_api_impl.cc	(revision 25704)
+++ runtime/vm/dart_api_impl.cc	(working copy)
@@ -2136,6 +2136,23 @@
   return Api::NewError("Invalid length passed in to access array elements");   \
 
 
+static Dart_Handle CopyBytes(const TypedData& array,
+                             intptr_t offset,
+                             uint8_t* native_array,
+                             intptr_t length) {
+  ASSERT(array.IsTypedData());
+  ASSERT(array.ElementSizeInBytes() == 1);
+  if (!Utils::RangeCheck(offset, length, array.Length())) {
+    return Api::NewError("Invalid length passed in to access list elements");
+  }
+  NoGCScope no_gc;
+  memmove(native_array,
+          reinterpret_cast<uint8_t*>(array.DataAddr(offset)),
+          length);
+  return Api::Success();
+}
+
+
 DART_EXPORT Dart_Handle Dart_ListGetAsBytes(Dart_Handle list,
                                             intptr_t offset,
                                             uint8_t* native_array,
@@ -2146,14 +2163,26 @@
   if (obj.IsTypedData()) {
     const TypedData& array = TypedData::Cast(obj);
     if (array.ElementSizeInBytes() == 1) {
-      if (Utils::RangeCheck(offset, length, array.Length())) {
-        NoGCScope no_gc;
-        memmove(native_array,
-                reinterpret_cast<uint8_t*>(array.DataAddr(offset)),
-                length);
-        return Api::Success();
+      return CopyBytes(array, offset, native_array, length);
+    }
+  }
+  if (RawObject::IsTypedDataViewClassId(obj.GetClassId())) {
+    const Instance& view = Instance::Cast(obj);
+    if (TypedDataView::ElementSizeInBytes(view) == 1) {
+      intptr_t view_length = Smi::Value(TypedDataView::Length(view));
+      if (!Utils::RangeCheck(offset, length, view_length)) {
+        return Api::NewError(
+            "Invalid length passed in to access list elements");
       }
-      return Api::NewError("Invalid length passed in to access list elements");
+      const Instance& data = Instance::Handle(TypedDataView::Data(view));
+      intptr_t data_offset =
+          Smi::Value(TypedDataView::OffsetInBytes(view)) + offset;

[siva, 2013/08/01 16:29:31]

This data_offset computation can be moved inside the
if (array.ElementSizeInBytes() == 1) { ... } block.

or you could compute this value early and reuse the obj handle for
'data'.

[Florian Schneider, 2013/08/02 11:07:33]

Done.

+      if (data.IsTypedData()) {
+        const TypedData& array = TypedData::Cast(data);
+        if (array.ElementSizeInBytes() == 1) {
+          return CopyBytes(array, data_offset, native_array, length);

[siva, 2013/08/01 16:29:31]

We end up doing the range check twice in this case, maybe we
could move the Range check out of CopyBytes and do the RangeCheck
outside for the TypedData case too.

[Florian Schneider, 2013/08/02 11:07:33]

Agree.  The second range check is redundant because there is already one
performed on the length of the TypedDataView object and a view is always within
the bounds of the underlying buffer.

I left it here as a safe-guard, but instead I will turn it into an ASSERT.

+        }
+      }
     }
   }
   if (obj.IsArray()) {
@@ -2193,6 +2222,7 @@
       const Array& args = Array::Handle(isolate, Array::New(kNumArgs));
       args.SetAt(0, instance);  // Set up the receiver as the first argument.
       for (int i = 0; i < length; i++) {
+        HandleScope handle_scope(isolate);

[siva, 2013/08/01 16:29:31]

Could you use HANDLESCOPE(isolate); here to be consistent with the Usage
pattern.

[Florian Schneider, 2013/08/02 11:07:33]

Done.

         intobj = Integer::New(offset + i);
         args.SetAt(1, intobj);
         result = DartEntry::InvokeFunction(function, args);