Optimize js_util callConstructor for 0-4 arguments.

Some usages of `callConstructor` will have unnecessary checks removed, when checks on the arguments can be elided. The compilers will optimize further if they can. Example optimizations: https://paste.googleplex.com/4594240957972480 Change-Id: I0e6e7e4d1268580cbfab84599b1c9da6fc64e7c7 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/213114 Reviewed-by: Srujan Gaddam <srujzs@google.com> Commit-Queue: Riley Porter <rileyporter@google.com>
2024-10-14 09:43:57 +00:00 · 2021-09-13 22:43:10 +00:00 · 2021-09-13 22:43:10 +00:00 · 0dd3c97147
parent 14940d1c9c
commit 0dd3c97147
4 changed files with 423 additions and 32 deletions
--- a/pkg/_js_interop_checks/lib/src/transformations/js_util_optimizer.dart
+++ b/pkg/_js_interop_checks/lib/src/transformations/js_util_optimizer.dart
@ -14,6 +14,8 @@ class JsUtilOptimizer extends Transformer {
  final Procedure _jsTarget;
  final Procedure _callMethodTarget;
  final List<Procedure> _callMethodUncheckedTargets;
+  final Procedure _callConstructorTarget;
+  final List<Procedure> _callConstructorUncheckedTargets;
  final Procedure _getPropertyTarget;
  final Procedure _setPropertyTarget;
  final Procedure _setPropertyUncheckedTarget;
@ -43,6 +45,12 @@ class JsUtilOptimizer extends Transformer {
            5,
            (i) => _coreTypes.index.getTopLevelProcedure(
                'dart:js_util', '_callMethodUnchecked$i')),
+        _callConstructorTarget = _coreTypes.index
+            .getTopLevelProcedure('dart:js_util', 'callConstructor'),
+        _callConstructorUncheckedTargets = List<Procedure>.generate(
+            5,
+            (i) => _coreTypes.index.getTopLevelProcedure(
+                'dart:js_util', '_callConstructorUnchecked$i')),
        _getPropertyTarget = _coreTypes.index
            .getTopLevelProcedure('dart:js_util', 'getProperty'),
        _setPropertyTarget = _coreTypes.index
@ -90,6 +98,8 @@ class JsUtilOptimizer extends Transformer {
      node = _lowerSetProperty(node);
    } else if (node.target == _callMethodTarget) {
      node = _lowerCallMethod(node);
+    } else if (node.target == _callConstructorTarget) {
+      node = _lowerCallConstructor(node);
    }
    node.transformChildren(this);
    return node;
@ -147,70 +157,102 @@ class JsUtilOptimizer extends Transformer {
    assert(arguments.positional.length == 3);
    assert(arguments.named.isEmpty);

-    // Lower List.empty factory call.
-    var argumentsList = arguments.positional.last;
+    return _lowerToCallUnchecked(
+        node, _callMethodUncheckedTargets, arguments.positional.sublist(0, 2));
+  }
+
+  /// Lowers the given js_util `callConstructor` call to `_callConstructorUncheckedN`
+  /// when the additional validation checks on the arguments can be elided.
+  ///
+  /// Calls will be lowered when using a List literal or constant list with 0-4
+  /// elements for the `callConstructor` arguments, or the `List.empty()` factory.
+  /// Removing the checks allows further inlining by the compilers.
+  StaticInvocation _lowerCallConstructor(StaticInvocation node) {
+    Arguments arguments = node.arguments;
+    assert(arguments.types.isEmpty);
+    assert(arguments.positional.length == 2);
+    assert(arguments.named.isEmpty);
+
+    return _lowerToCallUnchecked(
+        node, _callConstructorUncheckedTargets, [arguments.positional.first]);
+  }
+
+  /// Helper to lower the given [node] to the relevant unchecked target in the
+  /// [callUncheckedTargets] based on whether the validation checks on the
+  /// [originalArguments] can be elided.
+  ///
+  /// Calls will be lowered when using a List literal or constant list with 0-4
+  /// arguments, or the `List.empty()` factory. Removing the checks allows further
+  /// inlining by the compilers.
+  StaticInvocation _lowerToCallUnchecked(
+      StaticInvocation node,
+      List<Procedure> callUncheckedTargets,
+      List<Expression> originalArguments) {
+    var argumentsList = node.arguments.positional.last;
+    // Lower arguments in a List.empty factory call.
    if (argumentsList is StaticInvocation &&
        argumentsList.target == _listEmptyFactory) {
-      return _createNewCallMethodNode([], arguments, node.fileOffset);
+      return _createCallUncheckedNode(callUncheckedTargets, [],
+          originalArguments, node.fileOffset, node.arguments.fileOffset);
    }

-    // Lower other kinds of Lists.
-    var callMethodArguments;
+    // Lower arguments in other kinds of Lists.
+    var callUncheckedArguments;
    var entryType;
    if (argumentsList is ListLiteral) {
-      if (argumentsList.expressions.length >=
-          _callMethodUncheckedTargets.length) {
+      if (argumentsList.expressions.length >= callUncheckedTargets.length) {
        return node;
      }
-      callMethodArguments = argumentsList.expressions;
+      callUncheckedArguments = argumentsList.expressions;
      entryType = argumentsList.typeArgument;
    } else if (argumentsList is ConstantExpression &&
        argumentsList.constant is ListConstant) {
      var argumentsListConstant = argumentsList.constant as ListConstant;
-      if (argumentsListConstant.entries.length >=
-          _callMethodUncheckedTargets.length) {
+      if (argumentsListConstant.entries.length >= callUncheckedTargets.length) {
        return node;
      }
-      callMethodArguments = argumentsListConstant.entries
+      callUncheckedArguments = argumentsListConstant.entries
          .map((constant) => ConstantExpression(
              constant, constant.getType(_staticTypeContext)))
          .toList();
      entryType = argumentsListConstant.typeArgument;
    } else {
-      // Skip lowering any other type of List.
+      // Skip lowering arguments in any other type of List.
      return node;
    }

-    // Check the overall List entry type, then verify each argument if needed.
+    // Check the arguments List type, then verify each argument if needed.
    if (!_allowedInteropType(entryType)) {
-      for (var argument in callMethodArguments) {
+      for (var argument in callUncheckedArguments) {
        if (!_allowedInterop(argument)) {
          return node;
        }
      }
    }

-    return _createNewCallMethodNode(
-        callMethodArguments, arguments, node.fileOffset);
+    return _createCallUncheckedNode(
+        callUncheckedTargets,
+        callUncheckedArguments,
+        originalArguments,
+        node.fileOffset,
+        node.arguments.fileOffset);
  }

-  /// Creates a new StaticInvocation node for `_callMethodUncheckedN` with the
-  /// given 0-4 arguments.
-  StaticInvocation _createNewCallMethodNode(
-      List<Expression> callMethodArguments,
-      Arguments arguments,
-      int nodeFileOffset) {
-    assert(callMethodArguments.length <= 4);
+  /// Creates a new StaticInvocation node for the relevant unchecked target
+  /// with the given 0-4 arguments.
+  StaticInvocation _createCallUncheckedNode(
+      List<Procedure> callUncheckedTargets,
+      List<Expression> callUncheckedArguments,
+      List<Expression> originalArguments,
+      int nodeFileOffset,
+      int argumentsFileOffset) {
+    assert(callUncheckedArguments.length <= 4);
    return StaticInvocation(
-        _callMethodUncheckedTargets[callMethodArguments.length],
+        callUncheckedTargets[callUncheckedArguments.length],
        Arguments(
-          [
-            arguments.positional[0],
-            arguments.positional[1],
-            ...callMethodArguments
-          ],
+          [...originalArguments, ...callUncheckedArguments],
          types: [],
-        )..fileOffset = arguments.fileOffset)
+        )..fileOffset = argumentsFileOffset)
      ..fileOffset = nodeFileOffset;
  }

--- a/sdk/lib/js_util/js_util.dart
+++ b/sdk/lib/js_util/js_util.dart
@ -195,6 +195,38 @@ dynamic callConstructor(Object constr, List<Object?>? arguments) {
  //     return _wrapToDart(jsObj);
 }

+/// Unchecked version for 0 arguments, only used in a CFE transformation.
+@pragma('dart2js:tryInline')
+dynamic _callConstructorUnchecked0(Object constr) {
+  return JS('Object', 'new #()', constr);
+}
+
+/// Unchecked version for 1 argument, only used in a CFE transformation.
+@pragma('dart2js:tryInline')
+dynamic _callConstructorUnchecked1(Object constr, Object? arg1) {
+  return JS('Object', 'new #(#)', constr, arg1);
+}
+
+/// Unchecked version for 2 arguments, only used in a CFE transformation.
+@pragma('dart2js:tryInline')
+dynamic _callConstructorUnchecked2(Object constr, Object? arg1, Object? arg2) {
+  return JS('Object', 'new #(#, #)', constr, arg1, arg2);
+}
+
+/// Unchecked version for 3 arguments, only used in a CFE transformation.
+@pragma('dart2js:tryInline')
+dynamic _callConstructorUnchecked3(
+    Object constr, Object? arg1, Object? arg2, Object? arg3) {
+  return JS('Object', 'new #(#, #, #)', constr, arg1, arg2, arg3);
+}
+
+/// Unchecked version for 4 arguments, only used in a CFE transformation.
+@pragma('dart2js:tryInline')
+dynamic _callConstructorUnchecked4(
+    Object constr, Object? arg1, Object? arg2, Object? arg3, Object? arg4) {
+  return JS('Object', 'new #(#, #, #, #)', constr, arg1, arg2, arg3, arg4);
+}
+
 /// Exception for when the promise is rejected with a `null` or `undefined`
 /// value.
 ///
--- a/tests/lib/js/js_util/properties_test.dart
+++ b/tests/lib/js/js_util/properties_test.dart
@ -85,6 +85,19 @@ class CallMethodTest {
  external five(a, b, c, d, e);
 }

+@JS()
+external get Zero;
+@JS()
+external get One;
+@JS()
+external get Two;
+@JS()
+external get Three;
+@JS()
+external get Four;
+@JS()
+external get Five;
+
 main() {
  eval(r"""
    function Foo(a) {
@ -157,6 +170,25 @@ main() {
    CallMethodTest.prototype.five = function(a, b, c, d, e) {
      return 'five';
    }
+
+    function Zero() {
+      this.count = 0;
+    }
+    function One(a) {
+      this.count = 1;
+    }
+    function Two(a, b) {
+      this.count = 2;
+    }
+    function Three(a, b, c) {
+      this.count = 3;
+    }
+    function Four(a, b, c, d) {
+      this.count = 4;
+    }
+    function Five(a, b, c, d, e) {
+      this.count = 5;
+    }
    """);

  group('newObject', () {
@ -543,8 +575,131 @@ main() {

  group('callConstructor', () {
    test('typed object', () {
-      Foo f = js_util.callConstructor(JSFooType, [42]);
+      var f = js_util.callConstructor(JSFooType, [42]);
      expect(f.a, equals(42));
+
+      var f2 =
+          js_util.callConstructor(js_util.getProperty(f, 'constructor'), [5]);
+      expect(f2.a, equals(5));
+    });
+
+    test('typed literal', () {
+      ExampleTypedLiteral literal = js_util.callConstructor(
+          js_util.getProperty(ExampleTypedLiteral(), 'constructor'), []);
+      expect(literal.a, equals(null));
+    });
+
+    test('callConstructor with List edge cases', () {
+      expect(
+          js_util.getProperty(
+              js_util.callConstructor(Zero, List.empty()), 'count'),
+          equals(0));
+      expect(
+          js_util.getProperty(
+              js_util.callConstructor(Zero, List<int>.empty()), 'count'),
+          equals(0));
+
+      expect(
+          js_util.getProperty(
+              js_util.callConstructor(Two, List<int>.filled(2, 0)), 'count'),
+          equals(2));
+      expect(
+          js_util.getProperty(
+              js_util.callConstructor(Three, List<int>.generate(3, (i) => i)),
+              'count'),
+          equals(3));
+
+      Iterable<String> iterableStrings = <String>['foo', 'bar'];
+      expect(
+          js_util.getProperty(
+              js_util.callConstructor(Two, List.of(iterableStrings)), 'count'),
+          equals(2));
+
+      const l1 = [1, 2];
+      const l2 = [3, 4];
+      expect(
+          js_util.getProperty(
+              js_util.callConstructor(Four, List.from(l1)..addAll(l2)),
+              'count'),
+          equals(4));
+      expect(
+          js_util.getProperty(js_util.callConstructor(Four, l1 + l2), 'count'),
+          equals(4));
+      expect(
+          js_util.getProperty(
+              js_util.callConstructor(Four, List.unmodifiable([1, 2, 3, 4])),
+              'count'),
+          equals(4));
+
+      var setElements = {1, 2};
+      expect(
+          js_util.getProperty(
+              js_util.callConstructor(Two, setElements.toList()), 'count'),
+          equals(2));
+
+      var spreadList = [1, 2, 3];
+      expect(
+          js_util.getProperty(
+              js_util.callConstructor(Four, [1, ...spreadList]), 'count'),
+          equals(4));
+    });
+
+    test('edge cases for lowering to _callConstructorUncheckedN', () {
+      expect(js_util.getProperty(js_util.callConstructor(Zero, []), 'count'),
+          equals(0));
+      expect(js_util.getProperty(js_util.callConstructor(One, [1]), 'count'),
+          equals(1));
+      expect(
+          js_util.getProperty(
+              js_util.callConstructor(Four, [1, 2, 3, 4]), 'count'),
+          equals(4));
+      expect(
+          js_util.getProperty(
+              js_util.callConstructor(Five, [1, 2, 3, 4, 5]), 'count'),
+          equals(5));
+
+      // List with a type declaration, short circuits element checking
+      expect(
+          js_util.getProperty(
+              js_util.callConstructor(Two, <int>[1, 2]), 'count'),
+          equals(2));
+
+      // List as a variable instead of a List Literal or constant
+      var list = [1, 2];
+      expect(js_util.getProperty(js_util.callConstructor(Two, list), 'count'),
+          equals(2));
+
+      // Mixed types of elements to check in the given list.
+      var x = 4;
+      var str = 'cat';
+      var b = false;
+      var evens = [2, 4, 6];
+      expect(
+          js_util.getProperty(
+              js_util.callConstructor(Four, [x, str, b, evens]), 'count'),
+          equals(4));
+      var obj = Object();
+      expect(js_util.getProperty(js_util.callConstructor(One, [obj]), 'count'),
+          equals(1));
+      var nullElement = null;
+      expect(
+          js_util.getProperty(
+              js_util.callConstructor(One, [nullElement]), 'count'),
+          equals(1));
+
+      // const lists.
+      expect(
+          js_util.getProperty(js_util.callConstructor(One, const [3]), 'count'),
+          equals(1));
+      const constList = [10, 20, 30];
+      expect(
+          js_util.getProperty(
+              js_util.callConstructor(Three, constList), 'count'),
+          equals(3));
+      expect(
+          js_util.getProperty(
+              js_util.callConstructor(One, DartClass.staticConstList), 'count'),
+          equals(1));
    });
  });
 }
--- a/tests/lib_2/js/js_util/properties_test.dart
+++ b/tests/lib_2/js/js_util/properties_test.dart
@ -87,6 +87,19 @@ class CallMethodTest {
  external five(a, b, c, d, e);
 }

+@JS()
+external get Zero;
+@JS()
+external get One;
+@JS()
+external get Two;
+@JS()
+external get Three;
+@JS()
+external get Four;
+@JS()
+external get Five;
+
 main() {
  eval(r"""
    function Foo(a) {
@ -159,6 +172,25 @@ main() {
    CallMethodTest.prototype.five = function(a, b, c, d, e) {
      return 'five';
    }
+
+    function Zero() {
+      this.count = 0;
+    }
+    function One(a) {
+      this.count = 1;
+    }
+    function Two(a, b) {
+      this.count = 2;
+    }
+    function Three(a, b, c) {
+      this.count = 3;
+    }
+    function Four(a, b, c, d) {
+      this.count = 4;
+    }
+    function Five(a, b, c, d, e) {
+      this.count = 5;
+    }
    """);

  group('newObject', () {
@ -547,8 +579,138 @@ main() {

  group('callConstructor', () {
    test('typed object', () {
-      Foo f = js_util.callConstructor(JSFooType, [42]);
+      var f = js_util.callConstructor(JSFooType, [42]);
      expect(f.a, equals(42));
+
+      var f2 =
+          js_util.callConstructor(js_util.getProperty(f, 'constructor'), [5]);
+      expect(f2.a, equals(5));
+    });
+
+    test('typed literal', () {
+      ExampleTypedLiteral literal = js_util.callConstructor(
+          js_util.getProperty(ExampleTypedLiteral(), 'constructor'), []);
+      expect(literal.a, equals(null));
+    });
+
+    test('callConstructor with List edge cases', () {
+      expect(
+          js_util.getProperty(js_util.callConstructor(Zero, List()), 'count'),
+          equals(0));
+      expect(
+          js_util.getProperty(
+              js_util.callConstructor(Zero, List<int>()), 'count'),
+          equals(0));
+      expect(
+          js_util.getProperty(
+              js_util.callConstructor(Zero, List.empty()), 'count'),
+          equals(0));
+      expect(
+          js_util.getProperty(
+              js_util.callConstructor(Zero, List<int>.empty()), 'count'),
+          equals(0));
+
+      expect(
+          js_util.getProperty(
+              js_util.callConstructor(Two, List<int>.filled(2, 0)), 'count'),
+          equals(2));
+      expect(
+          js_util.getProperty(
+              js_util.callConstructor(Three, List<int>.generate(3, (i) => i)),
+              'count'),
+          equals(3));
+
+      Iterable<String> iterableStrings = <String>['foo', 'bar'];
+      expect(
+          js_util.getProperty(
+              js_util.callConstructor(Two, List.of(iterableStrings)), 'count'),
+          equals(2));
+
+      const l1 = [1, 2];
+      const l2 = [3, 4];
+      expect(
+          js_util.getProperty(
+              js_util.callConstructor(Four, List.from(l1)..addAll(l2)),
+              'count'),
+          equals(4));
+      expect(
+          js_util.getProperty(js_util.callConstructor(Four, l1 + l2), 'count'),
+          equals(4));
+      expect(
+          js_util.getProperty(
+              js_util.callConstructor(Four, List.unmodifiable([1, 2, 3, 4])),
+              'count'),
+          equals(4));
+
+      var setElements = {1, 2};
+      expect(
+          js_util.getProperty(
+              js_util.callConstructor(Two, setElements.toList()), 'count'),
+          equals(2));
+
+      var spreadList = [1, 2, 3];
+      expect(
+          js_util.getProperty(
+              js_util.callConstructor(Four, [1, ...spreadList]), 'count'),
+          equals(4));
+    });
+
+    test('edge cases for lowering to _callConstructorUncheckedN', () {
+      expect(js_util.getProperty(js_util.callConstructor(Zero, []), 'count'),
+          equals(0));
+      expect(js_util.getProperty(js_util.callConstructor(One, [1]), 'count'),
+          equals(1));
+      expect(
+          js_util.getProperty(
+              js_util.callConstructor(Four, [1, 2, 3, 4]), 'count'),
+          equals(4));
+      expect(
+          js_util.getProperty(
+              js_util.callConstructor(Five, [1, 2, 3, 4, 5]), 'count'),
+          equals(5));
+
+      // List with a type declaration, short circuits element checking
+      expect(
+          js_util.getProperty(
+              js_util.callConstructor(Two, <int>[1, 2]), 'count'),
+          equals(2));
+
+      // List as a variable instead of a List Literal or constant
+      var list = [1, 2];
+      expect(js_util.getProperty(js_util.callConstructor(Two, list), 'count'),
+          equals(2));
+
+      // Mixed types of elements to check in the given list.
+      var x = 4;
+      var str = 'cat';
+      var b = false;
+      var evens = [2, 4, 6];
+      expect(
+          js_util.getProperty(
+              js_util.callConstructor(Four, [x, str, b, evens]), 'count'),
+          equals(4));
+      var obj = Object();
+      expect(js_util.getProperty(js_util.callConstructor(One, [obj]), 'count'),
+          equals(1));
+      var nullElement = null;
+      expect(
+          js_util.getProperty(
+              js_util.callConstructor(One, [nullElement]), 'count'),
+          equals(1));
+
+      // const lists.
+      expect(
+          js_util.getProperty(js_util.callConstructor(One, const [3]), 'count'),
+          equals(1));
+      const constList = [10, 20, 30];
+      expect(
+          js_util.getProperty(
+              js_util.callConstructor(Three, constList), 'count'),
+          equals(3));
+      expect(
+          js_util.getProperty(
+              js_util.callConstructor(One, DartClass.staticConstList), 'count'),
+          equals(1));
    });
  });
 }