Skip to content

Commit a1cca73

Browse files
mkustermannCommit Queue
authored andcommitted
[dart2wasm] Fix exponential compile time in dynamic closure calls
In the loop that has 2 recursive calls: with a name and without a name we should stop early if we already know that this is unreachable (e.g. all name combinations require a name or don't have a name). This should make us now have O(#name-combinations) recursive calls, emitting unreachables as soon as there's a set of names that aren't feasible in any name combination. Change-Id: I4c91a5fa91709398aa80ffd8dc278e30638acb0c Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/462400 Reviewed-by: Ömer Ağacan <omersa@google.com> Commit-Queue: Martin Kustermann <kustermann@google.com>
1 parent d46bd8c commit a1cca73

File tree

2 files changed

+196
-31
lines changed

2 files changed

+196
-31
lines changed

pkg/dart2wasm/lib/translator.dart

Lines changed: 72 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -2516,17 +2516,53 @@ class _ClosureArgumentsToVtableEntryDispatcherGenerator
25162516

25172517
// Check for each name whether it's there or not.
25182518
final allCombinations = representation.nameCombinations.toList();
2519-
final sortedNames = allCombinations.expand((nc) => nc.names).toList()
2520-
..sort();
2519+
final sortedNames =
2520+
allCombinations.expand((nc) => nc.names).toSet().toList()..sort();
25212521
final nameIndexVar = b.addLocal(w.NumType.i32);
25222522

2523-
void codeGenNamedHandling(
2524-
List<String> currentNames, List<w.ValueType> typeStack, int nameIndex) {
2525-
final currentCombination = NameCombination(currentNames);
2526-
final isValidCombination = currentNames.isEmpty ||
2527-
allCombinations.any((nc) => nc.compareTo(currentCombination) == 0);
2523+
int matchingCombinations(List<String> currentNames, int nextNameIndex) {
2524+
int prefixMatches = 0;
2525+
bool exactMatch = false;
2526+
if (nextNameIndex == 0) {
2527+
assert(currentNames.isEmpty);
2528+
exactMatch = true;
2529+
prefixMatches = 1 + allCombinations.length;
2530+
} else {
2531+
for (final nc in allCombinations) {
2532+
if (currentNames.length <= nc.names.length) {
2533+
bool found = true;
2534+
for (int i = 0; i < currentNames.length; ++i) {
2535+
if (currentNames[i] != nc.names[i]) {
2536+
found = false;
2537+
break;
2538+
}
2539+
}
2540+
if (found) {
2541+
if (currentNames.length == nc.names.length) {
2542+
prefixMatches++;
2543+
exactMatch = true;
2544+
} else {
2545+
if (sortedNames[nextNameIndex - 1]
2546+
.compareTo(nc.names[currentNames.length]) <
2547+
0) {
2548+
prefixMatches++;
2549+
}
2550+
}
2551+
}
2552+
}
2553+
}
2554+
}
2555+
return exactMatch ? prefixMatches : -prefixMatches;
2556+
}
2557+
2558+
final currentNames = <String>[];
25282559

2529-
if (isValidCombination) {
2560+
void generateNameHandling(int nextNameIndex) {
2561+
final match = matchingCombinations(currentNames, nextNameIndex);
2562+
final hasExactMatch = match > 0;
2563+
final hasNonExactMatches = match < 0 || match > 1;
2564+
final hasMoreMatches = match != 0;
2565+
if (hasExactMatch) {
25302566
b.comment('Check whether all named are loaded');
25312567
b.local_get(namedArgsLocal);
25322568
b.array_len();
@@ -2543,31 +2579,33 @@ class _ClosureArgumentsToVtableEntryDispatcherGenerator
25432579
.heapType as w.FunctionType);
25442580
b.return_();
25452581
b.end();
2546-
} else {
2547-
if (util.compilerAssertsEnabled) {
2548-
b.comment('Check there are more names passed by the caller,');
2549-
b.comment('because the currently processed name set');
2550-
b.comment('(which are: ${currentNames.join('-')}) does not');
2551-
b.comment(' correspond to a valid name combination.');
2552-
b.local_get(namedArgsLocal);
2553-
b.array_len();
2554-
b.local_get(nameIndexVar);
2555-
b.i32_eq();
2556-
b.if_();
2557-
b.comment('Unsupported name combination.');
2558-
b.comment('Maybe bug in closure representation building');
2582+
if (!hasNonExactMatches) {
2583+
b.comment('More names passed than expected.');
25592584
b.unreachable();
2560-
b.end();
2585+
return;
25612586
}
2562-
}
2563-
2564-
if (nameIndex == sortedNames.length) {
2565-
b.comment('More names passed then expected.');
2587+
} else if (hasMoreMatches && util.compilerAssertsEnabled) {
2588+
b.comment('Check there are more names passed by the caller,');
2589+
b.comment('because the currently processed name set');
2590+
b.comment('(which are: ${currentNames.join('-')}) does not');
2591+
b.comment(' correspond to a valid name combination.');
2592+
b.local_get(namedArgsLocal);
2593+
b.array_len();
2594+
b.local_get(nameIndexVar);
2595+
b.i32_eq();
2596+
b.if_();
2597+
b.comment('Unsupported name combination.');
2598+
b.comment('May be bug in closure representation building');
2599+
b.unreachable();
2600+
b.end();
2601+
} else {
2602+
b.comment('The names "${currentNames.join('-')}" are not part '
2603+
'of a used name combination.');
25662604
b.unreachable();
25672605
return;
25682606
}
25692607

2570-
final newName = sortedNames[nameIndex];
2608+
final newName = sortedNames[nextNameIndex];
25712609
final symbol = translator.symbols.symbolForNamedParameter(newName);
25722610

25732611
b.comment('Load next name and see if it corresponds to "$newName"');
@@ -2592,16 +2630,19 @@ class _ClosureArgumentsToVtableEntryDispatcherGenerator
25922630
b.i32_add();
25932631
b.local_set(nameIndexVar);
25942632

2595-
codeGenNamedHandling([...currentNames, newName],
2596-
[...typeStack, translator.topType], nameIndex + 1);
2633+
currentNames.add(newName);
2634+
typeStack.add(translator.topType);
2635+
generateNameHandling(nextNameIndex + 1);
2636+
typeStack.removeLast();
2637+
currentNames.removeLast();
25972638
}
25982639
b.end();
25992640

26002641
b.comment('Name "$newName" was *not* provided by caller.');
2601-
codeGenNamedHandling(currentNames, typeStack, nameIndex + 1);
2642+
generateNameHandling(nextNameIndex + 1);
26022643
}
26032644

2604-
codeGenNamedHandling([], typeStack, 0);
2645+
generateNameHandling(0);
26052646
}
26062647

26072648
// This function is purely used for checking assumptions made by the code this
Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
import 'package:expect/expect.dart';
6+
7+
void main() {
8+
final l = <dynamic>['a', 1, foo];
9+
final closure = l[int.parse('2')];
10+
11+
final sb = StringBuffer();
12+
13+
Expect.equals(foo(), closure());
14+
15+
Expect.equals(foo(a0: 0), closure(a0: 0));
16+
Expect.equals(foo(a1: 1), closure(a1: 1));
17+
Expect.equals(foo(a2: 2), closure(a2: 2));
18+
Expect.equals(foo(a3: 3), closure(a3: 3));
19+
Expect.equals(foo(a4: 4), closure(a4: 4));
20+
Expect.equals(foo(a5: 5), closure(a5: 5));
21+
Expect.equals(foo(a6: 6), closure(a6: 6));
22+
Expect.equals(foo(a7: 7), closure(a7: 7));
23+
Expect.equals(foo(a8: 8), closure(a8: 8));
24+
Expect.equals(foo(a9: 9), closure(a9: 9));
25+
Expect.equals(foo(a10: 10), closure(a10: 10));
26+
Expect.equals(foo(a11: 11), closure(a11: 11));
27+
Expect.equals(foo(a12: 12), closure(a12: 12));
28+
Expect.equals(foo(a13: 13), closure(a13: 13));
29+
Expect.equals(foo(a14: 14), closure(a14: 14));
30+
Expect.equals(foo(a15: 15), closure(a15: 15));
31+
32+
Expect.equals(foo(a0: 0, a1: 1), closure(a0: 0, a1: 1));
33+
Expect.equals(foo(a2: 2, a3: 3), closure(a2: 2, a3: 3));
34+
Expect.equals(foo(a4: 4, a5: 5), closure(a4: 4, a5: 5));
35+
Expect.equals(foo(a6: 6, a7: 7), closure(a6: 6, a7: 7));
36+
Expect.equals(foo(a8: 8, a9: 9), closure(a8: 8, a9: 9));
37+
Expect.equals(foo(a10: 10, a11: 11), closure(a10: 10, a11: 11));
38+
Expect.equals(foo(a12: 12, a13: 13), closure(a12: 12, a13: 13));
39+
Expect.equals(foo(a14: 14, a15: 15), closure(a14: 14, a15: 15));
40+
41+
Expect.equals(
42+
foo(a0: 0, a1: 1, a2: 2, a3: 3),
43+
closure(a0: 0, a1: 1, a2: 2, a3: 3),
44+
);
45+
Expect.equals(
46+
foo(a4: 4, a5: 5, a6: 6, a7: 7),
47+
closure(a4: 4, a5: 5, a6: 6, a7: 7),
48+
);
49+
Expect.equals(
50+
foo(a8: 8, a9: 9, a10: 10, a11: 11),
51+
closure(a8: 8, a9: 9, a10: 10, a11: 11),
52+
);
53+
Expect.equals(
54+
foo(a12: 12, a13: 13, a14: 14, a15: 15),
55+
closure(a12: 12, a13: 13, a14: 14, a15: 15),
56+
);
57+
58+
Expect.equals(
59+
foo(a0: 0, a1: 1, a2: 2, a3: 3, a4: 4, a5: 5, a6: 6, a7: 7),
60+
closure(a0: 0, a1: 1, a2: 2, a3: 3, a4: 4, a5: 5, a6: 6, a7: 7),
61+
);
62+
Expect.equals(
63+
foo(a8: 8, a9: 9, a10: 10, a11: 11, a12: 12, a13: 13, a14: 14, a15: 15),
64+
closure(a8: 8, a9: 9, a10: 10, a11: 11, a12: 12, a13: 13, a14: 14, a15: 15),
65+
);
66+
67+
Expect.equals(
68+
foo(
69+
a0: 0,
70+
a1: 1,
71+
a2: 2,
72+
a3: 3,
73+
a4: 4,
74+
a5: 5,
75+
a6: 6,
76+
a7: 7,
77+
a8: 8,
78+
a9: 9,
79+
a10: 10,
80+
a11: 11,
81+
a12: 12,
82+
a13: 13,
83+
a14: 14,
84+
a15: 15,
85+
),
86+
closure(
87+
a0: 0,
88+
a1: 1,
89+
a2: 2,
90+
a3: 3,
91+
a4: 4,
92+
a5: 5,
93+
a6: 6,
94+
a7: 7,
95+
a8: 8,
96+
a9: 9,
97+
a10: 10,
98+
a11: 11,
99+
a12: 12,
100+
a13: 13,
101+
a14: 14,
102+
a15: 15,
103+
),
104+
);
105+
}
106+
107+
String foo({
108+
a0,
109+
a1,
110+
a2,
111+
a3,
112+
a4,
113+
a5,
114+
a6,
115+
a7,
116+
a8,
117+
a9,
118+
a10,
119+
a11,
120+
a12,
121+
a13,
122+
a14,
123+
a15,
124+
}) => '$a0 $a1 $a2 $a3 $a4 $a5 $a6 $a7 $a8 $a9 $a10 $a11 $a12 $a13 $a14 $a15';

0 commit comments

Comments
 (0)