Using `ExternalDartReference` as a function parameter is expensive

[leonsenft]

Consider the following example. Passing a Dart function with a JS incompatible parameter requires wrapping it to do conversions on the argument(s) and return type:

extension type WritableSignal<T>(JSFunction _) implements Signal<T> {
  // JS binding requires JSExportedDartFunction.
  @JS('u')
  external void _update(JSExportedDartFunction update);

  // Dart API uses generic T, making it incompatible with `toJS`.
  void update(T Function(T) function) {
    _update(_jsCompatibleFunction(function).toJS);
  }

  // Wrap the Dart function to convert the arguments and return value to `toJS` compatible types.
  ExternalDartReference? Function(ExternalDartReference?) _jsCompatibleFunction(
    T Function(T) function,
  ) {
    return (value) {
      return function(trustAs(value.toDartObject)).toExternalReference;
    };
  }
}

This produces an extra closure object, and despite using techniques to optimize away the cost of the conversions (see https://github.com/dart-lang/sdk/issues/55339), the closure remains:

  A = {
    WritableSignal__jsCompatibleFunction(_this, $function) {
      return new A.WritableSignal__jsCompatibleFunction_closure($function);
    },
    WritableSignal__jsCompatibleFunction_closure: function WritableSignal__jsCompatibleFunction_closure(t0) {
      this.$function = t0;
    },
    main() {
      ...
      y.u(t1._as(A.allowInterop(A.WritableSignal__jsCompatibleFunction(y, new A.main_closure0()))));
      ...
    },
    main_closure0: function main_closure0() {
    },
  };
  A.WritableSignal__jsCompatibleFunction_closure.prototype = {
    call$1(value) {
      return this.$function.call$1(value);
    }
  };
  A.main_closure0.prototype = {
    call$1(value) {
      return value + 1;
    }
  }

Note that the A.WritableSignal__jsCompatibleFunction() call doesn't actually do anything. It simply forwards its argument to the closure it wraps. This code is equivalent to:

  A = {
    main() {
      ...
      y.u(t1._as(A.allowInterop(new A.main_closure0())));
      ...
    },
    main_closure0: function main_closure0() {
    },
  };
  A.main_closure0.prototype = {
    call$1(value) {
      return value + 1;
    }
  }

For reference this is essentially what I had before migrating to dart:js_interop, so I think there's still room to improve here and get performance parity with package:js.

As a side note, if the Dart function has no parameter, and only a return type, we can cast/convert it directly:

Signal<T> computed<T>(T Function() computation) =>
    _computed(trustAs<ExternalDartReference Function()>(computation).toJS);

This doesn't work with the example above because we can't cast a Function type with a parameter to a broader type.

[sigmundch]

@leonsenft - can you clarify, is this function that you are passing to JS going to be invoked from JS, or only passed back to Dart opaquely?

[leonsenft]

Technically it's invoked from JS, but directly through a Dart call.

When the signal is instantiated, it initializes the update() method here.

When we call update in Dart:

value.update((current) => current + 1);

The callback is passed through update() and gets called here.

[sigmundch]

Thanks! I see what you mean now.

So tricky!

@srujzs - I'm wondering if it's possible for the interop library to have a higher order conversion. That is, just like .toJS converts functions that already expect interop-types in it's signatures, maybe there could be a version of allow-interop that includes parameter conversions in the process? Since allow-interop already creates a JS closure, maybe we could piggyback the argument conversion cost into it as well, rather than having a second closure in the user-level code?

Not sure how compatible this is with the Wasm support, though.

More generally - we also currently don't specialize the conversions we use in allowinterop, so potentially we could also do better there. For example, the current default will use Function.apply to splice arguments in place, but maybe we can generate different code for .toJS when we statically know how many arguments it takes. Since 0, 1, and 2 arg callabcks are the most common, we could have special logic for those (and include a subset of possible conversions if needed.)

[srujzs]

That is, just like .toJS converts functions that already expect interop-types in it's signatures, maybe there could be a version of allow-interop that includes parameter conversions in the process? Since allow-interop already creates a JS closure, maybe we could piggyback the argument conversion cost into it as well, rather than having a second closure in the user-level code?

So essentially a toJS that auto-converts its parameters and return value e.g. ((T param) => param).toJSAutoConvert? In theory, on dart2wasm, we jsify and dartify as needed on parameter and return values, so this could be extended to support that. However, if we had a parameter type like List<JSAny> it's not clear if we're converting a JS array or internalizing a Dart List so we'd need to be careful of that. In general, it's likely to result in slower code on dart2wasm. On the JS backends, conversions are free for most if not all JS types, so we might be able to get away with doing nothing.

More generally - we also currently don't specialize the conversions we use in allowinterop, so potentially we could also do better there. For example, the current default will use Function.apply to splice arguments in place, but maybe we can generate different code for .toJS when we statically know how many arguments it takes.

I think it's a good idea to speed up Function.toJS and avoid unnecessary calls. It may also allow us to handle a different feature request to allow additional args to be passed to toJS'd closures.

---

As a side comment, I'm a little confused by this:

This doesn't work with the example above because we can't cast a Function type with a parameter to a broader type.

Since we're eliding casts with trustAs, I'm confused where the cast would fail if we were to convert a T Function(T) to an ExternalDartReference? Function(ExternalDartReference?).

[leonsenft]

Since we're eliding casts with trustAs, I'm confused where the cast would fail if we were to convert a T Function(T) to an ExternalDartReference? Function(ExternalDartReference?).

Yeah that confused me as well. If I replace my implementation of _jsCompatibleFunction() with a cast:

  @JS('u')
  external void _update(JSExportedDartFunction update);

  /// Update the value of the signal based on its current value, and notify any
  /// dependents.
  void update(T Function(T) function) {
    _update(trustAs<ExternalDartReference? Function(ExternalDartReference?)>(
            function)
        .toJS);
  }

It produces this error at runtime:

  TypeError: Instance of '(int) => int': type '(int) => int' is not a subtype of type '(Object?) => Object?'
  dart:sdk_internal                                         _installSpecializedAsCheck
  dart/angular/signals/lib/src/optimizations.dart:5:38      trustAs
  dart/angular/signals/lib/signals.dart:27:13               WritableSignal.update

I'm not sure what _installSpecializedAsCheck is, but it would appear as though the @pragma('dart2js:as:trust') might be ignored?

[leonsenft]

Oh, does DDC respect @pragma('dart2js:as:trust') or will it still perform the cast? I think maybe the issue is that DDC still performs the as cast, right?

[leonsenft]

Oh, does DDC respect @pragma('dart2js:as:trust') or will it still perform the cast? I think maybe the issue is that DDC still performs the as cast, right?

Confirmed that the test passes with dart2js, but not DDC. Is there a pragma for DDC?

[srujzs]

Oooh, sorry, I see what you meant by parameter now - yes, the contravariance is an issue. I don't think there's a way for DDC to elide type-checks with pragmas.

[srujzs]

I mention one possible solution in https://github.com/dart-lang/sdk/issues/55536, where we make ExternalDartReference generic with a bound of T extends Object?. That would allow something like (I'm using U as WritableSignal's type parameter so that we don't confuse the different type parameters):

void update(U Function(U) function) {
  _update((function as ExternalDartReference<U> Function(
          ExternalDartReference<U>))
      .toJS);
}

The other alternative might be to make the bound T extends Object (which I prefer, extension types that wrap a nullable value are weird, especially when you throw the concept of undefined into the mix). I think we can still do the above, with some changes:

void update(U? Function(U?) function) {
  _update((function as ExternalDartReference<U>? Function(
          ExternalDartReference<U>?))
      .toJS);
}

Here, U would need to be bound to Object in WritableSignal. set could be updated to take a U?. This avoids the need for the wrapper function.

---

Of course, either way, such a cast won't work on dart2wasm (I figure you likely don't care about dart2wasm for now). We'd need to do something like the proposal in https://github.com/dart-lang/sdk/issues/55342#issuecomment-2030332990 for a consistent API. I'm generally against such an API for at least two reasons:

1. I mention this above, but in some cases, it's ambiguous what auto-conversion is desired from a given Dart type.
2. It'll encourage slower code because it's easier to use.

That being said, an interesting alternative to this is to have a "Function.toJSAllExternalReferences" where the only applicable conversion is to and from ExternalDartReference<T>. In other words, all input to this resulting JSFunction would undergo a toDartObject, and the output would undergo a toExternalReference. Basically, by using this conversion, you're claiming the original function only ever gets called with Dart values and the result only ever is used in Dart. The function passed to update would apply in this case.