Duplicate wrapper functions generated as a result of `Function.toJS`

[osa1]

import 'dart:js_interop';

double dartFunction1(double x, [double y = 1.0]) {
  print("Dart function 1 called with x = $x, y = $y");
  return x + y;
}

double dartFunction2(double x, [double y = 1.0]) {
  print("Dart function 2 called with x = $x, y = $y");
  return x - y;
}

@JS()
external void callDartFromJS(JSFunction f);

void main() {
  callDartFromJS(dartFunction1.toJS);
  callDartFromJS(dartFunction2.toJS);
}

In this program we generate these two identical trampolines for dartFunction1 and dartFunction2:

Trampoline 1

(func $_266 (;384;) (export "_266") (param $callback (;0;) anyref) (param $argumentsLengthWasmI32 (;1;) i32) (param $x1 (;2;) externref) (param $x2 (;3;) externref) (result externref)
  (local $var4 (ref $#Top))
  (local $argumentsLength i64)
  (local $var6 (ref null $#Top))
  (local $var7 (ref $#Closure-0-2))
  (local $var8 f64)
  (local $var9 f64)
  (local $var10 i64)
  (local $var11 (ref null $#Top))
  (local $var12 (ref $#Closure-0-1))
  (local $var13 f64)
  (local $var14 i64)
  local.get $callback
  ref.cast $#Top
  local.set $var4
  local.get $argumentsLengthWasmI32
  i64.extend_i32_s
  local.set $argumentsLength
  local.get $argumentsLength
  i64.const 2
  i64.ge_s
  if
    local.get $var4
    ref.cast $#Closure-0-2
    local.tee $var7
    struct.get $#Closure-0-2 $field2
    local.get $x1
    ref.null none
    call $dartifyRaw
    call $<obj> as double
    local.set $var8
    i32.const 75
    local.get $var8
    struct.new $BoxedDouble
    local.get $x2
    ref.null none
    call $dartifyRaw
    call $<obj> as double
    local.set $var9
    i32.const 75
    local.get $var9
    struct.new $BoxedDouble
    local.get $var7
    struct.get $#Closure-0-2 $field3
    struct.get $#Vtable-0-2 $field3
    call_ref $type191
    local.set $var6
    local.get $var6
    ref.is_null
    if (result externref)
      call $WasmExternRef.nullRef
    else
      local.get $var6
      call $jsifyRaw
    end
    return
  end
  local.get $argumentsLength
  local.set $var10
  i32.const 57
  local.get $var10
  struct.new $BoxedInt
  global.get $global628
  call $identical
  if
    local.get $var4
    ref.cast $#Closure-0-1
    local.tee $var12
    struct.get $#Closure-0-1 $field2
    local.get $x1
    ref.null none
    call $dartifyRaw
    call $<obj> as double
    local.set $var13
    i32.const 75
    local.get $var13
    struct.new $BoxedDouble
    local.get $var12
    struct.get $#Closure-0-1 $field3
    struct.get $#Vtable-0-1 $field2
    call_ref $type185
    local.set $var11
    local.get $var11
    ref.is_null
    if (result externref)
      call $WasmExternRef.nullRef
    else
      local.get $var11
      call $jsifyRaw
    end
    return
  end
  global.get $global672
  local.get $argumentsLength
  local.set $var14
  i32.const 57
  local.get $var14
  struct.new $BoxedInt
  global.get $global673
  call $JSStringImpl._interpolate3
  call $Error._throwWithCurrentStackTrace
  unreachable
)

Trampoline 2

(func $_267 (;393;) (export "_267") (param $callback (;0;) anyref) (param $argumentsLengthWasmI32 (;1;) i32) (param $x1 (;2;) externref) (param $x2 (;3;) externref) (result externref)
  (local $var4 (ref $#Top))
  (local $argumentsLength i64)
  (local $var6 (ref null $#Top))
  (local $var7 (ref $#Closure-0-2))
  (local $var8 f64)
  (local $var9 f64)
  (local $var10 i64)
  (local $var11 (ref null $#Top))
  (local $var12 (ref $#Closure-0-1))
  (local $var13 f64)
  (local $var14 i64)
  local.get $callback
  ref.cast $#Top
  local.set $var4
  local.get $argumentsLengthWasmI32
  i64.extend_i32_s
  local.set $argumentsLength
  local.get $argumentsLength
  i64.const 2
  i64.ge_s
  if
    local.get $var4
    ref.cast $#Closure-0-2
    local.tee $var7
    struct.get $#Closure-0-2 $field2
    local.get $x1
    ref.null none
    call $dartifyRaw
    call $<obj> as double
    local.set $var8
    i32.const 75
    local.get $var8
    struct.new $BoxedDouble
    local.get $x2
    ref.null none
    call $dartifyRaw
    call $<obj> as double
    local.set $var9
    i32.const 75
    local.get $var9
    struct.new $BoxedDouble
    local.get $var7
    struct.get $#Closure-0-2 $field3
    struct.get $#Vtable-0-2 $field3
    call_ref $type191
    local.set $var6
    local.get $var6
    ref.is_null
    if (result externref)
      call $WasmExternRef.nullRef
    else
      local.get $var6
      call $jsifyRaw
    end
    return
  end
  local.get $argumentsLength
  local.set $var10
  i32.const 57
  local.get $var10
  struct.new $BoxedInt
  global.get $global628
  call $identical
  if
    local.get $var4
    ref.cast $#Closure-0-1
    local.tee $var12
    struct.get $#Closure-0-1 $field2
    local.get $x1
    ref.null none
    call $dartifyRaw
    call $<obj> as double
    local.set $var13
    i32.const 75
    local.get $var13
    struct.new $BoxedDouble
    local.get $var12
    struct.get $#Closure-0-1 $field3
    struct.get $#Vtable-0-1 $field2
    call_ref $type185
    local.set $var11
    local.get $var11
    ref.is_null
    if (result externref)
      call $WasmExternRef.nullRef
    else
      local.get $var11
      call $jsifyRaw
    end
    return
  end
  global.get $global672
  local.get $argumentsLength
  local.set $var14
  i32.const 57
  local.get $var14
  struct.new $BoxedInt
  global.get $global673
  call $JSStringImpl._interpolate3
  call $Error._throwWithCurrentStackTrace
  unreachable
)

While wasm-opt is able to de-duplicate them, the wrappers are still a lot of code, and we're currently spending a lot of time in wasm-opt when compiling Flutter apps. It would make sense to avoid generating duplicate wrappers in dart2wasm.

Alternatively, we could specialize these wrappers based on the callback. Currently the wrappers are calling the callbacks indirectly:

local.get $var12
struct.get $#Closure-0-1 $field3
struct.get $#Vtable-0-1 $field2
call_ref $type185

But since they're specific to a single Dart function, they could call the Dart function directly, with a call instruction.

Note that this will probably cause binary size increase as wasm-opt is currently removing duplicate wrappers, and the wrappers won't be duplicate when we specialize them based on callbacks they call.

Given that we want to improve binary sizes and compilation times, and we have no applications right now where JS-to-Dart calls are a bottleneck, I think it would make sense to cache and share wrapper functions in dart2wasm output. @srujzs @mkustermann any thoughts?

[srujzs]

We could generate procedures based on the unique type and shape and indeed cache them. We do something similar when we use object literal constructors so we don't generate too many procedures:

sdk/pkg/dart2wasm/lib/js/interop_specializer.dart

Line 307 in 5de031c

// To avoid one method for every invocation, we optimize and compute one

. Can we do something similar for the procedure that calls the imported JS function as well (the JS function returns a JS function, which when called, is what calls the trampoline)?

We may have to be a little careful here with generics. Currently, there's a notion of a "cast closure" we pass along so that we can capture the type parameters at the invocation-site and then later do the casts in the trampoline. The procedures generated there would need to be treated differently than those without the cast closure.

But since they're specific to a single Dart function, they could call the Dart function directly, with a call instruction.

IIUC, this will only work if we know the target (instance members would be complicated too). The current semantics assume a tear-off, so it's also a little awkward to wrap a function that never gets called but JSExportedDartFunction.toDart has to return something I suppose.