Dart2JS: Use of .runtimeType bloats entire output.

[matanlurey]

[SDK is latest available in google3]

Master issue: #31328

import 'dart:html';

void main() {
  // We use dart:html to block constant folding.
  var addEvent = document.addEventListener;
  addEvent('click', _run);
  addEvent('unrelated', _unrelated);
}

void _run(dynamic d) {
  var list = <String>[];
  String s = d;
  list.add(s);
  for (var x in list) {
    print(x);
  }
}

bool topLevelThing;

void _unrelated(_) {
  print(topLevel.runtimeType);
}

Emits the following dart2js:

main: function() {
  var addEvent = C.HtmlDocument_methods.get$addEventListener(document);
  addEvent.call$2("click", F.main___run$closure());
  addEvent.call$2("unrelated", F.main___unrelated$closure());
},
_run: [function(d) {
  var list, t1, _i;
  list = H.setRuntimeTypeInfo([], [P.String]);
  list.push(d);
  for (t1 = list.length, _i = 0; _i < list.length; list.length === t1 || (0, H.throwConcurrentModificationError)(list), ++_i)
    H.printString(H.S(list[_i]));
}, "call$1", "main___run$closure", 2, 0, 0],
_unrelated: [function(_) {
  var t1 = H.throwNoSuchMethod("", "topLevel", [], null);
  P.print(t1.get$runtimeType(t1));
}, "call$1", "main___unrelated$closure", 2, 0, 0]

Notice specifically, H.setRuntimeTypeInfo([], [P.String]).

This isn't used at all in the execution of the problem, but because an unrelated .runtimeType was used elsewhere in the program, we assume all reified generics need their types set. This really hurts performance in unpredictable ways and encourages teams not to use generics :-/

[rakudrama]

Dart 2 will require many instances to carry type parameters too.

Any down-cast requires types that might match to have instances with the type parameters available.
A call to Function.apply is like a downcast to the argument types of all functions that might make their way to Function.apply (the analysis tends to show only about half of closures are proven not to get to Function.apply, meaning the other half need type checks somewhere in Function.apply).

Since Dart 2 is similar to having .runtimeType, I think we need to make this kind of code-with-types run faster, and not rely on being able to optimize it out.

Can you quantify "really hurts performance" ?

---

In this very limited example it is possible to eliminate the attachment of the type parameter (setRuntimeTypeInfo call.)
We could see that list does not escape into any other code and the type is not used. For that to happen we need to eliminate the concurrent modification error (since it does escape the list). It is the fact that list appears to escape somewhere is that makes us think that printString might change list.length.

I have some CLs that improve the situation, I'll revisit once Kernel / Dart 2 is done.

[matanlurey]

@rakudrama:

Dart 2 will require many instances to carry type parameters too.

That's what I'm afraid of. Even in production mode?

A call to Function.apply is like a downcast to the argument types of all functions that might make their way to Function.apply (the analysis tends to show only about half of closures are proven not to get to Function.apply, meaning the other half need type checks somewhere in Function.apply).

Can you clarify this? 50% of closures are hit by Function.apply?

Since Dart 2 is similar to having .runtimeType, I think we need to make this kind of code-with-types run faster, and not rely on being able to optimize it out.

I guess I don't... understand why. Dart 2 requires types for optimization and heap soundness, but as long as the output of the code is the same I don't understand why Dart2JS needs to implement the specification. Nobody runs Dart2JS in spec mode in Dart 1 (--trust-types and --trust-primitives), so the prior art is there already.

Can you quantify "really hurts performance" ?

Sure! I ran some experiments: https://github.com/matanlurey/dart-js-improvements#rtti

In this (contrived, but based on real data internally) example, persisting RTTI added 3x to the size of the binary and slowed down initialization time by 70%, even though the output of both programs (with and without RTTI) were identical. To reproduce:

git clone https://github.com/matanlurey/dart-js-improvements.git
pub build benchmark 

# Gets size numbers.
pub run jsbench

# Gets runtime initialization cost.
dart tool/patch.dart
dart tool/sample.dart

We could see that list does not escape into any other code and the type is not used. For that to happen we need to eliminate the concurrent modification error (since it does escape the list).

Honestly we should just eliminate all of these classes of errors in production code.

I have some CLs that improve the situation, I'll revisit once Kernel / Dart 2 is done.

No rush. I know for sure you can improve it, but I want to improve the concept of Dart2JS so you won't have to try and implement the entire Dart 2.0 specification in the JS runtime. If we're going to do that, we might as well just switch to web assembly now.

[rakudrama]

"50% of closures are hit by Function.apply?"

No. 50% of closures are used in ways that we cannot prove do not flow into Function.apply. Double negative is not the same as positive, since there are things you can prove true, things you can prove false, and things you can't prove.
If dart2js thinks a function might be sent to Function.apply, it needs ensure the function has enough metadata to support being applied.
With Dart 2, that includes checking the arguments. Checking an argument of type Set<int> requires that all classes C that implement Set can be checked to see if they implement Set<int>. This usually involves the type parameters of C.

@matanlurey
There is something I find confusing with the requests you make. In #31428 you state "Almost every cast should be checked" and
"I'd like to see a new function to perform an "unsafe cast" instead of relying on implicit casts."
This seems in conflict with your above statement "I don't understand why Dart2JS needs to implement the specification".

Can you explain this discrepancy?

[matanlurey]

@rakudrama:

"50% of closures are hit by Function.apply?"

No. 50% of closures are used in ways that we cannot prove do not flow into Function.apply. Double negative is not the same as positive, since there are things you can prove true, things you can prove false, and things you can't prove.

If dart2js thinks a function might be sent to Function.apply, it needs ensure the function has enough metadata to support being applied.

With Dart 2, that includes checking the arguments. Checking an argument of type Set requires that all classes C that implement Set can be checked to see if they implement Set. This usually involves the type parameters of C.

That's really scary. I would hope Function.apply is used vary sparingly. Is there anything obvious pattern-wise we could do to reduce dart2js thinking Function.apply (up to just banningFunction.apply)?

There is something I find confusing with the requests you make. In #31428 you state "Almost every cast should be checked" and
"I'd like to see a new function to perform an "unsafe cast" instead of relying on implicit casts."

Users expect this to fail at runtime:

dynamic x = 5;
var y = x as String;

But we need a wait to opt-out for things we know are of a type but we can't express it:

void _internalFoo(dynamic /*String|int*/ x) {
  if (x is String) {
    // ..
  } else {
    var typedX = unsafeCast<int>(x);
    // ..
  }
}

This seems in conflict with your above statement "I don't understand why Dart2JS needs to implement the specification".

Can you explain this discrepancy?

I don't understand the need to implement Dart features that significantly affect the output and runtime performance of the JavaScript code and VM when they aren't required in most programs. For example, every internal client uses --trust-*, even though it disables spec mode.

[matanlurey]

I'm guessing that this either is as fixed as it will be, or as priorities allow.

Likely we will work on linting or other ways to prevent use of this API.

[sigmundch]

FWIW - @johnniwinther implemented a cool optimization so that .runtimeType can be used on certain patterns (like a.runtimeType == b.runtimeType, without incurring in the extra cost of reifying all type parameters. There are patterns that still makes us give up, but some patterns now are less expensive.

[johnniwinther]

Actually, in Dart 2, we never give up! We use the static type of the receiver in uses of e.runtimeType that we couldn't classify. So in code like

  method(Foo o) => o.runtimeType;

we only reify type parameters on subtypes of Foo. An even if we have

  method(Object o) => o.runtimeType;

we "only" reify type parameters on all generic classes, but do not for instance give up and assume that all passed type arguments are needed.