Skip to content

Commit c572fbf

Browse files
vsmenoncommit-bot@chromium.org
vsmenon
authored and
[email protected]
committed
[dartdevc] Fix DDK performance regression vs DDC
This explicitly types JS calls via the generic param. Note, we also have a type string (first arg, described in foreign_helper) that the compilers use. We have an analyzer hack with old DDC to use that first string arg as a static type. With DDK, we need to use the generic instead. If we don't, we end up with extraneous dynamic calls, null checks, and casts. Exec time numbers (smaller is better): - DDC - richards: 5668.555240793201 us - deltablue: 11736.842105263158 us - DDK before - richards: 40760 us - deltablue: 98476.19047619049 us - DDK after - richards: 1889.5184135977336 us - deltablue: 8432.773109243699 us Change-Id: I720d3b40d5875d9cd6cea1c614d838f06aa86776 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/112268 Commit-Queue: Vijay Menon <[email protected]> Reviewed-by: Nicholas Shahan <[email protected]>
1 parent 77f50c0 commit c572fbf

26 files changed

+326
-307
lines changed

‎sdk/lib/_internal/js_dev_runtime/lib/js/dart2js/js_dart2js.dart‎

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,7 @@ class JsObject {
213213
int get hashCode => 0;
214214

215215
bool operator ==(other) =>
216-
other is JsObject && JS('bool', '# === #', _jsObject, other._jsObject);
216+
other is JsObject && JS<bool>('!', '# === #', _jsObject, other._jsObject);
217217

218218
/**
219219
* Returns `true` if the JavaScript object contains the specified property
@@ -225,7 +225,7 @@ class JsObject {
225225
if (property is! String && property is! num) {
226226
throw ArgumentError("property is not a String or num");
227227
}
228-
return JS('bool', '# in #', property, _jsObject);
228+
return JS<bool>('!', '# in #', property, _jsObject);
229229
}
230230

231231
/**
@@ -237,7 +237,7 @@ class JsObject {
237237
if (property is! String && property is! num) {
238238
throw ArgumentError("property is not a String or num");
239239
}
240-
JS('bool', 'delete #[#]', _jsObject, property);
240+
JS<bool>('!', 'delete #[#]', _jsObject, property);
241241
}
242242

243243
/**
@@ -246,15 +246,15 @@ class JsObject {
246246
* This is the equivalent of the `instanceof` operator in JavaScript.
247247
*/
248248
bool instanceof(JsFunction type) {
249-
return JS('bool', '# instanceof #', _jsObject, _convertToJS(type));
249+
return JS<bool>('!', '# instanceof #', _jsObject, _convertToJS(type));
250250
}
251251

252252
/**
253253
* Returns the result of the JavaScript objects `toString` method.
254254
*/
255255
String toString() {
256256
try {
257-
return JS('String', 'String(#)', _jsObject);
257+
return JS<String>('!', 'String(#)', _jsObject);
258258
} catch (e) {
259259
return super.toString();
260260
}
@@ -272,7 +272,7 @@ class JsObject {
272272
}
273273
if (args != null) args = List.from(args.map(_convertToJS));
274274
var fn = JS('', '#[#]', _jsObject, method);
275-
if (JS('bool', 'typeof(#) !== "function"', fn)) {
275+
if (JS<bool>('!', 'typeof(#) !== "function"', fn)) {
276276
throw NoSuchMethodError(_jsObject, Symbol(method), args, {});
277277
}
278278
return _convertToDart(JS('', '#.apply(#, #)', fn, _jsObject, args));
@@ -379,8 +379,9 @@ class JsArray<E> extends JsObject with ListMixin<E> {
379379
// Check the length honours the List contract.
380380
var len = JS('', '#.length', _jsObject);
381381
// JavaScript arrays have lengths which are unsigned 32-bit integers.
382-
if (JS('bool', 'typeof # === "number" && (# >>> 0) === #', len, len, len)) {
383-
return JS('int', '#', len);
382+
if (JS<bool>(
383+
'!', 'typeof # === "number" && (# >>> 0) === #', len, len, len)) {
384+
return JS<int>('!', '#', len);
384385
}
385386
throw StateError('Bad JsArray length');
386387
}
@@ -396,7 +397,7 @@ class JsArray<E> extends JsObject with ListMixin<E> {
396397
}
397398

398399
void addAll(Iterable<E> iterable) {
399-
var list = (JS('bool', '# instanceof Array', iterable))
400+
var list = (JS<bool>('!', '# instanceof Array', iterable))
400401
? iterable
401402
: List.from(iterable);
402403
callMethod('push', list);
@@ -519,10 +520,10 @@ Object _convertToDart(o) {
519520
Object _wrapToDart(o) => _putIfAbsent(_dartProxies, o, _wrapToDartHelper);
520521

521522
Object _wrapToDartHelper(o) {
522-
if (JS('bool', 'typeof # == "function"', o)) {
523+
if (JS<bool>('!', 'typeof # == "function"', o)) {
523524
return JsFunction._fromJs(o);
524525
}
525-
if (JS('bool', '# instanceof Array', o)) {
526+
if (JS<bool>('!', '# instanceof Array', o)) {
526527
return JsArray._fromJs(o);
527528
}
528529
return JsObject._fromJs(o);

‎sdk/lib/_internal/js_dev_runtime/lib/js_util/dart2js/js_util_dart2js.dart‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,20 +58,20 @@ _convertDataTree(data) {
5858

5959
newObject() => JS('=Object', '{}');
6060

61-
hasProperty(o, name) => JS('bool', '# in #', name, o);
61+
hasProperty(o, name) => JS<bool>('!', '# in #', name, o);
6262
getProperty(o, name) => JS('Object', '#[#]', o, name);
6363
setProperty(o, name, value) => JS('', '#[#]=#', o, name, value);
6464

6565
callMethod(o, String method, List args) =>
6666
JS('Object', '#[#].apply(#, #)', o, method, o, args);
6767

68-
instanceof(o, Function type) => JS('bool', '# instanceof #', o, type);
68+
instanceof(o, Function type) => JS<bool>('!', '# instanceof #', o, type);
6969
callConstructor(Function constr, List arguments) {
7070
if (arguments == null) {
7171
return JS('Object', 'new #()', constr);
7272
}
7373

74-
if (JS('bool', '# instanceof Array', arguments)) {
74+
if (JS<bool>('!', '# instanceof Array', arguments)) {
7575
int argumentCount = JS('!', '#.length', arguments);
7676
switch (argumentCount) {
7777
case 0:
@@ -112,7 +112,7 @@ callConstructor(Function constr, List arguments) {
112112
var args = <dynamic>[null]..addAll(arguments);
113113
var factoryFunction = JS('', '#.bind.apply(#, #)', constr, constr, args);
114114
// Without this line, calling factoryFunction as a constructor throws
115-
JS('String', 'String(#)', factoryFunction);
115+
JS<String>('!', 'String(#)', factoryFunction);
116116
// This could return an UnknownJavaScriptObject, or a native
117117
// object for which there is an interceptor
118118
return JS('Object', 'new #()', factoryFunction);

‎sdk/lib/_internal/js_dev_runtime/patch/async_patch.dart‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ _async<T>(Function() initGenerator) {
4444
onValue = (value) {
4545
var iteratorResult = JS('', '#.next(#)', iter, value);
4646
value = JS('', '#.value', iteratorResult);
47-
return JS('bool', '#.done', iteratorResult) ? value : onAwait(value);
47+
return JS<bool>('!', '#.done', iteratorResult) ? value : onAwait(value);
4848
};
4949

5050
// If the awaited Future throws, we want to convert this to an exception
@@ -59,7 +59,7 @@ _async<T>(Function() initGenerator) {
5959
var iteratorResult = JS(
6060
'', '#.throw(#)', iter, dart.createErrorWithStack(value, stackTrace));
6161
value = JS('', '#.value', iteratorResult);
62-
return JS('bool', '#.done', iteratorResult) ? value : onAwait(value);
62+
return JS<bool>('!', '#.done', iteratorResult) ? value : onAwait(value);
6363
};
6464

6565
var zone = Zone.current;
@@ -83,7 +83,7 @@ _async<T>(Function() initGenerator) {
8383
iter = JS('', '#[Symbol.iterator]()', initGenerator());
8484
var iteratorValue = JS('', '#.next(null)', iter);
8585
var value = JS('', '#.value', iteratorValue);
86-
if (JS('bool', '#.done', iteratorValue)) {
86+
if (JS<bool>('!', '#.done', iteratorValue)) {
8787
// TODO(jmesserly): this is a workaround for ignored cast failures.
8888
// Remove it once we've fixed those. We should be able to call:
8989
//

‎sdk/lib/_internal/js_dev_runtime/patch/collection_patch.dart‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ class _HashSet<E> extends _InternalSet<E>
181181
bool contains(Object key) {
182182
if (key == null) {
183183
key = null;
184-
} else if (JS('bool', '#[#] !== #', key, dart.extensionSymbol('_equals'),
184+
} else if (JS<bool>('!', '#[#] !== #', key, dart.extensionSymbol('_equals'),
185185
dart.identityEquals)) {
186186
@notNull
187187
var k = key;
@@ -194,12 +194,12 @@ class _HashSet<E> extends _InternalSet<E>
194194
}
195195
return false;
196196
}
197-
return JS('bool', '#.has(#)', _map, key);
197+
return JS<bool>('!', '#.has(#)', _map, key);
198198
}
199199

200200
E lookup(Object key) {
201201
if (key == null) return null;
202-
if (JS('bool', '#[#] !== #', key, dart.extensionSymbol('_equals'),
202+
if (JS<bool>('!', '#[#] !== #', key, dart.extensionSymbol('_equals'),
203203
dart.identityEquals)) {
204204
@notNull
205205
var k = key;
@@ -220,7 +220,7 @@ class _HashSet<E> extends _InternalSet<E>
220220
if (key == null) {
221221
if (JS('', '#.has(null)', map)) return false;
222222
key = null;
223-
} else if (JS('bool', '#[#] !== #', key, dart.extensionSymbol('_equals'),
223+
} else if (JS<bool>('!', '#[#] !== #', key, dart.extensionSymbol('_equals'),
224224
dart.identityEquals)) {
225225
var keyMap = _keyMap;
226226
@notNull
@@ -250,8 +250,8 @@ class _HashSet<E> extends _InternalSet<E>
250250
for (E key in objects) {
251251
if (key == null) {
252252
key = null; // converts undefined to null, if needed.
253-
} else if (JS('bool', '#[#] !== #', key, dart.extensionSymbol('_equals'),
254-
dart.identityEquals)) {
253+
} else if (JS<bool>('!', '#[#] !== #', key,
254+
dart.extensionSymbol('_equals'), dart.identityEquals)) {
255255
key = putLinkedMapKey(key, _keyMap);
256256
}
257257
JS('', '#.add(#)', map, key);
@@ -264,7 +264,7 @@ class _HashSet<E> extends _InternalSet<E>
264264
bool remove(Object key) {
265265
if (key == null) {
266266
key = null;
267-
} else if (JS('bool', '#[#] !== #', key, dart.extensionSymbol('_equals'),
267+
} else if (JS<bool>('!', '#[#] !== #', key, dart.extensionSymbol('_equals'),
268268
dart.identityEquals)) {
269269
@notNull
270270
var k = key;
@@ -286,7 +286,7 @@ class _HashSet<E> extends _InternalSet<E>
286286
}
287287
}
288288
var map = _map;
289-
if (JS('bool', '#.delete(#)', map, key)) {
289+
if (JS<bool>('!', '#.delete(#)', map, key)) {
290290
_modifications = (_modifications + 1) & 0x3ffffff;
291291
return true;
292292
}
@@ -310,8 +310,8 @@ class _ImmutableSet<E> extends _HashSet<E> {
310310
for (Object key in entries) {
311311
if (key == null) {
312312
key = null; // converts undefined to null, if needed.
313-
} else if (JS('bool', '#[#] !== #', key, dart.extensionSymbol('_equals'),
314-
dart.identityEquals)) {
313+
} else if (JS<bool>('!', '#[#] !== #', key,
314+
dart.extensionSymbol('_equals'), dart.identityEquals)) {
315315
key = putLinkedMapKey(key, _keyMap);
316316
}
317317
JS('', '#.add(#)', map, key);
@@ -352,7 +352,7 @@ class _IdentityHashSet<E> extends _InternalSet<E>
352352

353353
bool add(E element) {
354354
var map = _map;
355-
if (JS('bool', '#.has(#)', map, element)) return false;
355+
if (JS<bool>('!', '#.has(#)', map, element)) return false;
356356
JS('', '#.add(#)', map, element);
357357
_modifications = (_modifications + 1) & 0x3ffffff;
358358
return true;
@@ -370,7 +370,7 @@ class _IdentityHashSet<E> extends _InternalSet<E>
370370
}
371371

372372
bool remove(Object element) {
373-
if (JS('bool', '#.delete(#)', _map, element)) {
373+
if (JS<bool>('!', '#.delete(#)', _map, element)) {
374374
_modifications = (_modifications + 1) & 0x3ffffff;
375375
return true;
376376
}
@@ -545,10 +545,10 @@ abstract class _InternalSet<E> extends _SetBase<E> {
545545
int get length => JS<int>('!', '#.size', _map);
546546

547547
@notNull
548-
bool get isEmpty => JS('bool', '#.size == 0', _map);
548+
bool get isEmpty => JS<bool>('!', '#.size == 0', _map);
549549

550550
@notNull
551-
bool get isNotEmpty => JS('bool', '#.size != 0', _map);
551+
bool get isNotEmpty => JS<bool>('!', '#.size != 0', _map);
552552

553553
Iterator<E> get iterator => DartIterator<E>(_jsIterator());
554554

‎sdk/lib/_internal/js_dev_runtime/patch/convert_patch.dart‎

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ _parseJson(String source, reviver(Object key, Object value)) {
3636
parsed = JS('=Object|JSExtendableArray|Null|bool|num|String',
3737
'JSON.parse(#)', source);
3838
} catch (e) {
39-
throw FormatException(JS('String', 'String(#)', e));
39+
throw FormatException(JS<String>('!', 'String(#)', e));
4040
}
4141

4242
if (reviver == null) {
@@ -55,16 +55,17 @@ _convertJsonToDart(json, reviver(Object key, Object value)) {
5555
assert(reviver != null);
5656
walk(e) {
5757
// JavaScript null, string, number, bool are in the correct representation.
58-
if (JS('bool', '# == null', e) || JS('bool', 'typeof # != "object"', e)) {
58+
if (JS<bool>('!', '# == null', e) ||
59+
JS<bool>('!', 'typeof # != "object"', e)) {
5960
return e;
6061
}
6162

6263
// This test is needed to avoid identifying '{"__proto__":[]}' as an Array.
6364
// TODO(sra): Replace this test with cheaper '#.constructor === Array' when
6465
// bug 621 below is fixed.
65-
if (JS('bool', 'Object.getPrototypeOf(#) === Array.prototype', e)) {
66+
if (JS<bool>('!', 'Object.getPrototypeOf(#) === Array.prototype', e)) {
6667
// In-place update of the elements since JS Array is a Dart List.
67-
for (int i = 0; i < JS('int', '#.length', e); i++) {
68+
for (int i = 0; i < JS<int>('!', '#.length', e); i++) {
6869
// Use JS indexing to avoid range checks. We know this is the only
6970
// reference to the list, but the compiler will likely never be able to
7071
// tell that this instance of the list cannot have its length changed by
@@ -100,19 +101,19 @@ _convertJsonToDartLazy(object) {
100101
if (object == null) return null;
101102

102103
// JavaScript string, number, bool already has the correct representation.
103-
if (JS('bool', 'typeof # != "object"', object)) {
104+
if (JS<bool>('!', 'typeof # != "object"', object)) {
104105
return object;
105106
}
106107

107108
// This test is needed to avoid identifying '{"__proto__":[]}' as an array.
108109
// TODO(sra): Replace this test with cheaper '#.constructor === Array' when
109110
// bug https://code.google.com/p/v8/issues/detail?id=621 is fixed.
110-
if (JS('bool', 'Object.getPrototypeOf(#) !== Array.prototype', object)) {
111+
if (JS<bool>('!', 'Object.getPrototypeOf(#) !== Array.prototype', object)) {
111112
return _JsonMap(object);
112113
}
113114

114115
// Update the elements in place since JS arrays are Dart lists.
115-
for (int i = 0; i < JS('int', '#.length', object); i++) {
116+
for (int i = 0; i < JS<int>('!', '#.length', object); i++) {
116117
// Use JS indexing to avoid range checks. We know this is the only
117118
// reference to the list, but the compiler will likely never be able to
118119
// tell that this instance of the list cannot have its length changed by
@@ -319,14 +320,14 @@ class _JsonMap extends MapBase<String, dynamic> {
319320
// ------------------------------------------
320321

321322
static bool _hasProperty(object, String key) =>
322-
JS('bool', 'Object.prototype.hasOwnProperty.call(#,#)', object, key);
323+
JS<bool>('!', 'Object.prototype.hasOwnProperty.call(#,#)', object, key);
323324
static _getProperty(object, String key) => JS('', '#[#]', object, key);
324325
static _setProperty(object, String key, value) =>
325326
JS('', '#[#]=#', object, key, value);
326327
static List _getPropertyNames(object) =>
327328
JS('JSExtendableArray', 'Object.keys(#)', object);
328329
static bool _isUnprocessed(object) =>
329-
JS('bool', 'typeof(#)=="undefined"', object);
330+
JS<bool>('!', 'typeof(#)=="undefined"', object);
330331
static _newJavaScriptObject() => JS('=Object', 'Object.create(null)');
331332
}
332333

@@ -402,9 +403,9 @@ class Utf8Decoder {
402403
bool allowMalformed, List<int> codeUnits, int start, int end) {
403404
// Test `codeUnits is NativeUint8List`. Dart's NativeUint8List is
404405
// implemented by JavaScript's Uint8Array.
405-
if (JS('bool', '# instanceof Uint8Array', codeUnits)) {
406+
if (JS<bool>('!', '# instanceof Uint8Array', codeUnits)) {
406407
// JS 'cast' to avoid a downcast equivalent to the is-check we hand-coded.
407-
NativeUint8List casted = JS('NativeUint8List', '#', codeUnits);
408+
NativeUint8List casted = JS<NativeUint8List>('!', '#', codeUnits);
408409
return _convertInterceptedUint8List(allowMalformed, casted, start, end);
409410
}
410411
}
@@ -438,7 +439,7 @@ class Utf8Decoder {
438439
}
439440

440441
return _useTextDecoderChecked(decoder,
441-
JS('NativeUint8List', '#.subarray(#, #)', codeUnits, start, end));
442+
JS<NativeUint8List>('!', '#.subarray(#, #)', codeUnits, start, end));
442443
}
443444

444445
static String _useTextDecoderChecked(decoder, NativeUint8List codeUnits) {
@@ -451,7 +452,7 @@ class Utf8Decoder {
451452
// back on unintercepted decoder. The fallback will either succeed in
452453
// decoding, or report the problem better than TextDecoder.
453454
try {
454-
return JS('String', '#.decode(#)', decoder, codeUnits);
455+
return JS<String>('!', '#.decode(#)', decoder, codeUnits);
455456
} catch (e) {}
456457
return null;
457458
}

‎sdk/lib/_internal/js_dev_runtime/patch/core_patch.dart‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,10 +37,10 @@ int identityHashCode(Object object) {
3737
// for them to be equivalent to their computed hashCode function.
3838
int hash = JS('int|Null', r'#[#]', object, dart.identityHashCode_);
3939
if (hash == null) {
40-
hash = JS('int', '(Math.random() * 0x3fffffff) | 0');
40+
hash = JS<int>('!', '(Math.random() * 0x3fffffff) | 0');
4141
JS('void', r'#[#] = #', object, dart.identityHashCode_, hash);
4242
}
43-
return JS('int', '#', hash);
43+
return JS<int>('!', '#', hash);
4444
}
4545

4646
// Patch for Object implementation.
@@ -399,7 +399,7 @@ class List<E> {
399399
@patch
400400
factory List([@undefined int _length]) {
401401
dynamic list;
402-
if (JS('bool', '# === void 0', _length)) {
402+
if (JS<bool>('!', '# === void 0', _length)) {
403403
list = JS('', '[]');
404404
} else {
405405
int length = JS('!', '#', _length);
@@ -564,7 +564,7 @@ class RegExp {
564564
// Patch for 'identical' function.
565565
@patch
566566
bool identical(Object a, Object b) {
567-
return JS('bool', '(# == null ? # == null : # === #)', a, b, a, b);
567+
return JS<bool>('!', '(# == null ? # == null : # === #)', a, b, a, b);
568568
}
569569

570570
@patch

0 commit comments

Comments
 (0)