From 65bca53b6e656d43c60ccaa808ad84a74a35da12 Mon Sep 17 00:00:00 2001
From: Martin Kustermann <kustermann@google.com>
Date: Mon, 14 Jun 2021 14:41:33 +0000
Subject: [PATCH] [vm] Avoid going to runtime for String.split()

This makes the small benchmark on the github issue more than 2.5x
faster.

Closes https://github.com/dart-lang/sdk/issues/46352

TEST=Existing test suite.

Change-Id: I82c53b3553e04f2afe23606b09b3199cb9b6a926
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/203502
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Tess Strickland <sstrickl@google.com>
---
 runtime/lib/string.cc                      | 29 ----------------------
 runtime/vm/bootstrap_natives.h             |  1 -
 runtime/vm/object_test.cc                  |  2 +-
 sdk/lib/_internal/vm/lib/string_patch.dart | 15 +++++++++--
 4 files changed, 14 insertions(+), 33 deletions(-)

diff --git a/runtime/lib/string.cc b/runtime/lib/string.cc
index 996d92d26d8..9fb91ee176d 100644
--- a/runtime/lib/string.cc
+++ b/runtime/lib/string.cc
@@ -257,35 +257,6 @@ DEFINE_NATIVE_ENTRY(OneByteString_substringUnchecked, 0, 3) {
   return OneByteString::New(receiver, start, end - start, Heap::kNew);
 }
 
-// This is high-performance code.
-DEFINE_NATIVE_ENTRY(OneByteString_splitWithCharCode, 0, 2) {
-  const String& receiver =
-      String::CheckedHandle(zone, arguments->NativeArgAt(0));
-  ASSERT(receiver.IsOneByteString());
-  GET_NON_NULL_NATIVE_ARGUMENT(Smi, smi_split_code, arguments->NativeArgAt(1));
-  const intptr_t len = receiver.Length();
-  const intptr_t split_code = smi_split_code.Value();
-  const GrowableObjectArray& result = GrowableObjectArray::Handle(
-      zone, GrowableObjectArray::New(16, Heap::kNew));
-  String& str = String::Handle(zone);
-  intptr_t start = 0;
-  intptr_t i = 0;
-  for (; i < len; i++) {
-    if (split_code == OneByteString::CharAt(receiver, i)) {
-      str = OneByteString::SubStringUnchecked(receiver, start, (i - start),
-                                              Heap::kNew);
-      result.Add(str);
-      start = i + 1;
-    }
-  }
-  str = OneByteString::SubStringUnchecked(receiver, start, (i - start),
-                                          Heap::kNew);
-  result.Add(str);
-  result.SetTypeArguments(TypeArguments::Handle(
-      zone, isolate->group()->object_store()->type_argument_string()));
-  return result.ptr();
-}
-
 DEFINE_NATIVE_ENTRY(Internal_allocateOneByteString, 0, 1) {
   GET_NON_NULL_NATIVE_ARGUMENT(Integer, length_obj, arguments->NativeArgAt(0));
   const int64_t length = length_obj.AsInt64Value();
diff --git a/runtime/vm/bootstrap_natives.h b/runtime/vm/bootstrap_natives.h
index 359d482f5d5..373dffbaba5 100644
--- a/runtime/vm/bootstrap_natives.h
+++ b/runtime/vm/bootstrap_natives.h
@@ -130,7 +130,6 @@ namespace dart {
   V(StringBase_joinReplaceAllResult, 4)                                        \
   V(StringBuffer_createStringFromUint16Array, 3)                               \
   V(OneByteString_substringUnchecked, 3)                                       \
-  V(OneByteString_splitWithCharCode, 2)                                        \
   V(OneByteString_allocateFromOneByteList, 3)                                  \
   V(TwoByteString_allocateFromTwoByteList, 3)                                  \
   V(String_getHashCode, 1)                                                     \
diff --git a/runtime/vm/object_test.cc b/runtime/vm/object_test.cc
index de6a83778b7..70256cf750f 100644
--- a/runtime/vm/object_test.cc
+++ b/runtime/vm/object_test.cc
@@ -4511,7 +4511,7 @@ ISOLATE_UNIT_TEST_CASE(PrintJSONPrimitives) {
         "vmName\":\"\",\"location\":{\"type\":\"SourceLocation\",\"script\":{"
         "\"type\":\"@Script\",\"fixedId\":true,\"id\":\"\",\"uri\":\"dart:core-"
         "patch\\/string_patch.dart\",\"_kind\":\"kernel\"},\"tokenPos\":32310,"
-        "\"endTokenPos\":44332},\"library\":{\"type\":\"@Library\",\"fixedId\":"
+        "\"endTokenPos\":44599},\"library\":{\"type\":\"@Library\",\"fixedId\":"
         "true,\"id\":\"\",\"name\":\"dart.core\",\"uri\":\"dart:core\"}},"
         "\"identityHashCode\":0,\"kind\":\"String\",\"id\":\"\",\"length\":2,"
         "\"valueAsString\":\"dw\"}",
diff --git a/sdk/lib/_internal/vm/lib/string_patch.dart b/sdk/lib/_internal/vm/lib/string_patch.dart
index a8b949a0bea..be6b8581742 100644
--- a/sdk/lib/_internal/vm/lib/string_patch.dart
+++ b/sdk/lib/_internal/vm/lib/string_patch.dart
@@ -985,8 +985,19 @@ class _OneByteString extends _StringBase {
   String _substringUncheckedNative(int startIndex, int endIndex)
       native "OneByteString_substringUnchecked";
 
-  List<String> _splitWithCharCode(int charCode)
-      native "OneByteString_splitWithCharCode";
+  List<String> _splitWithCharCode(int charCode) {
+    final parts = <String>[];
+    int i = 0;
+    int start = 0;
+    for (i = 0; i < this.length; ++i) {
+      if (this.codeUnitAt(i) == charCode) {
+        parts.add(this._substringUnchecked(start, i));
+        start = i + 1;
+      }
+    }
+    parts.add(this._substringUnchecked(start, i));
+    return parts;
+  }
 
   List<String> split(Pattern pattern) {
     // TODO(vegorov) investigate if this can be rewritten as `is _OneByteString`