[dart2wasm] Pass Dart double as f64 to JS interop

Don't convert Dart `double`s to `externref`s when calling JS interop
functions, pass them as `double`.

We pass other unboxed values (`int`s and `bool`s) as `externref`s, as
before:

- `int`s are most efficiently passed as `externref`s, as small integers
  can be converted to `i31ref` and externalized without allocation.

- `bool`s passed as `i32` mostly work because JS treats 0 as false and
  everything else as true, but the values can still be observed as
  numbers rather than bools, which causes some test failures.

Changes:

- Make raw interop procedures take `double` as argument, when the Dart
  type for the interop function argument is non-nullable `double`.

- Pass static type of the value and expected type (by the interop
  function) to `jsifyValue`.

- `jsifyValue` then takes the static type and expected type into account
  to avoid conversions when both are `double`s.

- `jsifyValue` is refactored to avoid the type conversion mapping
  allocation on every call.

New benchmark result before the changes:

    WasmJSInterop.call.void.1ArgsDouble(RunTimeRaw): 0.018275229357798167 ns.

After:

    WasmJSInterop.call.void.1ArgsDouble(RunTimeRaw): 0.014034965034965034 ns.

Issue: #60357
Change-Id: Ia70671f9a8e14f359f1119beda123e94aacdd2cd
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/422480
Commit-Queue: Ömer Ağacan <[email protected]>
Reviewed-by: Srujan Gaddam <[email protected]>

Author: osa1

benchmarks/WasmJSInterop/WasmJSInterop.dart

@@ -19,12 +19,15 @@ import 'package:benchmark_harness/benchmark_harness.dart';
 @JS()
 external void eval(String code);
 
-// This returns `void` to avoid adding `dartify` overheads to the benchmark
+// These return `void` to avoid adding `dartify` overheads to the benchmark
 // results.
-// V8 can't figure out this doesn't do anything so the loop and JS calls aren't
-// eliminated.
+// V8 can't figure out that these don't do anything so the loops and JS calls
+// aren't eliminated.
 @JS()
-external void intId(int i);
+external void intFun(int i);
+
+@JS()
+external void doubleFun(double d);
 
 // Run benchmarked code for at least 2 seconds.
 const int minimumMeasureDurationMillis = 2000;
@@ -38,15 +41,34 @@ class IntPassingBenchmark {
   double measure() =>
       BenchmarkBase.measureFor(() {
         for (int i = start; i < end; i += 1) {
-          intId(i);
+          intFun(i);
         }
       }, minimumMeasureDurationMillis) /
       (end - start);
 }
 
+class DoublePassingBenchmark {
+  final double start;
+  final double step;
+  final int calls;
+
+  DoublePassingBenchmark(this.start, this.step, this.calls);
+
+  double measure() =>
+      BenchmarkBase.measureFor(() {
+        double d = start;
+        for (int i = 0; i < calls; i += 1) {
+          doubleFun(d);
+          d *= step;
+        }
+      }, minimumMeasureDurationMillis) /
+      calls;
+}
+
 void main() {
   eval('''
-    self.intId = (i) => i;
+    self.intFun = (i) => i;
+    self.doubleFun = (d) => d;
     ''');
 
   final maxI31 = (1 << 30) - 1;
@@ -56,6 +78,12 @@ void main() {
 
   final large = IntPassingBenchmark(maxI31 + 1, maxI31 + 1000001).measure();
   report('WasmJSInterop.call.void.1ArgsInt', large);
+
+  // Have more than one call site to the `double` benchmark to avoid inlining
+  // too much, and for fair comparison with the `int` benchmark above.
+  DoublePassingBenchmark(1.0, 1.0, 10).measure();
+  final double = DoublePassingBenchmark(1.0, 12.34, 1000000).measure();
+  report('WasmJSInterop.call.void.1ArgsDouble', double);
 }
 
 /// Reports in Golem-specific format.

pkg/dart2wasm/lib/js/callback_specializer.dart

@@ -53,7 +53,8 @@ class CallbackSpecializer {
         type: callExpr.getStaticType(_staticTypeContext),
         isSynthesized: true);
 
-    final jsified = jsifyValue(temp, _util, _staticTypeContext.typeEnvironment);
+    final jsified = jsifyValue(temp, _util.nullableWasmExternRefType, _util,
+        _staticTypeContext.typeEnvironment);
 
     return ReturnStatement(Let(temp, jsified));
   }

pkg/dart2wasm/lib/js/interop_specializer.dart

@@ -80,10 +80,16 @@ abstract class _Specializer {
     // Initialize variable declarations.
     List<String> jsParameterStrings = [];
     List<VariableDeclaration> dartPositionalParameters = [];
-    for (int j = 0; j < parameters.length; j++) {
-      String parameterString = 'x$j';
+    for (int i = 0; i < parameters.length; i++) {
+      final VariableDeclaration parameter = parameters[i];
+      final DartType parameterType = parameter.type;
+      final interopFunctionParameterType =
+          parameterType == _util.coreTypes.doubleNonNullableRawType
+              ? _util.coreTypes.doubleNonNullableRawType
+              : _util.nullableWasmExternRefType;
+      String parameterString = 'x$i';
       dartPositionalParameters.add(VariableDeclaration(parameterString,
-          type: _util.nullableWasmExternRefType, isSynthesized: true));
+          type: interopFunctionParameterType, isSynthesized: true));
       jsParameterStrings.add(parameterString);
     }
 
@@ -130,12 +136,19 @@ abstract class _ProcedureSpecializer extends _Specializer {
   @override
   Expression specialize() {
     // Return the replacement body.
-    Expression invocation = StaticInvocation(
-        _getOrCreateInteropProcedure(),
-        Arguments(parameters
-            .map<Expression>((variable) => jsifyValue(variable, factory._util,
-                factory._staticTypeContext.typeEnvironment))
-            .toList()));
+    final interopProcedure = _getOrCreateInteropProcedure();
+    final interopProcedureType =
+        interopProcedure.computeSignatureOrFunctionType();
+    final List<Expression> jsifiedArguments = [];
+    for (int i = 0; i < parameters.length; i += 1) {
+      jsifiedArguments.add(jsifyValue(
+          parameters[i],
+          interopProcedureType.positionalParameters[i],
+          factory._util,
+          factory._staticTypeContext.typeEnvironment));
+    }
+    final invocation =
+        StaticInvocation(interopProcedure, Arguments(jsifiedArguments));
     return _util.castInvocationForReturn(invocation, function.returnType);
   }
 }
@@ -225,18 +238,23 @@ abstract class _PositionalInvocationSpecializer extends _InvocationSpecializer {
   Expression specialize() {
     // Create or get the specialized procedure for the invoked number of
     // arguments. Cast as needed and return the final invocation.
-    final staticInvocation = StaticInvocation(
-        _getOrCreateInteropProcedure(),
-        Arguments(invocation.arguments.positional.map<Expression>((expr) {
-          final temp = VariableDeclaration(null,
-              initializer: expr,
-              type: expr.getStaticType(factory._staticTypeContext),
-              isSynthesized: true);
-          return Let(
-              temp,
-              jsifyValue(temp, factory._util,
-                  factory._staticTypeContext.typeEnvironment));
-        }).toList()));
+    final interopProcedure = _getOrCreateInteropProcedure();
+    final interopProcedureType =
+        interopProcedure.computeSignatureOrFunctionType();
+    final List<Expression> jsifiedArguments = [];
+    final List<Expression> arguments = invocation.arguments.positional;
+    for (int i = 0; i < arguments.length; i += 1) {
+      final temp = VariableDeclaration(null,
+          initializer: arguments[i],
+          type: arguments[i].getStaticType(factory._staticTypeContext),
+          isSynthesized: true);
+      jsifiedArguments.add(Let(
+          temp,
+          jsifyValue(temp, interopProcedureType.positionalParameters[i],
+              factory._util, factory._staticTypeContext.typeEnvironment)));
+    }
+    final staticInvocation =
+        StaticInvocation(interopProcedure, Arguments(jsifiedArguments));
     return _util.castInvocationForReturn(
         staticInvocation, invocation.getStaticType(_staticTypeContext));
   }
@@ -321,21 +339,24 @@ class _ObjectLiteralSpecializer extends _InvocationSpecializer {
     for (NamedExpression expr in invocation.arguments.named) {
       namedArgs[expr.name] = expr.value;
     }
+    final interopProcedureType =
+        interopProcedure.computeSignatureOrFunctionType();
     final arguments =
         parameters.map<Expression>((decl) => namedArgs[decl.name!]!).toList();
-    final positionalArgs = arguments.map<Expression>((expr) {
+    final List<Expression> jsifiedArguments = [];
+    for (int i = 0; i < arguments.length; i += 1) {
       final temp = VariableDeclaration(null,
-          initializer: expr,
-          type: expr.getStaticType(_staticTypeContext),
+          initializer: arguments[i],
+          type: arguments[i].getStaticType(factory._staticTypeContext),
           isSynthesized: true);
-      return Let(
+      jsifiedArguments.add(Let(
           temp,
-          jsifyValue(
-              temp, factory._util, factory._staticTypeContext.typeEnvironment));
-    }).toList();
+          jsifyValue(temp, interopProcedureType.positionalParameters[i],
+              factory._util, factory._staticTypeContext.typeEnvironment)));
+    }
     assert(factory._extensionIndex.isStaticInteropType(function.returnType));
     return invokeOneArg(_util.jsValueBoxTarget,
-        StaticInvocation(interopProcedure, Arguments(positionalArgs)));
+        StaticInvocation(interopProcedure, Arguments(jsifiedArguments)));
   }
 }
 

pkg/dart2wasm/lib/js/util.dart

@@ -92,6 +92,41 @@ class CoreTypesUtil {
   final Class jsArrayBufferImplClass;
   final Class byteBufferClass;
 
+  // NB. We rely on iteration ordering being insertion order to handle subtypes
+  // before supertypes to convert as `int` and `double` before `num`.
+  late final Map<Class, Procedure> _externRefConverterMap = {
+    coreTypes.boolClass: toJSBoolean,
+    coreTypes.intClass: jsifyInt,
+    coreTypes.doubleClass: toJSNumber,
+    coreTypes.numClass: jsifyNum,
+    jsValueClass: jsifyJSValue,
+    coreTypes.stringClass: jsifyString,
+    jsInt8ArrayImplClass: jsifyJSInt8ArrayImpl,
+    jsUint8ArrayImplClass: jsifyJSUint8ArrayImpl,
+    jsUint8ClampedArrayImplClass: jsifyJSUint8ClampedArrayImpl,
+    jsInt16ArrayImplClass: jsifyJSInt16ArrayImpl,
+    jsUint16ArrayImplClass: jsifyJSUint16ArrayImpl,
+    jsInt32ArrayImplClass: jsifyJSInt32ArrayImpl,
+    jsUint32ArrayImplClass: jsifyJSUint32ArrayImpl,
+    jsFloat32ArrayImplClass: jsifyJSFloat32ArrayImpl,
+    jsFloat64ArrayImplClass: jsifyJSFloat64ArrayImpl,
+    int8ListClass: jsInt8ArrayFromDartInt8List,
+    uint8ListClass: jsUint8ArrayFromDartUint8List,
+    uint8ClampedListClass: jsUint8ClampedArrayFromDartUint8ClampedList,
+    int16ListClass: jsInt16ArrayFromDartInt16List,
+    uint16ListClass: jsUint16ArrayFromDartUint16List,
+    int32ListClass: jsInt32ArrayFromDartInt32List,
+    uint32ListClass: jsUint32ArrayFromDartUint32List,
+    float32ListClass: jsFloat32ArrayFromDartFloat32List,
+    float64ListClass: jsFloat64ArrayFromDartFloat64List,
+    jsDataViewImplClass: jsifyJSDataViewImpl,
+    byteDataClass: jsifyByteData,
+    coreTypes.listClass: jsifyRawList,
+    jsArrayBufferImplClass: jsifyJSArrayBufferImpl,
+    byteBufferClass: jsArrayBufferFromDartByteBuffer,
+    coreTypes.functionClass: jsifyFunction,
+  };
+
   CoreTypesUtil(this.coreTypes, this.extensionIndex)
       : allowInteropTarget = coreTypes.index
             .getTopLevelProcedure('dart:js_util', 'allowInterop'),
@@ -367,6 +402,43 @@ class CoreTypesUtil {
     }
     return AsExpression(expression, staticType);
   }
+
+  /// Return the function to convert a value with type [valueType] to Dart
+  /// interop type [expectedType].
+  ///
+  /// [expectedType] can be any interop type, but for now this only handles the
+  /// interop types generated by [_Specializer._getRawInteropProcedure].
+  ///
+  /// `null` return value means no conversion is needed, the value can be passed
+  /// to the interop function directly.
+  ///
+  /// The argument passed to the returned conversion function needs to be
+  /// non-nullable. This function does not check the nullability of [valueType]
+  /// and assume that the argument passed to the conversion function won't be
+  /// `null`.
+  Procedure? _conversionProcedure(
+      DartType valueType, DartType expectedType, TypeEnvironment typeEnv) {
+    if (expectedType == coreTypes.doubleNonNullableRawType) {
+      assert(valueType is InterfaceType &&
+          valueType.classNode == coreTypes.doubleClass);
+      return null;
+    }
+
+    assert(expectedType == nullableWasmExternRefType,
+        'Unexpected expected type: $expectedType');
+
+    for (final entry in _externRefConverterMap.entries) {
+      if (typeEnv.isSubtypeOf(
+          valueType,
+          InterfaceType(entry.key, Nullability.nonNullable),
+          SubtypeCheckMode.withNullabilities)) {
+        return entry.value;
+      }
+    }
+
+    // `dynamic` or `Object?`, convert based on runtime type.
+    return jsifyRawTarget;
+  }
 }
 
 StaticInvocation invokeOneArg(Procedure target, Expression arg) =>
@@ -383,20 +455,22 @@ InstanceInvocation invokeMethod(VariableDeclaration receiver, Procedure target,
 bool parametersNeedParens(List<String> parameters) =>
     parameters.isEmpty || parameters.length > 1;
 
-Expression jsifyValue(VariableDeclaration variable, CoreTypesUtil coreTypes,
-    TypeEnvironment typeEnv) {
-  final Procedure conversionProcedure;
+Expression jsifyValue(VariableDeclaration variable, DartType expectedType,
+    CoreTypesUtil coreTypes, TypeEnvironment typeEnv) {
+  final Procedure? conversionProcedure;
 
   if (coreTypes.extensionIndex.isStaticInteropType(variable.type) ||
       coreTypes.extensionIndex.isExternalDartReferenceType(variable.type)) {
     conversionProcedure = coreTypes.jsValueUnboxTarget;
   } else {
     conversionProcedure =
-        _conversionProcedure(variable.type, coreTypes, typeEnv);
+        coreTypes._conversionProcedure(variable.type, expectedType, typeEnv);
   }
 
-  final conversion =
-      StaticInvocation(conversionProcedure, Arguments([VariableGet(variable)]));
+  final conversion = conversionProcedure == null
+      ? VariableGet(variable)
+      : StaticInvocation(
+          conversionProcedure, Arguments([VariableGet(variable)]));
 
   if (variable.type.isPotentiallyNullable) {
     return ConditionalExpression(
@@ -408,54 +482,3 @@ Expression jsifyValue(VariableDeclaration variable, CoreTypesUtil coreTypes,
     return conversion;
   }
 }
-
-Procedure _conversionProcedure(
-    DartType type, CoreTypesUtil util, TypeEnvironment typeEnv) {
-  // NB. We rely on iteration ordering being insertion order to handle subtypes
-  // before supertypes to convert as `int` and `double` before `num`.
-  final Map<Class, Procedure> converterMap = {
-    util.coreTypes.boolClass: util.toJSBoolean,
-    util.coreTypes.intClass: util.jsifyInt,
-    util.coreTypes.doubleClass: util.toJSNumber,
-    util.coreTypes.numClass: util.jsifyNum,
-    util.jsValueClass: util.jsifyJSValue,
-    util.coreTypes.stringClass: util.jsifyString,
-    util.jsInt8ArrayImplClass: util.jsifyJSInt8ArrayImpl,
-    util.jsUint8ArrayImplClass: util.jsifyJSUint8ArrayImpl,
-    util.jsUint8ClampedArrayImplClass: util.jsifyJSUint8ClampedArrayImpl,
-    util.jsInt16ArrayImplClass: util.jsifyJSInt16ArrayImpl,
-    util.jsUint16ArrayImplClass: util.jsifyJSUint16ArrayImpl,
-    util.jsInt32ArrayImplClass: util.jsifyJSInt32ArrayImpl,
-    util.jsUint32ArrayImplClass: util.jsifyJSUint32ArrayImpl,
-    util.jsFloat32ArrayImplClass: util.jsifyJSFloat32ArrayImpl,
-    util.jsFloat64ArrayImplClass: util.jsifyJSFloat64ArrayImpl,
-    util.int8ListClass: util.jsInt8ArrayFromDartInt8List,
-    util.uint8ListClass: util.jsUint8ArrayFromDartUint8List,
-    util.uint8ClampedListClass:
-        util.jsUint8ClampedArrayFromDartUint8ClampedList,
-    util.int16ListClass: util.jsInt16ArrayFromDartInt16List,
-    util.uint16ListClass: util.jsUint16ArrayFromDartUint16List,
-    util.int32ListClass: util.jsInt32ArrayFromDartInt32List,
-    util.uint32ListClass: util.jsUint32ArrayFromDartUint32List,
-    util.float32ListClass: util.jsFloat32ArrayFromDartFloat32List,
-    util.float64ListClass: util.jsFloat64ArrayFromDartFloat64List,
-    util.jsDataViewImplClass: util.jsifyJSDataViewImpl,
-    util.byteDataClass: util.jsifyByteData,
-    util.coreTypes.listClass: util.jsifyRawList,
-    util.jsArrayBufferImplClass: util.jsifyJSArrayBufferImpl,
-    util.byteBufferClass: util.jsArrayBufferFromDartByteBuffer,
-    util.coreTypes.functionClass: util.jsifyFunction,
-  };
-
-  for (final entry in converterMap.entries) {
-    if (typeEnv.isSubtypeOf(
-        type,
-        InterfaceType(entry.key, Nullability.nonNullable),
-        SubtypeCheckMode.withNullabilities)) {
-      return entry.value;
-    }
-  }
-
-  // `dynamic` or `Object?`, convert based on runtime type.
-  return util.jsifyRawTarget;
-}