async/Future causes erasable generic types to be retained

[spebbe]

Hi! dart2js 2.9.1 on mac, but behaves similarly on at least 2.7 and 2.8.

This is probably not a minimal example, but it's small-ish :-) Consider the following implementation and use of a minimal Option type:

abstract class Option<A> {
  B fold<B>(B ifNone(), B ifSome(A a));

  Option<B> map<B>(B Function(A a) f) => fold(() => None(), (a) => Some(f(a)));
}

class Some<A> extends Option<A> {
  final A a;

  Some(this.a);

  @override B fold<B>(B Function() ifNone, B Function(A a) ifSome) => ifSome(a);

  @override String toString() => "Some($a)";
}

class None<A> extends Option<A> {
  @override B fold<B>(B Function() ifNone, B Function(A a) ifSome) => ifNone();

  @override String toString() => "None";
}

void run() {
  print(Some("hello").map((s) => s + "!"));
}

void main() {
  run();
}

When compiled with dart2js --lax-runtime-type-to-string --omit-implicit-checks --trust-primitives, the rti optimizer correctly omits the generic type parameters for the Option class hierarchy from the generated js.

Modifying main (not run()!) to

void main() async {
  run();
}

or even

void main() {
  new Future(() {});
  run();
}

, causes the generic types to be retained in the generated js. Is this the expected behaviour? If so, is there some workaround for it? In this simple example, the consequences aren't too severe, but for more complex structures and in large programs that intentionally try to stay away from depending on reified types, the performance penalty and increased code size becomes problematic.

Thanks!

[sigmundch]

/cc @fishythefish

[fishythefish]

This doesn't ring a bell. I'll investigate when I have some time.

[fishythefish]

Sorry for the delay, but I have an explanation of what's going on here.

In order to repro, you don't actually need async. For example, adding the following line somewhere should also result in the same issue:

print(null is void Function());

It doesn't matter where this goes as long as it's called, the value null is irrelevant, and print is just to ensure the code is live. Even the particular function type doesn't matter as long as we're checking against some function type, and the check could be implicit rather than explicit.

When we compile code that has a type check against a function type F, we process each local function/closure in order to check if its type is a potential subtype of F. (In other words, is there some instantiation of type variables which would cause it to be a subtype of F?) If so, then that local function/closure needs type arguments at runtime in order to perform the type check.

I discovered a couple months ago that our implementation of the potential subtype check was unsound, and we decided to conservatively assume that it always returns true until a proper implementation is written. (Here's the change.) Unfortunately, this means that as soon as you have a function type check anywhere in your code, we assume that all local functions/closures need all of their type arguments, and in your example, this causes their enclosing scopes to also require them. Larger apps often don't have this regression because they need the type arguments for other reasons anyway or because they already have type checks against broad function types.

Where does async come into this? Well, as soon as we see any async code, we pull in all the bits of machinery that make async work, such as zones and error callbacks. Somewhere in the async internals, there's a check against the function type dynamic Function(Object, StackTrace) (which is presumably some kind of error handler), and this leads to the same problem as above.

---

@sigmundch Let me know if you feel it's worth reprioritizing the isPotentialSubtype implementation.

[spebbe]

Thank you so much for your investigation and thorough explanation, @fishythefish! Your explanation makes a lot of sense, but I believe there might be multiple factors at play here though, since this behaviour is present in dart2js 2.7.0 (and possibly earlier releases) as well, released several months before the change you linked to was applied. print(null is void Function()); also triggers the behaviour on 2.7.0.

I fully understand if you cannot give this a high priority at the moment, but if it makes any difference, micro benchmarks indicate that successful rti optimization/type erasure would significantly improve performance and code size of two projects I maintain, one commercial web app (https://www.soundtrap.com) and one relatively popular open source library (https://github.com/spebbe/dartz). Both code bases intentionally refrain from relying on reified types as much as possible, in order to avoid the associated runtime/size costs.

Alternatively, an explicit type reification opt-out mechanism, as discussed in #36610, would significantly decrease the need for automatic rti optimizations.

Again, many thanks!

[fishythefish]

No problem, @spebbe!

On 2.7.0, I'd expect this to depend on which function type you test against. isPotentialSubtype wasn't returning true unconditionally, but we were still using it to determine when closures needed type arguments/signatures.

For instance, in this example code, ifNone has type B Function(). If B is instantiated to void, then we get void Function(). Therefore B Function() is a potential subtype of void Function(), so ifNone needs type arguments at runtime in case it participates in this type test. (Of course, only the value null can ever be tested, but we don't take this into account.)

Does the issue go away if you use a function type that cannot possibly match your closures?

Unfortunately, you're right that we can't prioritize this at the moment, although I'm hoping to fix the potential subtype implementation for fun on a rainy day. However, we have some other ideas for making generics less expensive generally, so hopefully that helps mitigate some of the regression.