Avoid type casts in Future implementation due to usage of untyped function

[mkustermann]

The _FutureListener implementation looks like this:

class _FutureListener<S, T> {
  ...
  final Function? callback;
  ...
  _FutureListener.then(
      this.result, FutureOr<T> Function(S) onValue, Function? errorCallback)
      : callback = onValue ...;

  _FutureListener.thenAwait(
      this.result, _FutureOnValue<S, T> onValue, Function errorCallback)
      : callback = onValue ...;
  ...
  FutureOr<T> Function(S) get _onValue {
    assert(handlesValue);
    return callback as dynamic;
  }
  ...
  FutureOr<T> handleValue(S sourceResult) {
    return _zone.runUnary<FutureOr<T>, S>(_onValue, sourceResult);
  }
  ...
}

This causes a performance problem due to the runtime type check in _onValue from Function? to FutureOr<T> Function(S). The interesting thing is that we actually have correctly typed closures passed as arguments to the constructor, though then we "intentionally" loose those function types when assigning to the Function? callback.

Those function subtype checks are rather expensive in the VM and can cause apps to run over a performance cliff (once the size of a cache is exhausted). stc_overflow_in_future_impl.dart contains an example that causes a SubtypeTestCache overflow and makes the code call to runtime for every subsequent type check.

Even if we optimized those function type checks in the VM more - the fact that we have to do the runtime type checks is a performance issue for async heavy code.

@lrhn This is a big issue for async heavy code (including for some 1p customers). Do you think the code can be re-written to avoid the typechecks in _onValue, _errorTest, _whenCompleteAction?

(/cc @sstrickl This is a case of STC overflow for our 1p customer)

[mkustermann]

Given that the tests should succeed always, I think the easy solution would be to use unsafeCast<> there.
=> cl/230240

[rakudrama]

There are 5 fields instead of 7 if callback was split three ways.
It does not seem like a big win.
The memory saving is an illusion if the type tests are causing a large cache to exist that otherwise would not.
Would a type-safe 3-field version avoid the type check, and cache, or would there still be checks due to parametric covariance?

Is there some unfortunate coupling with the VM (callback is annotated)?

[mkustermann]

Would a type-safe 3-field version avoid the type check, and cache, or would there still be checks due to parametric covariance?

It would avoid the checks, the constructor would get the properly typed function, we would store into a field of the same type and don't have to cast when loading the field.

Though IMHO given how critical the future implementation is for perf (and possibly memory), I think it justifies using a unsafeCast<> - we do that in some other critical places as well.

[lrhn]

It's hard to avoid type checks in the error handler, because it has to accept both unary and binary callbacks. (I really want to just force binary callbacks, we never succeeded at making the stack trace not be allocated when you don't use it. Since you can always add a .catchError((e, s) {}) to a future at any later point, we always have to have the stacktrace anyway.)
On the other hand, errors are probably not on the critical performance path anyway. If you need to optimize error handling, you have too many errors.

We could add just one more field, and keep the then and test callbacks typed.

  FutureOr<T> Function(S)? _onValueCallback;
  Function? _onErrorOrWhenCompleteCallback;
  bool Function(Object)? _errorTestCallback;

Then the then branch, which is the vast majority of cases in practice, will be well-typed.
The catchError/test case will be typed, but that's just because we need a third field for that when the on-value callback is typed.
The whenComplete will need a cast, to dynamic Function(), which I hope we can be somewhat efficient about.

We still need to check the results of the callbacks for being futures, including is Future<T> for the value and error callbacks (just Future<void> for whenComplete). There is no way around that.

This conflation of fields is only because we try to avoid polymorphism. If each kind of handler could have its own class, we could be completely typed and have no superfluous fields. How efficient would we be able to make that? That is, is one polymorhic call more expensive than what are doing here?