Skip to content

Commit 59c8081

Browse files
committed
[dart2wasm] Use shared closure argument dispatchers for dynamic call entries
This removes 4591 functions from ACX gallery main module which translates to -43 KB / -1.6% (-3.5% code section) We have already today a callsite guarantee that the closure call (type, positional, named) arguments are valid arguments to the target closure (they also have the right type). That means the `closure.vtable.dynamicCall` entry's only purpose is to unpack the (type, positional, named) argument arrays and call the target. Instead of calling the target directly (as we did so far) we now unpack argument arrays and call the right vtable entry. This logic can be shared amongst all closures of the same representation and therefore leads to big reduction in wasm functions. => We do that in this CL. There's two exceptions to this: * In dynamic module scenario we don't have closed-world knowledge of closure definitions & closure call site. There's no specific vtable entries for positional+name combinations we could forward to. * In closed world scenario where there's a usage of `Function.apply` with named arguments: We don't generate vtable entries for all possible name combinations a closure can be called with. So we change the closure layouter algorithm to find out if there's a usage of `Function.apply` with named arguments. A few tangential changes: * Fix a bug revealed by this change: The static tearoff instantiation constant's dynamic call entry must pass the generic closure object when calling the generic closure. => The shared dynamic call entry dispatchers will now verify (in assertion) mode the assumptions, which revealed this issue * The closure layouter algorithm will now consider `obj.foo(a: ...)` as a potential dynamic call site (due to call-via-field) and therefore record the name combinations used there => Tested via `web/wasm/closures/dynamic_call_via_field_test` We test the optimization by checking in 3 tests that show what ends up in the vtables: * `pkg/dart2wasm/test/ir_tests/dyn_closure.dart` => uses dynamic calls => dynamic call entries are "closure arguments dispatcher" * `pkg/dart2wasm/test/ir_tests/dyn_closure_function_apply.dart` => uses Function.apply without names => dynamic call entries are "closure arguments dispatcher" * `pkg/dart2wasm/test/ir_tests/dyn_closure_function_apply_named.dart` => uses Function.apply with named arguments => dynamic call entries are closure specific Issue #60458 Change-Id: I099984b542b05920b02596410a1bf6a08d2a0302 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/460080 Reviewed-by: Ömer Ağacan <[email protected]>
1 parent 43dd090 commit 59c8081

18 files changed

+812
-67
lines changed

pkg/dart2wasm/lib/closures.dart

Lines changed: 54 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -59,10 +59,12 @@ class ClosureImplementation {
5959
/// an entry for each (non-empty) combination of argument names that closures
6060
/// with this layout can be called with.
6161
class ClosureRepresentation {
62-
/// The struct field index in the vtable struct at which the function
63-
/// entries start.
62+
/// The number of type arguments.
6463
final int typeCount;
6564

65+
/// The maximum number of positional parameters.
66+
final int maxPositionalCount;
67+
6668
/// The Wasm struct type for the vtable.
6769
final w.StructType vtableStruct;
6870

@@ -129,13 +131,16 @@ class ClosureRepresentation {
129131
ClosureRepresentation(
130132
Translator translator,
131133
this.typeCount,
134+
this.maxPositionalCount,
132135
this.vtableStruct,
133136
this.closureStruct,
134137
this._indexOfCombination,
135138
this.instantiationContextStruct);
136139

137140
bool get isGeneric => typeCount > 0;
138141

142+
bool get hasNamed => _indexOfCombination != null;
143+
139144
/// Where the vtable entries for function calls start in the vtable struct.
140145
int get vtableBaseIndex => isGeneric
141146
? ClosureLayouter.vtableBaseIndexGeneric
@@ -216,6 +221,10 @@ class ClosureLayouter extends RecursiveVisitor {
216221
// The member currently being visited while collecting function signatures.
217222
Member? currentMember;
218223

224+
/// Whether the kernel [Component] uses `Function.apply` and possibly passes
225+
/// non-empty map for named arguments.
226+
late bool usesFunctionApplyWithNamedArguments;
227+
219228
// For non-generic closures. The entries are:
220229
// 0: Dynamic call entry
221230
// 1-...: Entries for calling the closure
@@ -400,8 +409,9 @@ class ClosureLayouter extends RecursiveVisitor {
400409
(translator.component.metadata["vm.procedure-attributes.metadata"]
401410
as ProcedureAttributesMetadataRepository)
402411
.mapping;
403-
404412
void collect() {
413+
usesFunctionApplyWithNamedArguments = false;
414+
405415
// Dynamic module enabled builds use dynamic call entry points for all
406416
// closure invocations so we don't need to generate any representation
407417
// info.
@@ -478,12 +488,12 @@ class ClosureLayouter extends RecursiveVisitor {
478488

479489
ClosureRepresentation _createRepresentation(
480490
int typeCount,
481-
int positionalCount,
491+
int maxPositionalCount,
482492
List<String> names,
483493
ClosureRepresentation? parent,
484494
Map<NameCombination, int>? indexOfCombination,
485495
Iterable<int> paramCounts) {
486-
List<String> nameTags = ["$typeCount", "$positionalCount", ...names];
496+
List<String> nameTags = ["$typeCount", "$maxPositionalCount", ...names];
487497
String vtableName = ["#Vtable", ...nameTags].join("-");
488498
String closureName = ["#Closure", ...nameTags].join("-");
489499
w.StructType parentVtableStruct = parent?.vtableStruct ??
@@ -508,7 +518,7 @@ class ClosureLayouter extends RecursiveVisitor {
508518
if (typeCount > 0) {
509519
// Add or set vtable field for the instantiation function.
510520
instantiatedRepresentation =
511-
getClosureRepresentation(0, positionalCount, names)!;
521+
getClosureRepresentation(0, maxPositionalCount, names)!;
512522
w.RefType inputType = w.RefType.def(closureBaseStruct, nullable: false);
513523
w.RefType outputType = w.RefType.def(
514524
instantiatedRepresentation.closureStruct,
@@ -564,6 +574,7 @@ class ClosureLayouter extends RecursiveVisitor {
564574
ClosureRepresentation representation = ClosureRepresentation(
565575
translator,
566576
typeCount,
577+
maxPositionalCount,
567578
vtableStruct,
568579
closureStruct,
569580
indexOfCombination,
@@ -618,7 +629,7 @@ class ClosureLayouter extends RecursiveVisitor {
618629
vtableBaseIndexNonGeneric + instantiationTrampolines.length,
619630
vtableStruct,
620631
vtableBaseIndexGeneric +
621-
(positionalCount + 1) +
632+
(maxPositionalCount + 1) +
622633
genericIndex)
623634
: translator
624635
.getDummyValuesCollectorForModule(module)
@@ -1008,8 +1019,27 @@ class ClosureLayouter extends RecursiveVisitor {
10081019
currentMember = null;
10091020
}
10101021

1022+
@override
1023+
void visitStaticInvocation(StaticInvocation node) {
1024+
super.visitStaticInvocation(node);
1025+
if (node.target == translator.functionApply) {
1026+
// Function.apply(function, positionalArguments, [namedArguments])
1027+
if (node.arguments.positional.length > 2) {
1028+
usesFunctionApplyWithNamedArguments = true;
1029+
}
1030+
}
1031+
}
1032+
1033+
@override
1034+
void visitStaticTearOff(StaticTearOff node) {
1035+
visitStaticTearOffConstantReference(StaticTearOffConstant(node.target));
1036+
}
1037+
10111038
@override
10121039
void visitStaticTearOffConstantReference(StaticTearOffConstant constant) {
1040+
if (constant.target == translator.functionApply) {
1041+
usesFunctionApplyWithNamedArguments = true;
1042+
}
10131043
_visitFunctionNode(constant.function);
10141044
}
10151045

@@ -1028,9 +1058,23 @@ class ClosureLayouter extends RecursiveVisitor {
10281058

10291059
@override
10301060
void visitDynamicInvocation(DynamicInvocation node) {
1031-
if (node.name.text == "call") {
1032-
_visitFunctionInvocation(node.arguments);
1033-
}
1061+
// NOTE: One may have two different kinds of calls here:
1062+
// ```
1063+
// dynamic x;
1064+
// x(namedArg: 1);
1065+
// x.foo(namedArg: 1);
1066+
// ```
1067+
// It may appear as we only have to handle the first case, namely
1068+
// `node.name.text == "call"`, but the second case can also be a
1069+
// dynamic closure callsite via call-through-field:
1070+
// ```
1071+
// class Foo {
1072+
// void Function({int? namedArg}) get foo => ...;
1073+
// }
1074+
// ```
1075+
// then a `x.foo(namedArg: 1)` will be executed at runtime via
1076+
// `var tmp = x.foo; tmp(namedArg: 1)`.
1077+
_visitFunctionInvocation(node.arguments);
10341078
super.visitDynamicInvocation(node);
10351079
}
10361080
}

pkg/dart2wasm/lib/constants.dart

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1180,12 +1180,12 @@ class ConstantCreator extends ConstantVisitor<ConstantInfo?>
11801180

11811181
final b = function.body;
11821182

1183-
final closureLocal = function.locals[0];
11841183
final typeArgsListLocal = function.locals[1]; // empty
11851184
final posArgsListLocal = function.locals[2];
11861185
final namedArgsListLocal = function.locals[3];
11871186

1188-
b.local_get(closureLocal);
1187+
constants.instantiateConstant(
1188+
b, tearOffConstantInfo.constant, translator.topTypeNonNullable);
11891189
constants.instantiateConstant(
11901190
b, typeArgsArrayConstantInfo.constant, typeArgsListLocal.type);
11911191
b.local_get(posArgsListLocal);

pkg/dart2wasm/lib/intrinsics.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2021,7 +2021,7 @@ class Intrinsifier {
20212021
b.end();
20222022
b.local_set(posArgsLocal);
20232023

2024-
// Convert named argument map to list, to be passed to shape and type
2024+
// Convert named argument map to array, to be passed to shape and type
20252025
// checkers and the dynamic call entry.
20262026
final namedArgsListLocal =
20272027
b.addLocal(translator.nullableObjectArrayTypeRef);

pkg/dart2wasm/lib/kernel_nodes.dart

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,8 @@ mixin KernelNodes {
276276
index.getTopLevelProcedure("dart:core", "_runtimeTypeEquals");
277277
late final Procedure runtimeTypeHashCode =
278278
index.getTopLevelProcedure("dart:core", "_runtimeTypeHashCode");
279+
late final Procedure? functionApply =
280+
index.tryGetProcedure('dart:core', 'Function', 'apply');
279281

280282
// dart:core invocation/exception procedures
281283
late final Procedure invocationGetterFactory =

0 commit comments

Comments
 (0)