Improve performance of Dart_ListGetAsBytes.

runtime/vm/dart_api_impl.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 "include/dart_api.h"
 #include "include/dart_mirrors_api.h"
 #include "include/dart_native_api.h"
 
 #include "platform/assert.h"
 #include "vm/bigint_operations.h"
@@ skipping 2118 matching lines @@
       }                                                                        \
       const Integer& integer = Integer::Cast(element);                         \
       native_array[i] = static_cast<uint8_t>(integer.AsInt64Value() & 0xff);   \
       ASSERT(integer.AsInt64Value() <= 0xff);                                  \
     }                                                                          \
     return Api::Success();                                                     \
   }                                                                            \
   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,
                                             intptr_t length) {
   Isolate* isolate = Isolate::Current();
   DARTSCOPE(isolate);
   const Object& obj = Object::Handle(isolate, Api::UnwrapHandle(list));
   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()) {
     GET_LIST_ELEMENT_AS_BYTES(isolate,
                               Array,
                               obj,
                               native_array,
                               offset,
                               length);
   }
@@ skipping 19 matching lines @@
         Array::Handle(ArgumentsDescriptor::New(kNumArgs)));
     const Function& function = Function::Handle(
         isolate,
         Resolver::ResolveDynamic(instance, Symbols::IndexToken(), args_desc));
     if (!function.IsNull()) {
       Object& result = Object::Handle(isolate);
       Integer& intobj = Integer::Handle(isolate);
       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);
         if (result.IsError()) {
           return Api::NewHandle(isolate, result.raw());
         }
         if (!result.IsInteger()) {
           return Api::NewError("%s expects the argument 'list' to be "
                                "a List of int", CURRENT_FUNC);
         }
@@ skipping 1903 matching lines @@
   }
   {
     NoGCScope no_gc;
     RawObject* raw_obj = obj.raw();
     isolate->heap()->SetPeer(raw_obj, peer);
   }
   return Api::Success();
 }
 
 }  // namespace dart