Create string efficiently from Uint16List/View.

runtime/lib/string.cc

Index: runtime/lib/string.cc
diff --git a/runtime/lib/string.cc b/runtime/lib/string.cc
index 5b450f4a71adf581e2bf87f04e07698e365f575d..0bdd1627bb025b079f268810e15b722a95c2c414 100644
--- a/runtime/lib/string.cc
+++ b/runtime/lib/string.cc
@@ -258,6 +258,110 @@ DEFINE_NATIVE_ENTRY(ExternalOneByteString_getCid, 0) {
 }
 
 
+DEFINE_NATIVE_ENTRY(TwoByteString_allocateFromTwoByteList, 3) {
+  Instance& list = Instance::CheckedHandle(arguments->NativeArgAt(0));
+  GET_NON_NULL_NATIVE_ARGUMENT(Smi, start_obj, arguments->NativeArgAt(1));
+  GET_NON_NULL_NATIVE_ARGUMENT(Smi, end_obj, arguments->NativeArgAt(2));
+
+  intptr_t start = start_obj.Value();
+  intptr_t end = end_obj.Value();
+  if (start < 0) {
+    const Array& args = Array::Handle(Array::New(1));
+    args.SetAt(0, start_obj);
+    Exceptions::ThrowByType(Exceptions::kArgument, args);
+  }
+  intptr_t length = end - start;
+  if (length < 0) {
+    const Array& args = Array::Handle(Array::New(1));
+    args.SetAt(0, end_obj);
+    Exceptions::ThrowByType(Exceptions::kArgument, args);
+  }
+  ASSERT(length >= 0);

[siva, 2015/01/20 23:50:26]

Not sure why this assertion is needed here considering we have a strong check
above for length which results in a throw.

[Lasse Reichstein Nielsen, 2015/01/22 07:16:32]

ACK, I added the explicit check later, and forgot to remove the assert.

+
+  Heap::Space space = isolate->heap()->SpaceForAllocation(kTwoByteStringCid);
+  if (list.IsTypedData()) {
+    const TypedData& array = TypedData::Cast(list);
+    if (array.ElementType() != kUint16ArrayElement) {
+      const Array& args = Array::Handle(Array::New(1));
+      args.SetAt(0, list);
+      Exceptions::ThrowByType(Exceptions::kArgument, args);
+    }
+    if (end > array.LengthInBytes()) {
+      const Array& args = Array::Handle(Array::New(1));
+      args.SetAt(0, end_obj);
+      Exceptions::ThrowByType(Exceptions::kArgument, args);
+    }

[siva, 2015/01/20 23:50:26]

Maybe you could abstract this code out into a static helper function
static void CheckTypeAndLength(bool type_check, bool length_check) {
  if (!type_ok) {
    const Array& args = Array::Handle(Array::New(1));
    args.SetAt(0, list);
    Exceptions::ThrowByType(Exceptions::kArgument, args);
  }
  if (!length_ok) {
    const Array& args = Array::Handle(Array::New(1));
    args.SetAt(0, end_obj);
    Exceptions::ThrowByType(Exceptions::kArgument, args);
  }
}

and use it as
if (list.IsTypedData()) {
  const TypedData& array = TypedData::Cast(list);
  CheckTypeAndLength((array.ElementType() == kUint16ArrayElement),
                     (end <= array.LengthInBytes());
  return TwoByteString::New(array, start * sizeof(uint16_t), length, space);
} else if (....) {
  ....
  CheckTypeAndLength((array.ElementType() == kUint16ArrayElement),
                     (end <= array.LengthInBytes());
  ....
}

[Lasse Reichstein Nielsen, 2015/01/22 07:16:32]

The function also needs the list and end_obj values, so calls get a little
bigger.
I'll start out by using Exception::ThrowArgumentError instead of these larger
blocks, which should make the code more manageable.

+    return TwoByteString::New(array, start * sizeof(uint16_t), length, space);
+  } else if (list.IsExternalTypedData()) {
+    const ExternalTypedData& array = ExternalTypedData::Cast(list);
+    if (array.ElementType() != kUint16ArrayElement) {
+      const Array& args = Array::Handle(Array::New(1));
+      args.SetAt(0, list);
+      Exceptions::ThrowByType(Exceptions::kArgument, args);
+    }
+    if (end > array.LengthInBytes()) {
+      const Array& args = Array::Handle(Array::New(1));
+      args.SetAt(0, end_obj);
+      Exceptions::ThrowByType(Exceptions::kArgument, args);
+    }
+    return TwoByteString::New(array, start * sizeof(uint16_t), length, space);
+  } else if (RawObject::IsTypedDataViewClassId(list.GetClassId())) {
+    const intptr_t cid = list.GetClassId();
+    if (cid != kTypedDataUint16ArrayViewCid) {
+      const Array& args = Array::Handle(Array::New(1));
+      args.SetAt(0, list);
+      Exceptions::ThrowByType(Exceptions::kArgument, args);
+    }
+    if (end > Smi::Value(TypedDataView::Length(list))) {
+      const Array& args = Array::Handle(Array::New(1));
+      args.SetAt(0, end_obj);
+      Exceptions::ThrowByType(Exceptions::kArgument, args);
+    }
+    const Instance& data_obj = Instance::Handle(TypedDataView::Data(list));
+    intptr_t data_offset = Smi::Value(TypedDataView::OffsetInBytes(list));
+    if (data_obj.IsTypedData()) {
+      const TypedData& array = TypedData::Cast(data_obj);
+      return TwoByteString::New(array, data_offset + start * sizeof(uint16_t),
+                                length, space);
+    } else if (data_obj.IsExternalTypedData()) {
+      const ExternalTypedData& array = ExternalTypedData::Cast(data_obj);
+      return TwoByteString::New(array, data_offset + start * sizeof(uint16_t),
+                                length, space);
+    }
+  } else if (list.IsArray()) {
+    const Array& array = Array::Cast(list);
+    if (end > array.Length()) {
+      const Array& args = Array::Handle(Array::New(1));
+      args.SetAt(0, end_obj);
+      Exceptions::ThrowByType(Exceptions::kArgument, args);
+    }
+    String& string = String::Handle(TwoByteString::New(length, space));

[siva, 2015/01/20 23:50:26]

Since 'isolate' is available to you in the native call use it when creating
handles, e.g:
const String& string = String::Handle(isolate, TwoByteString::New(length,
space));

Also missing 'const'

[Lasse Reichstein Nielsen, 2015/01/22 07:16:32]

Done.

+    for (int i = 0; i < length; i++) {
+      intptr_t value =
+          Smi::Value(reinterpret_cast<RawSmi*>(array.At(start + i)));
+      TwoByteString::SetCharAt(string, i, value);
+    }
+    return string.raw();
+  } else if (list.IsGrowableObjectArray()) {
+    const GrowableObjectArray& array = GrowableObjectArray::Cast(list);
+    if (end > array.Length()) {
+      const Array& args = Array::Handle(Array::New(1));
+      args.SetAt(0, end_obj);
+      Exceptions::ThrowByType(Exceptions::kArgument, args);
+    }
+    String& string = String::Handle(TwoByteString::New(length, space));

[siva, 2015/01/20 23:50:26]

Ditto.

[Lasse Reichstein Nielsen, 2015/01/22 07:16:32]

Done.

+    for (int i = 0; i < length; i++) {
+      intptr_t value =
+          Smi::Value(reinterpret_cast<RawSmi*>(array.At(start + i)));
+      TwoByteString::SetCharAt(string, i, value);
+    }
+    return string.raw();
+  }
+  UNREACHABLE();
+  return Object::null();
+}
+
+
 DEFINE_NATIVE_ENTRY(String_getHashCode, 1) {
   const String& receiver = String::CheckedHandle(arguments->NativeArgAt(0));
   intptr_t hash_val = receiver.Hash();