[vm] Remove external strings

This change removes support for external strings from the VM along with
Dart_NewExternalLatin1String, Dart_NewExternalUTF16String and
Dart_IsExternalString Dart C API functions.

External strings are not used by the VM nor any known embedder, but
Dart VM was paying the maintenance and performance price for
the external string implementation classes.

TEST=ci

Change-Id: I094cd2d2b7ec0840e9f09e1ca9e5a7acd4e78c28
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/358760
Reviewed-by: Ryan Macnak <[email protected]>
Reviewed-by: Siva Annamalai <[email protected]>
Commit-Queue: Alexander Markov <[email protected]>

Author: alexmarkov

CHANGELOG.md

@@ -124,6 +124,7 @@ advantage of these improvements, set your package's
 [#53785]: https://github.com/dart-lang/sdk/issues/53785
 
 ### Dart Runtime
+
 - Dart VM flags and options can now be provided to any executable
   generated using `dart compile exe` via the `DART_VM_OPTIONS` environment
   variable. `DART_VM_OPTIONS` should be set to a list of comma-separated flags
@@ -137,6 +138,10 @@ advantage of these improvements, set your package's
   DART_VM_OPTIONS=--random_seed=42,--verbose_gc
   ```
 
+- Dart VM no longer supports external strings: `Dart_IsExternalString`,
+  `Dart_NewExternalLatin1String` and `Dart_NewExternalUTF16String` functions are
+  removed from Dart C API.
+
 ### Tools
 
 #### DevTools

pkg/vm_service/test/regexp_function_test.dart

@@ -62,8 +62,6 @@ final tests = <IsolateTest>[
         case {
           '_oneByteFunction': {'type': '@Function'},
           '_twoByteFunction': {'type': '@Function'},
-          '_externalOneByteFunction': {'type': '@Function'},
-          '_externalTwoByteFunction': {'type': '@Function'},
         }) {
       // Running with compiled regexp.
     } else {

runtime/include/dart_api.h

@@ -1926,7 +1926,6 @@ DART_EXPORT bool Dart_IsDouble(Dart_Handle object);
 DART_EXPORT bool Dart_IsBoolean(Dart_Handle object);
 DART_EXPORT bool Dart_IsString(Dart_Handle object);
 DART_EXPORT bool Dart_IsStringLatin1(Dart_Handle object); /* (ISO-8859-1) */
-DART_EXPORT bool Dart_IsExternalString(Dart_Handle object);
 DART_EXPORT bool Dart_IsList(Dart_Handle object);
 DART_EXPORT bool Dart_IsMap(Dart_Handle object);
 DART_EXPORT bool Dart_IsLibrary(Dart_Handle object);
@@ -2293,48 +2292,6 @@ DART_EXPORT Dart_Handle Dart_NewStringFromUTF16(const uint16_t* utf16_array,
 DART_EXPORT Dart_Handle Dart_NewStringFromUTF32(const int32_t* utf32_array,
                                                 intptr_t length);
 
-/**
- * Returns a String which references an external array of
- * Latin-1 (ISO-8859-1) encoded characters.
- *
- * \param latin1_array Array of Latin-1 encoded characters. This must not move.
- * \param length The length of the characters array.
- * \param peer An external pointer to associate with this string.
- * \param external_allocation_size The number of externally allocated
- *   bytes for peer. Used to inform the garbage collector.
- * \param callback A callback to be called when this string is finalized.
- *
- * \return The String object if no error occurs. Otherwise returns
- *   an error handle.
- */
-DART_EXPORT Dart_Handle
-Dart_NewExternalLatin1String(const uint8_t* latin1_array,
-                             intptr_t length,
-                             void* peer,
-                             intptr_t external_allocation_size,
-                             Dart_HandleFinalizer callback);
-
-/**
- * Returns a String which references an external array of UTF-16 encoded
- * characters.
- *
- * \param utf16_array An array of UTF-16 encoded characters. This must not move.
- * \param length The length of the characters array.
- * \param peer An external pointer to associate with this string.
- * \param external_allocation_size The number of externally allocated
- *   bytes for peer. Used to inform the garbage collector.
- * \param callback A callback to be called when this string is finalized.
- *
- * \return The String object if no error occurs. Otherwise returns
- *   an error handle.
- */
-DART_EXPORT Dart_Handle
-Dart_NewExternalUTF16String(const uint16_t* utf16_array,
-                            intptr_t length,
-                            void* peer,
-                            intptr_t external_allocation_size,
-                            Dart_HandleFinalizer callback);
-
 /**
  * Gets the C string representation of a String.
  * (It is a sequence of UTF-8 encoded values with a '\0' termination.)

runtime/lib/string.cc

@@ -104,19 +104,13 @@ DEFINE_NATIVE_ENTRY(StringBase_substringUnchecked, 0, 3) {
 static uint16_t CharacterLimit(const String& string,
                                intptr_t start,
                                intptr_t end) {
-  ASSERT(string.IsTwoByteString() || string.IsExternalTwoByteString());
+  ASSERT(string.IsTwoByteString());
   // Maybe do loop unrolling, and handle two uint16_t in a single uint32_t
   // operation.
   NoSafepointScope no_safepoint;
   uint16_t result = 0;
-  if (string.IsTwoByteString()) {
-    for (intptr_t i = start; i < end; i++) {
-      result |= TwoByteString::CharAt(string, i);
-    }
-  } else {
-    for (intptr_t i = start; i < end; i++) {
-      result |= ExternalTwoByteString::CharAt(string, i);
-    }
+  for (intptr_t i = start; i < end; i++) {
+    result |= TwoByteString::CharAt(string, i);
   }
   return result;
 }
@@ -180,7 +174,7 @@ DEFINE_NATIVE_ENTRY(StringBase_joinReplaceAllResult, 0, 4) {
   if (is_onebyte) {
     // If any of the base string slices are not one-byte, the result will be
     // a two-byte string.
-    if (!base.IsOneByteString() && !base.IsExternalOneByteString()) {
+    if (!base.IsOneByteString()) {
       is_onebyte = CheckSlicesOneByte(base, matches, len);
     }
   }

runtime/lib/vmservice.cc

@@ -324,11 +324,6 @@ static void ContentsFinalizer(void* isolate_callback_data, void* peer) {
   delete[] data;
 }
 
-static void FilenameFinalizer(void* isolate_callback_data, void* peer) {
-  char* filename = reinterpret_cast<char*>(peer);
-  delete[] filename;
-}
-
 #endif
 
 DEFINE_NATIVE_ENTRY(VMService_DecodeAssets, 0, 1) {
@@ -365,9 +360,8 @@ DEFINE_NATIVE_ENTRY(VMService_DecodeAssets, 0, 1) {
       uint8_t* contents = archive.NextContent();
       intptr_t contents_length = archive.NextContentLength();
 
-      Dart_Handle dart_filename = Dart_NewExternalLatin1String(
-          reinterpret_cast<uint8_t*>(filename), filename_length, filename,
-          filename_length, FilenameFinalizer);
+      Dart_Handle dart_filename = Dart_NewStringFromUTF8(
+          reinterpret_cast<uint8_t*>(filename), filename_length);
       ASSERT(!Dart_IsError(dart_filename));
 
       Dart_Handle dart_contents = Dart_NewExternalTypedDataWithFinalizer(

runtime/observatory/lib/src/elements/instance_view.dart

@@ -442,14 +442,6 @@ class InstanceViewElement extends CustomElement implements Renderable {
       if (_instance.twoByteFunction != null) {
         members.add(member('twoByteFunction', _instance.twoByteFunction));
       }
-      if (_instance.externalOneByteFunction != null) {
-        members.add(member(
-            'externalOneByteFunction', _instance.externalOneByteFunction));
-      }
-      if (_instance.externalTwoByteFunction != null) {
-        members.add(member(
-            'externalTwoByteFunction', _instance.externalTwoByteFunction));
-      }
       if (_instance.oneByteBytecode != null) {
         members.add(member('oneByteBytecode', _instance.oneByteBytecode));
       }

runtime/observatory/lib/src/models/objects/instance.dart

@@ -449,18 +449,6 @@ abstract class Instance extends Object implements InstanceRef {
   ///   RegExp
   FunctionRef? get twoByteFunction;
 
-  /// [optional]
-  ///
-  /// Provided for instance kinds:
-  ///   RegExp
-  FunctionRef? get externalOneByteFunction;
-
-  /// [optional]
-  ///
-  /// Provided for instance kinds:
-  ///   RegExp
-  FunctionRef? get externalTwoByteFunction;
-
   /// [optional]
   ///
   /// Provided for instance kinds:

runtime/observatory/lib/src/service/object.dart

@@ -2896,8 +2896,6 @@ class Instance extends HeapObject implements M.Instance {
   Breakpoint? activationBreakpoint; // If a Closure.
   ServiceFunction? oneByteFunction; // If a RegExp.
   ServiceFunction? twoByteFunction; // If a RegExp.
-  ServiceFunction? externalOneByteFunction; // If a RegExp.
-  ServiceFunction? externalTwoByteFunction; // If a RegExp.
   Instance? oneByteBytecode; // If a RegExp.
   Instance? twoByteBytecode; // If a RegExp.
   bool? isCaseSensitive; // If a RegExp.
@@ -2984,10 +2982,6 @@ class Instance extends HeapObject implements M.Instance {
     bool isCompiled = map['_oneByteFunction'] is ServiceFunction;
     oneByteFunction = isCompiled ? map['_oneByteFunction'] : null;
     twoByteFunction = isCompiled ? map['_twoByteFunction'] : null;
-    externalOneByteFunction =
-        isCompiled ? map['_externalOneByteFunction'] : null;
-    externalTwoByteFunction =
-        isCompiled ? map['_externalTwoByteFunction'] : null;
     oneByteBytecode = map['_oneByteBytecode'];
     twoByteBytecode = map['_twoByteBytecode'];
 

runtime/observatory/tests/service/regexp_function_test.dart

@@ -51,10 +51,6 @@ var tests = <IsolateTest>[
       expect(f1 is ServiceFunction, isTrue);
       var f2 = await regex.twoByteFunction!.load();
       expect(f2 is ServiceFunction, isTrue);
-      var f3 = await regex.externalOneByteFunction!.load();
-      expect(f3 is ServiceFunction, isTrue);
-      var f4 = await regex.externalTwoByteFunction!.load();
-      expect(f4 is ServiceFunction, isTrue);
     }
   }
 ];

runtime/tests/vm/dart/exported_symbols_test.dart

@@ -168,7 +168,6 @@ main() {
     "Dart_IsCompilationError",
     "Dart_IsDouble",
     "Dart_IsError",
-    "Dart_IsExternalString",
     "Dart_IsFatalError",
     "Dart_IsFunction",
     "Dart_IsFuture",
@@ -238,10 +237,8 @@ main() {
     "Dart_NewByteBuffer",
     "Dart_NewCompilationError",
     "Dart_NewDouble",
-    "Dart_NewExternalLatin1String",
     "Dart_NewExternalTypedData",
     "Dart_NewExternalTypedDataWithFinalizer",
-    "Dart_NewExternalUTF16String",
     "Dart_NewFinalizableHandle",
     "Dart_NewInteger",
     "Dart_NewIntegerFromHexCString",

runtime/vm/benchmark_test.cc

@@ -205,47 +205,6 @@ void benchmark(int count) {
   benchmark->set_score(elapsed_time);
 }
 
-static void NoopFinalizer(void* isolate_callback_data, void* peer) {}
-
-//
-// Measure time accessing internal and external strings.
-//
-BENCHMARK(DartStringAccess) {
-  const int kNumIterations = 10000000;
-  Timer timer;
-  timer.Start();
-  Dart_EnterScope();
-
-  // Create strings.
-  uint8_t data8[] = {'o', 'n', 'e', 0xFF};
-  int external_peer_data = 123;
-  intptr_t char_size;
-  intptr_t str_len;
-  Dart_Handle external_string = Dart_NewExternalLatin1String(
-      data8, ARRAY_SIZE(data8), &external_peer_data, sizeof(data8),
-      NoopFinalizer);
-  Dart_Handle internal_string = NewString("two");
-
-  // Run benchmark.
-  for (int64_t i = 0; i < kNumIterations; i++) {
-    EXPECT(Dart_IsString(internal_string));
-    EXPECT(!Dart_IsExternalString(internal_string));
-    EXPECT_VALID(external_string);
-    EXPECT(Dart_IsExternalString(external_string));
-    void* external_peer = nullptr;
-    EXPECT_VALID(Dart_StringGetProperties(external_string, &char_size, &str_len,
-                                          &external_peer));
-    EXPECT_EQ(1, char_size);
-    EXPECT_EQ(4, str_len);
-    EXPECT_EQ(&external_peer_data, external_peer);
-  }
-
-  Dart_ExitScope();
-  timer.Stop();
-  int64_t elapsed_time = timer.TotalElapsedTime();
-  benchmark->set_score(elapsed_time);
-}
-
 static void vmservice_resolver(Dart_NativeArguments args) {}
 
 static Dart_NativeFunction NativeResolver(Dart_Handle name,

runtime/vm/class_finalizer.cc

@@ -255,10 +255,6 @@ void ClassFinalizer::VerifyBootstrapClasses() {
   ASSERT_EQUAL(OneByteString::InstanceSize(), cls.host_instance_size());
   cls = object_store->two_byte_string_class();
   ASSERT_EQUAL(TwoByteString::InstanceSize(), cls.host_instance_size());
-  cls = object_store->external_one_byte_string_class();
-  ASSERT_EQUAL(ExternalOneByteString::InstanceSize(), cls.host_instance_size());
-  cls = object_store->external_two_byte_string_class();
-  ASSERT_EQUAL(ExternalTwoByteString::InstanceSize(), cls.host_instance_size());
   cls = object_store->double_class();
   ASSERT_EQUAL(Double::InstanceSize(), cls.host_instance_size());
   cls = object_store->bool_class();

runtime/vm/class_id.h

@@ -132,9 +132,7 @@ static constexpr intptr_t kClassIdTagMax = (1 << 20) - 1;
 #define CLASS_LIST_STRINGS(V)                                                  \
   V(String)                                                                    \
   V(OneByteString)                                                             \
-  V(TwoByteString)                                                             \
-  V(ExternalOneByteString)                                                     \
-  V(ExternalTwoByteString)
+  V(TwoByteString)
 
 #define CLASS_LIST_TYPED_DATA(V)                                               \
   V(Int8Array)                                                                 \
@@ -274,7 +272,6 @@ bool IsNumberClassId(intptr_t index);
 bool IsIntegerClassId(intptr_t index);
 bool IsStringClassId(intptr_t index);
 bool IsOneByteStringClassId(intptr_t index);
-bool IsExternalStringClassId(intptr_t index);
 bool IsBuiltinListClassId(intptr_t index);
 bool IsTypeClassId(intptr_t index);
 bool IsTypedDataBaseClassId(intptr_t index);
@@ -348,21 +345,14 @@ inline bool IsIntegerClassId(intptr_t index) {
 
 // Make sure this check is updated when new StringCid types are added.
 COMPILE_ASSERT(kOneByteStringCid == kStringCid + 1 &&
-               kTwoByteStringCid == kStringCid + 2 &&
-               kExternalOneByteStringCid == kStringCid + 3 &&
-               kExternalTwoByteStringCid == kStringCid + 4);
+               kTwoByteStringCid == kStringCid + 2);
 
 inline bool IsStringClassId(intptr_t index) {
-  return (index >= kStringCid && index <= kExternalTwoByteStringCid);
+  return (index >= kStringCid && index <= kTwoByteStringCid);
 }
 
 inline bool IsOneByteStringClassId(intptr_t index) {
-  return (index == kOneByteStringCid || index == kExternalOneByteStringCid);
-}
-
-inline bool IsExternalStringClassId(intptr_t index) {
-  return (index == kExternalOneByteStringCid ||
-          index == kExternalTwoByteStringCid);
+  return (index == kOneByteStringCid);
 }
 
 inline bool IsArrayClassId(intptr_t index) {

runtime/vm/compiler/asm_intrinsifier_arm.cc

@@ -975,15 +975,15 @@ static void JumpIfString(Assembler* assembler,
                          Register cid,
                          Register tmp,
                          Label* target) {
-  assembler->RangeCheck(cid, tmp, kOneByteStringCid, kExternalTwoByteStringCid,
+  assembler->RangeCheck(cid, tmp, kOneByteStringCid, kTwoByteStringCid,
                         Assembler::kIfInRange, target);
 }
 
 static void JumpIfNotString(Assembler* assembler,
                             Register cid,
                             Register tmp,
                             Label* target) {
-  assembler->RangeCheck(cid, tmp, kOneByteStringCid, kExternalTwoByteStringCid,
+  assembler->RangeCheck(cid, tmp, kOneByteStringCid, kTwoByteStringCid,
                         Assembler::kIfNotInRange, target);
 }
 

runtime/vm/compiler/asm_intrinsifier_arm64.cc

@@ -1131,15 +1131,15 @@ static void JumpIfString(Assembler* assembler,
                          Register cid,
                          Register tmp,
                          Label* target) {
-  assembler->RangeCheck(cid, tmp, kOneByteStringCid, kExternalTwoByteStringCid,
+  assembler->RangeCheck(cid, tmp, kOneByteStringCid, kTwoByteStringCid,
                         Assembler::kIfInRange, target);
 }
 
 static void JumpIfNotString(Assembler* assembler,
                             Register cid,
                             Register tmp,
                             Label* target) {
-  assembler->RangeCheck(cid, tmp, kOneByteStringCid, kExternalTwoByteStringCid,
+  assembler->RangeCheck(cid, tmp, kOneByteStringCid, kTwoByteStringCid,
                         Assembler::kIfNotInRange, target);
 }
 

runtime/vm/compiler/asm_intrinsifier_ia32.cc

@@ -1100,15 +1100,13 @@ static void JumpIfNotInteger(Assembler* assembler,
 }
 
 static void JumpIfString(Assembler* assembler, Register cid, Label* target) {
-  assembler->RangeCheck(cid, kNoRegister, kOneByteStringCid,
-                        kExternalTwoByteStringCid, Assembler::kIfInRange,
-                        target);
+  assembler->RangeCheck(cid, kNoRegister, kOneByteStringCid, kTwoByteStringCid,
+                        Assembler::kIfInRange, target);
 }
 
 static void JumpIfNotString(Assembler* assembler, Register cid, Label* target) {
-  assembler->RangeCheck(cid, kNoRegister, kOneByteStringCid,
-                        kExternalTwoByteStringCid, Assembler::kIfNotInRange,
-                        target);
+  assembler->RangeCheck(cid, kNoRegister, kOneByteStringCid, kTwoByteStringCid,
+                        Assembler::kIfNotInRange, target);
 }
 
 static void JumpIfNotList(Assembler* assembler, Register cid, Label* target) {