Skip to content

Commit 20fc102

Browse files
mkustermannCommit Queue
mkustermann
authored and
Commit Queue
committed
[dart2wasm] Switch to only using JS strings
Currently we have 3 different string types (JS Strings, OneByteString and TwoByteString)s. There's some advantages to this, mainly that if strings are used purely inside Dart we have more control over optimizing them. But it does come with some issues * Operations on mixture of strings are slow * We get JS strings from outside (in DevTools e.g. websocket messages) * Any kind of DOM interaction requires copying strings * Regular expression matches can result in O(N*N) instead of O(N) * Encoding of string literals/constants is terrible, high size overhead * ... Now that there's a standardized way to access JS strings (via the `js-string` builtin spec) and this standard is finalized and enabled in Chrome & Firefox it makes sense for us to switch to it. It reduces app size: * Smaller size: hello world -25%, flute -5.5% * Faster startup The performance changes are nuanced, some workloads will improve significantly, some workloads will regress. Improvements will come especially in cases where strings are concatenated (due to JS not actually allocating new strings in this case). That impacts e.g. string interpolations, string buffer, json-to-string encoding, ... Regressions will come especially for cases where we have to construct strings from bytes (e.g. in utf8 decoder, utf8+json decoder) - mainly due to having to go through an intermediary `WasmArray<WasmI16>` to allocate strings. Also in cases where we access individual char codes from the strings. There's some follow-up improvements we can do, but it's better to not iterate on this CL even longer but get it landed. This CL will make the benchmarking system use `--require-js-string-builtin` as well as most of test CI (in `pkg/dart2wasm/tool/compile_benchmark`) Though we run some configurations via overriding with `--no-require-js-string-builtin` (in `tools/bots/test_matrix.json`) Issue flutter/flutter#159400 (comment) Issue #59699 TEST=ci Change-Id: I238ac65efe092de569da870f23134f889ac929f9 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/392903 Reviewed-by: Slava Egorov <[email protected]> Reviewed-by: Lasse Nielsen <[email protected]> Commit-Queue: Martin Kustermann <[email protected]>
1 parent 092728e commit 20fc102

27 files changed

+1028
-1364
lines changed

‎pkg/dart2wasm/lib/class_info.dart‎

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -106,10 +106,6 @@ class FieldIndex {
106106
check(translator.boxedBoolClass, "value", FieldIndex.boxValue);
107107
check(translator.boxedIntClass, "value", FieldIndex.boxValue);
108108
check(translator.boxedDoubleClass, "value", FieldIndex.boxValue);
109-
if (!translator.options.jsCompatibility) {
110-
check(translator.oneByteStringClass, "_array", FieldIndex.stringArray);
111-
check(translator.twoByteStringClass, "_array", FieldIndex.stringArray);
112-
}
113109
check(translator.listBaseClass, "_length", FieldIndex.listLength);
114110
check(translator.listBaseClass, "_data", FieldIndex.listArray);
115111
check(translator.hashFieldBaseClass, "_index", FieldIndex.hashBaseIndex);
@@ -268,6 +264,7 @@ class ClassInfoCollector {
268264
translator.coreTypes.recordClass,
269265
translator.index.getClass("dart:core", "_Type"),
270266
translator.index.getClass("dart:_list", "WasmListBase"),
267+
translator.index.getClass("dart:_string", "JSStringImpl"),
271268
};
272269
for (final name in const <String>[
273270
"ByteBuffer",
@@ -334,17 +331,15 @@ class ClassInfoCollector {
334331
}
335332

336333
// In the Wasm type hierarchy, Object, bool and num sit directly below
337-
// the Top type. The implementation classes WasmStringBase and _Type sit
338-
// directly below the public classes they implement.
339-
// All other classes sit below their superclass.
334+
// the Top type. The implementation classes of _Type sit directly below
335+
// the public classes they implement. All other classes sit below their
336+
// superclass.
340337
ClassInfo superInfo = cls == translator.coreTypes.boolClass ||
341338
cls == translator.coreTypes.numClass ||
342339
cls == translator.boxedIntClass ||
343340
cls == translator.boxedDoubleClass
344341
? topInfo
345-
: (!translator.options.jsCompatibility &&
346-
cls == translator.wasmStringBaseClass) ||
347-
cls == translator.typeClass
342+
: cls == translator.typeClass
348343
? translator.classInfo[cls.implementedTypes.single.classNode]!
349344
: translator.classInfo[superclass]!;
350345

@@ -754,10 +749,8 @@ class ClassIdNumbering {
754749
// we have to encode a class-id. Then we could reorder the subclasses
755750
// depending on usage count of the subclass trees.
756751
final fixedOrder = <Class, int>{
757-
translator.coreTypes.boolClass: -10,
758-
translator.coreTypes.numClass: -9,
759-
if (!translator.options.jsCompatibility)
760-
translator.wasmStringBaseClass: -8,
752+
translator.coreTypes.boolClass: -9,
753+
translator.coreTypes.numClass: -8,
761754
translator.jsStringClass: -7,
762755
translator.typeClass: -6,
763756
translator.listBaseClass: -5,

‎pkg/dart2wasm/lib/code_generator.dart‎

Lines changed: 14 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -857,9 +857,7 @@ abstract class AstCodeGenerator
857857
b.ref_null(w.HeapType.none);
858858
}
859859
final Location? location = node.location;
860-
final stringClass = translator.options.jsCompatibility
861-
? translator.jsStringClass
862-
: translator.stringBaseClass;
860+
final stringClass = translator.jsStringClass;
863861
final w.RefType stringRefType =
864862
translator.classInfo[stringClass]!.nullableType;
865863
if (location != null) {
@@ -2703,9 +2701,10 @@ abstract class AstCodeGenerator
27032701
}
27042702
}
27052703

2706-
if (node.expressions.every(isConstantString)) {
2704+
final expressions = node.expressions;
2705+
if (expressions.every(isConstantString)) {
27072706
StringBuffer result = StringBuffer();
2708-
for (final expr in node.expressions) {
2707+
for (final expr in expressions) {
27092708
result.write(extractConstantString(expr));
27102709
}
27112710
final expr = StringLiteral(result.toString());
@@ -2714,32 +2713,28 @@ abstract class AstCodeGenerator
27142713

27152714
late final Procedure target;
27162715

2717-
final expressions = node.expressions;
2718-
// We have special cases for 1/2/3/4 arguments in non-JSCM mode.
2719-
if (!translator.options.jsCompatibility && expressions.length <= 4) {
2716+
// We have special cases for 1/2/3/4 arguments.
2717+
if (expressions.length <= 4) {
27202718
final nullableObjectType =
27212719
translator.translateType(translator.coreTypes.objectNullableRawType);
27222720
for (final expression in expressions) {
27232721
translateExpression(expression, nullableObjectType);
27242722
}
27252723
if (expressions.length == 1) {
2726-
target = translator.stringInterpolate1;
2724+
target = translator.jsStringInterpolate1;
27272725
} else if (expressions.length == 2) {
2728-
target = translator.stringInterpolate2;
2726+
target = translator.jsStringInterpolate2;
27292727
} else if (expressions.length == 3) {
2730-
target = translator.stringInterpolate3;
2728+
target = translator.jsStringInterpolate3;
27312729
} else {
27322730
assert(expressions.length == 4);
2733-
target = translator.stringInterpolate4;
2731+
target = translator.jsStringInterpolate4;
27342732
}
27352733
} else {
27362734
final nullableObjectType = translator.coreTypes.objectNullableRawType;
2737-
makeArrayFromExpressions(node.expressions, nullableObjectType);
2738-
target = translator.options.jsCompatibility
2739-
? translator.jsStringInterpolate
2740-
: translator.stringInterpolate;
2735+
makeArrayFromExpressions(expressions, nullableObjectType);
2736+
target = translator.jsStringInterpolate;
27412737
}
2742-
27432738
return translator.outputOrVoid(call(target.reference));
27442739
}
27452740

@@ -4348,9 +4343,7 @@ class SwitchInfo {
43484343
} else if (check<IntLiteral, IntConstant>()) {
43494344
equalsMember = translator.boxedIntEquals;
43504345
} else if (check<StringLiteral, StringConstant>()) {
4351-
equalsMember = translator.options.jsCompatibility
4352-
? translator.jsStringEquals
4353-
: translator.stringEquals;
4346+
equalsMember = translator.jsStringEquals;
43544347
} else {
43554348
equalsMember = translator.coreTypes.identicalProcedure;
43564349
}
@@ -4413,9 +4406,7 @@ class SwitchInfo {
44134406
compare = (switchExprLocal, pushCaseExpr) {
44144407
codeGen.b.local_get(switchExprLocal);
44154408
pushCaseExpr();
4416-
codeGen.call(translator.options.jsCompatibility
4417-
? translator.jsStringEquals.reference
4418-
: translator.stringEquals.reference);
4409+
codeGen.call(translator.jsStringEquals.reference);
44194410
};
44204411
} else {
44214412
// Object identity switch

‎pkg/dart2wasm/lib/constants.dart‎

Lines changed: 4 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@
22
// for details. All rights reserved. Use of this source code is governed by a
33
// BSD-style license that can be found in the LICENSE file.
44

5-
import 'dart:typed_data';
6-
75
import 'package:kernel/ast.dart';
86
import 'package:kernel/core_types.dart';
97
import 'package:kernel/type_algebra.dart'
@@ -571,56 +569,11 @@ class ConstantCreator extends ConstantVisitor<ConstantInfo?>
571569

572570
@override
573571
ConstantInfo? visitStringConstant(StringConstant constant) {
574-
if (translator.options.jsCompatibility) {
575-
ClassInfo info = translator.classInfo[translator.jsStringClass]!;
576-
return createConstant(constant, info.nonNullableType, (b) {
577-
b.pushObjectHeaderFields(translator, info);
578-
translator.globals.readGlobal(b,
579-
translator.getInternalizedStringGlobal(b.module, constant.value));
580-
b.struct_new(info.struct);
581-
});
582-
}
583-
bool isOneByte = constant.value.codeUnits.every((c) => c <= 255);
584-
ClassInfo info = translator.classInfo[isOneByte
585-
? translator.oneByteStringClass
586-
: translator.twoByteStringClass]!;
587-
translator.functions.recordClassAllocation(info.classId);
588-
w.RefType type = info.nonNullableType;
589-
bool lazy = constant.value.length > maxArrayNewFixedLength;
590-
return createConstant(constant, type, lazy: lazy, (b) {
591-
w.ArrayType arrayType =
592-
(info.struct.fields[FieldIndex.stringArray].type as w.RefType)
593-
.heapType as w.ArrayType;
594-
572+
ClassInfo info = translator.classInfo[translator.jsStringClass]!;
573+
return createConstant(constant, info.nonNullableType, (b) {
595574
b.pushObjectHeaderFields(translator, info);
596-
if (lazy) {
597-
// Initialize string contents from passive data segment.
598-
w.DataSegmentBuilder segment;
599-
Uint8List bytes;
600-
if (isOneByte) {
601-
segment = constants.oneByteStringSegment ??=
602-
targetModule.dataSegments.define();
603-
bytes = Uint8List.fromList(constant.value.codeUnits);
604-
} else {
605-
assert(Endian.host == Endian.little);
606-
segment = constants.twoByteStringSegment ??=
607-
targetModule.dataSegments.define();
608-
bytes = Uint16List.fromList(constant.value.codeUnits)
609-
.buffer
610-
.asUint8List();
611-
}
612-
int offset = segment.length;
613-
segment.append(bytes);
614-
b.i32_const(offset);
615-
b.i32_const(constant.value.length);
616-
b.array_new_data(arrayType, segment);
617-
} else {
618-
// Initialize string contents from i32 constants on the stack.
619-
for (int charCode in constant.value.codeUnits) {
620-
b.i32_const(charCode);
621-
}
622-
b.array_new_fixed(arrayType, constant.value.length);
623-
}
575+
translator.globals.readGlobal(
576+
b, translator.getInternalizedStringGlobal(b.module, constant.value));
624577
b.struct_new(info.struct);
625578
});
626579
}

‎pkg/dart2wasm/lib/dynamic_modules.dart‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -775,9 +775,7 @@ class ConstantCanonicalizer extends ConstantVisitor<void> {
775775

776776
/// Values of these types are canonicalized by their == function.
777777
late final Set<Class> _equalsCheckerClasses = {
778-
if (translator.options.jsCompatibility) translator.jsStringClass,
779-
if (!translator.options.jsCompatibility) translator.oneByteStringClass,
780-
if (!translator.options.jsCompatibility) translator.twoByteStringClass,
778+
translator.jsStringClass,
781779
translator.symbolClass,
782780
translator.closureClass,
783781
};
@@ -1134,11 +1132,7 @@ class ConstantCanonicalizer extends ConstantVisitor<void> {
11341132

11351133
@override
11361134
void visitStringConstant(StringConstant node) {
1137-
_canonicalizeInstance(translator.options.jsCompatibility
1138-
? translator.jsStringClass
1139-
: (node.value.codeUnits.every((c) => c <= 255)
1140-
? translator.oneByteStringClass
1141-
: translator.twoByteStringClass));
1135+
_canonicalizeInstance(translator.jsStringClass);
11421136
}
11431137

11441138
@override

‎pkg/dart2wasm/lib/js/runtime_blob.dart‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,15 @@ const jsStringPolyfill = {
211211
}
212212
return result;
213213
},
214+
"intoCharCodeArray": (s, a, start) => {
215+
if (s == '') return 0;
216+
217+
const write = dartInstance.exports.$wasmI16ArraySet;
218+
for (var i = 0; i < s.length; ++i) {
219+
write(a, start++, s.charCodeAt(i));
220+
}
221+
return s.length;
222+
},
214223
};
215224
''';
216225

‎pkg/dart2wasm/lib/kernel_nodes.dart‎

Lines changed: 8 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -45,14 +45,6 @@ mixin KernelNodes {
4545
index.getClass("dart:_list", "GrowableList");
4646
late final Class immutableListClass =
4747
index.getClass("dart:_list", "ImmutableList");
48-
late final Class wasmStringBaseClass =
49-
index.getClass("dart:_internal", "WasmStringBase");
50-
late final Class stringBaseClass =
51-
index.getClass("dart:_string", "StringBase");
52-
late final Class oneByteStringClass =
53-
index.getClass("dart:_string", "OneByteString");
54-
late final Class twoByteStringClass =
55-
index.getClass("dart:_string", "TwoByteString");
5648
late final Class invocationClass = index.getClass("dart:core", 'Invocation');
5749
late final Class noSuchMethodErrorClass =
5850
index.getClass("dart:core", "NoSuchMethodError");
@@ -221,6 +213,14 @@ mixin KernelNodes {
221213
index.getProcedure("dart:_string", "JSStringImpl", "==");
222214
late final Procedure jsStringInterpolate =
223215
index.getProcedure("dart:_string", "JSStringImpl", "_interpolate");
216+
late final Procedure jsStringInterpolate1 =
217+
index.getProcedure("dart:_string", "JSStringImpl", "_interpolate1");
218+
late final Procedure jsStringInterpolate2 =
219+
index.getProcedure("dart:_string", "JSStringImpl", "_interpolate2");
220+
late final Procedure jsStringInterpolate3 =
221+
index.getProcedure("dart:_string", "JSStringImpl", "_interpolate3");
222+
late final Procedure jsStringInterpolate4 =
223+
index.getProcedure("dart:_string", "JSStringImpl", "_interpolate4");
224224

225225
// dart:collection procedures and fields
226226
late final Procedure mapFactory =
@@ -265,18 +265,6 @@ mixin KernelNodes {
265265
index.getProcedure("dart:core", "Object", "_nullToString");
266266
late final Procedure nullNoSuchMethod =
267267
index.getProcedure("dart:core", "Object", "_nullNoSuchMethod");
268-
late final Procedure stringEquals =
269-
index.getProcedure("dart:_string", "StringBase", "_equals");
270-
late final Procedure stringInterpolate =
271-
index.getProcedure("dart:_string", "StringBase", "_interpolate");
272-
late final Procedure stringInterpolate1 =
273-
index.getProcedure("dart:_string", "StringBase", "_interpolate1");
274-
late final Procedure stringInterpolate2 =
275-
index.getProcedure("dart:_string", "StringBase", "_interpolate2");
276-
late final Procedure stringInterpolate3 =
277-
index.getProcedure("dart:_string", "StringBase", "_interpolate3");
278-
late final Procedure stringInterpolate4 =
279-
index.getProcedure("dart:_string", "StringBase", "_interpolate4");
280268
late final Procedure truncDiv =
281269
index.getProcedure("dart:_boxed_int", "BoxedInt", "_truncDiv");
282270
late final Procedure runtimeTypeEquals =

‎pkg/dart2wasm/lib/target.dart‎

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -106,8 +106,6 @@ class WasmTarget extends Target {
106106
Class? _wasmDefaultSet;
107107
Class? _wasmImmutableMap;
108108
Class? _wasmImmutableSet;
109-
Class? _oneByteString;
110-
Class? _twoByteString;
111109
Class? _jsString;
112110
Class? _closure;
113111
Class? _boxedInt;
@@ -505,20 +503,8 @@ class WasmTarget extends Target {
505503

506504
@override
507505
Class concreteStringLiteralClass(CoreTypes coreTypes, String value) {
508-
// In JSCM all strings are JS strings.
509-
if (mode == Mode.jsCompatibility) {
510-
return _jsString ??=
511-
coreTypes.index.getClass("dart:_string", "JSStringImpl");
512-
}
513-
const int maxLatin1 = 0xff;
514-
for (int i = 0; i < value.length; ++i) {
515-
if (value.codeUnitAt(i) > maxLatin1) {
516-
return _twoByteString ??=
517-
coreTypes.index.getClass('dart:_string', 'TwoByteString');
518-
}
519-
}
520-
return _oneByteString ??=
521-
coreTypes.index.getClass('dart:_string', 'OneByteString');
506+
return _jsString ??=
507+
coreTypes.index.getClass("dart:_string", "JSStringImpl");
522508
}
523509

524510
// In dart2wasm we can't assume that `x == "hello"` means `x`'s class is

‎pkg/dart2wasm/lib/translator.dart‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -314,11 +314,7 @@ class Translator with KernelNodes {
314314
boxedIntClass: boxedIntClass,
315315
boxedDoubleClass: boxedDoubleClass,
316316
boxedBoolClass: coreTypes.boolClass,
317-
if (!options.jsCompatibility) ...{
318-
oneByteStringClass: stringBaseClass,
319-
twoByteStringClass: stringBaseClass
320-
},
321-
if (options.jsCompatibility) ...{jsStringClass: jsStringClass},
317+
jsStringClass: jsStringClass,
322318
};
323319

324320
/// Type for vtable entries for dynamic calls. These entries are used in
@@ -444,8 +440,7 @@ class Translator with KernelNodes {
444440
mapEntries.add(
445441
MapLiteralEntry(StringLiteral(libName), MapLiteral(subMapEntries)));
446442
});
447-
final stringClass =
448-
options.jsCompatibility ? jsStringClass : stringBaseClass;
443+
final stringClass = jsStringClass;
449444
loadLibraryImportMap.function.body = ReturnStatement(MapLiteral(mapEntries,
450445
keyType: InterfaceType(stringClass, Nullability.nonNullable),
451446
valueType: InterfaceType(coreTypes.mapNonNullableRawType.classNode,
@@ -1722,6 +1717,7 @@ class Translator with KernelNodes {
17221717
if (internalizedString != null) {
17231718
return internalizedString;
17241719
}
1720+
17251721
bool hasUnpairedSurrogate(String str) {
17261722
for (int i = 0; i < str.length; i++) {
17271723
int codeUnit = str.codeUnitAt(i);

‎pkg/dart2wasm/tool/compile_benchmark‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ SNAPSHOT_NAME="dart2wasm"
5959
# All arguments will be passed along to dart2wasm except specially recognized
6060
# flags.
6161
VM_ARGS=()
62-
DART2WASM_ARGS=()
62+
DART2WASM_ARGS=("--require-js-string-builtin")
6363
DART_FILE=""
6464
WASM_FILE=""
6565
while [ $# -gt 0 ]; do

‎pkg/vm/testcases/transformations/type_flow/transformer/records_dart2wasm.dart.expect‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ static method main() → dynamic {
2424
core::print(self::recordFieldAccess1((10, "hi")));
2525
core::print(self::recordFieldAccess2(({a: 20, b: "bye"})));
2626
core::print(self::recordDynamicFieldAccess([@vm.direct-call.metadata=dart._list::WasmListBase.[]??] [@vm.inferred-type.metadata=!? (receiver not int)] [@vm.inferred-type.metadata=dart._list::GrowableList?<dart.core::Object>] self::list{dynamic}.[](1)));
27-
core::print([@vm.direct-call.metadata=dart.core::Record_2.toString] [@vm.inferred-type.metadata=! (skip check) (receiver not int)](1, 2).{core::Object::toString}(){() → core::String});
27+
core::print([@vm.direct-call.metadata=dart.core::Record_2.toString] [@vm.inferred-type.metadata=library dart:_string::JSStringImpl (skip check) (receiver not int)](1, 2).{core::Object::toString}(){() → core::String});
2828
}
2929
constants {
3030
#C1 = 42

0 commit comments

Comments
 (0)