dart-lang/sdk
1
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

[ddc] Avoid revisiting subexpressions during hot reload invocation rewriting.

Browse Source 
In extreme cases with deeply nested invocations in closures, this can lead to an exponential recursive call pattern.

For example: https://github.com/spebbe/dartz/blob/8bf79e746d11e6a66c868027e5e1a25fdd270f45/lib/src/either.dart#L108

Moves all nested checks onto a single branch so that there is a single branch with no checks when the generation is the same, and another branch with all the necessary checks when the generation is different.

Bug: https://github.com/flutter/flutter/issues/173700
Change-Id: I3167a96e3ead67fd1d5c763ef2a7309d82c1a7c6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/445540
Commit-Queue: Nate Biggs <natebiggs@google.com>
Reviewed-by: Nicholas Shahan <nshahan@google.com>
This commit is contained in:
Nate Biggs
2025-08-22 20:48:35 -07:00
committed by Commit Queue
parent dab1af86db
commit e6f188b3fa
2 changed files with 318 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/dev_compiler/lib/src/kernel/compiler_new.dart
+93 -18
View File
@@ -248,6 +248,29 @@ class LibraryBundleCompiler implements old.Compiler {
}
}
/// Tracks the state of which branch the compiler is on for hot reload checks.
///
/// This allows hot reload generation checks to be batched into a single
/// branch statement.
///
/// This batched branching allows us to avoid exponential behavior when
/// recursing on deeply nested checked calls. Otherwise each branch makes 2
/// copies of all the sub-branches leading to 2^n checks.
enum HotReloadBranchState {
/// The compiler is not along any hot reload check branch yet.
none,
/// The compiler is along the branch with no extra checks. The check rewrite
/// logic will be skipped and the normal call will be generated. The root of
/// this branch will include a hot reload generation check.
uncheckedBranch,
/// The compiler is along the branch with extra checks. The check rewrite
/// logic will be applied for every call that needs it. The root of this
/// branch will include a hot reload generation check.
checkedBranch,
}
class LibraryCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
with OnceConstantVisitorDefaultMixin<js_ast.Expression>
implements
@@ -256,6 +279,8 @@ class LibraryCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
final Options _options;
final SymbolData _symbolData;
HotReloadBranchState hotReloadCheckedBranch = HotReloadBranchState.none;
/// Maps each `Class` node compiled in the module to the `Identifier`s used to
/// name the class in JavaScript.
///
@@ -5207,7 +5232,7 @@ class LibraryCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
// the sub-expressions will have the correct mapping applied.
return jsExpression.toStatement()..sourceInformation = continueSourceMap;
}
return _visitExpression(expr).toStatement();
return jsExpression.toStatement();
}
@override
@@ -6176,10 +6201,7 @@ class LibraryCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
// Since there are no arguments (unlike methods) the dynamic get path can
// be reused for the hot reload checks on a getter.
var checkedGet = _emitCast(
_emitDynamicGet(
_visitExpression(receiver),
_emitMemberName(memberName),
),
_emitDynamicGet(jsReceiver, _emitMemberName(memberName)),
node.resultType,
)..sourceInformation = _nodeStart(node);
return _emitHotReloadSafeInvocation(instanceGet, checkedGet);
@@ -6527,11 +6549,13 @@ class LibraryCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
}
var receiver = node.receiver;
var jsReceiver = _visitExpression(receiver);
var jsArguments = _emitArgumentList(node.arguments, target: target);
if (node.isNativeListInvariantAddInvocation(_coreTypes.listClass)) {
// TODO(nshahan): If this code is retained, can it become invalid after a
// hot reload?
return js.call('#.push(#)', [jsReceiver, jsArguments]);
return js.call('#.push(#)', [
jsReceiver,
_emitArgumentList(node.arguments, target: target),
]);
}
var name = node.name.text;
if (name == 'call') {
@@ -6548,12 +6572,23 @@ class LibraryCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
// names of the Object instance members.
// TODO(nshahan): What should be checked after a hot reload. I think only
// the return type of the NSM can change.
return _runtimeCall('#(#, #)', [name, jsReceiver, jsArguments]);
return _runtimeCall('#(#, #)', [
name,
jsReceiver,
_emitArgumentList(node.arguments, target: target),
]);
}
// Otherwise generate this as a normal typed method call.
var jsName = _emitMemberName(name, member: target);
var invocation = js.call('#.#(#)', [jsReceiver, jsName, jsArguments]);
if (_shouldRewriteInvocationWithHotReloadChecks(target)) {
js_ast.Expression generateCall() {
var args = _emitArgumentList(node.arguments, target: target);
return js.call('#.#(#)', [jsReceiver, jsName, args]);
}
// Only consider checks if we're not in the unchecked branch.
if (_shouldRewriteInvocationWithHotReloadChecks(target) &&
hotReloadCheckedBranch != HotReloadBranchState.uncheckedBranch) {
var checkedInvocation = _rewriteInvocationWithHotReloadChecks(
jsReceiver,
jsName,
@@ -6562,10 +6597,25 @@ class LibraryCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
node.getStaticType(_staticTypeContext),
_nodeStart(node),
);
// If we're within the checked branch (i.e. not at the root) then return
// the checked call as-is.
if (hotReloadCheckedBranch == HotReloadBranchState.checkedBranch) {
return checkedInvocation;
}
// We're at the root of the branch so we need to generate the unchecked
// branch as well.
hotReloadCheckedBranch = HotReloadBranchState.uncheckedBranch;
final invocation = generateCall();
hotReloadCheckedBranch = HotReloadBranchState.none;
// As an optimization, avoid extra checks when the invocation code was
// compiled in the same generation that it is running.
return _emitHotReloadSafeInvocation(invocation, checkedInvocation);
}
final invocation = generateCall();
return _isNullCheckableJsInterop(node.interfaceTarget)
? _wrapWithJsInteropNullCheck(invocation)
: invocation;
@@ -7641,12 +7691,17 @@ class LibraryCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
}
}
var fn = _emitStaticTarget(target);
var args = _emitArgumentList(node.arguments, target: target);
var staticCall = js_ast.Call(fn, args)
..sourceInformation = _nodeStart(node);
if (_shouldRewriteInvocationWithHotReloadChecks(target)) {
var checkedCall = _rewriteInvocationWithHotReloadChecks(
js_ast.Call generateCall(js_ast.PropertyAccess fn) {
var args = _emitArgumentList(node.arguments, target: target);
return js_ast.Call(fn, args)..sourceInformation = _nodeStart(node);
}
// Only consider checks if we're not in the unchecked branch.
if (_shouldRewriteInvocationWithHotReloadChecks(target) &&
hotReloadCheckedBranch != HotReloadBranchState.uncheckedBranch) {
final fn = _emitStaticTarget(target);
var checkedInvocation = _rewriteInvocationWithHotReloadChecks(
fn.receiver,
fn.selector,
target,
@@ -7654,10 +7709,25 @@ class LibraryCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
node.getStaticType(_staticTypeContext),
_nodeStart(node),
);
// If we're within the checked branch (i.e. not at the root) then return
// the checked call as-is.
if (hotReloadCheckedBranch == HotReloadBranchState.checkedBranch) {
return checkedInvocation;
}
// We're at the root of the branch so we need to generate the unchecked
// branch as well.
hotReloadCheckedBranch = HotReloadBranchState.uncheckedBranch;
final invocation = generateCall(fn);
hotReloadCheckedBranch = HotReloadBranchState.none;
// As an optimization, avoid extra checks when the invocation code was
// compiled in the same generation that it is running.
return _emitHotReloadSafeInvocation(staticCall, checkedCall);
return _emitHotReloadSafeInvocation(invocation, checkedInvocation);
}
final staticCall = generateCall(_emitStaticTarget(target));
return _isNullCheckableJsInterop(target)
? _wrapWithJsInteropNullCheck(staticCall)
: staticCall;
@@ -7713,6 +7783,8 @@ class LibraryCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
DartType expectedReturnType,
SourceLocation? originalCallSiteSourceLocation,
) {
final savedCheckedBranch = hotReloadCheckedBranch;
hotReloadCheckedBranch = HotReloadBranchState.checkedBranch;
var hoistedPositionalVariables = <js_ast.Expression>[];
var hoistedNamedVariables = <String, js_ast.Expression>{};
js_ast.Expression? letAssignments;
@@ -7799,7 +7871,7 @@ class LibraryCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
])..sourceInformation = originalCallSiteSourceLocation;
// Cast the result of the checked call or the value returned from a
// `NoSuchMethod` invocation.
return js_ast.Binary(
final result = js_ast.Binary(
',',
letAssignments,
js.call('# == # ? # : #', [
@@ -7809,6 +7881,9 @@ class LibraryCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
_emitCast(checkResult, expectedReturnType),
]),
);
hotReloadCheckedBranch = savedCheckedBranch;
return result;
}
js_ast.Expression _emitJSObjectGetPrototypeOf(
tests/web/nested_closure_invocations_test.dart
+225
View File
@@ -0,0 +1,225 @@
// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
abstract class Either<L, R> {
Either();
B fold<B>(B ifLeft(L l), B ifRight(R r));
static Either<L, U> map20<
L,
A,
A2 extends A,
B,
B2 extends B,
C,
C2 extends C,
D,
D2 extends D,
E,
E2 extends E,
F,
F2 extends F,
G,
G2 extends G,
H,
H2 extends H,
I,
I2 extends I,
J,
J2 extends J,
K,
K2 extends K,
LL,
LL2 extends LL,
M,
M2 extends M,
N,
N2 extends N,
O,
O2 extends O,
P,
P2 extends P,
Q,
Q2 extends Q,
R,
R2 extends R,
S,
S2 extends S,
T,
T2 extends T,
U
>(
Either<L, A2> fa,
Either<L, B2> fb,
Either<L, C2> fc,
Either<L, D2> fd,
Either<L, E2> fe,
Either<L, F2> ff,
Either<L, G2> fg,
Either<L, H2> fh,
Either<L, I2> fi,
Either<L, J2> fj,
Either<L, K2> fk,
Either<L, LL> fl,
Either<L, M> fm,
Either<L, N> fn,
Either<L, O> fo,
Either<L, P> fp,
Either<L, Q> fq,
Either<L, R> fr,
Either<L, S> fs,
Either<L, T> ft,
U fun(
A a,
B b,
C c,
D d,
E e,
F f,
G g,
H h,
I i,
J j,
K k,
LL l,
M m,
N n,
O o,
P p,
Q q,
R r,
S s,
T t,
),
) => fa.fold(
left,
(a) => fb.fold(
left,
(b) => fc.fold(
left,
(c) => fd.fold(
left,
(d) => fe.fold(
left,
(e) => ff.fold(
left,
(f) => fg.fold(
left,
(g) => fh.fold(
left,
(h) => fi.fold(
left,
(i) => fj.fold(
left,
(j) => fk.fold(
left,
(k) => fl.fold(
left,
(l) => fm.fold(
left,
(m) => fn.fold(
left,
(n) => fo.fold(
left,
(o) => fp.fold(
left,
(p) => fq.fold(
left,
(q) => fr.fold(
left,
(r) => fs.fold(
left,
(s) => ft.fold(
left,
(t) => right(
fun(
a,
b,
c,
d,
e,
f,
g,
h,
i,
j,
k,
l,
m,
n,
o,
p,
q,
r,
s,
t,
),
),
),
),
),
),
),
),
),
),
),
),
),
),
),
),
),
),
),
),
),
);
}
Either<L, R> left<L, R>(L l) => new Left(l);
Either<L, R> right<L, R>(R r) => new Right(r);
class Left<L, R> extends Either<L, R> {
final L _l;
B fold<B>(B ifLeft(L l), B ifRight(R r)) => ifLeft(_l);
Left(this._l);
}
class Right<L, R> extends Either<L, R> {
final R _r;
B fold<B>(B ifLeft(L l), B ifRight(R r)) => ifRight(_r);
Right(this._r);
}
void main() {
Either.map20(
left(0),
left(1),
left(2),
left(3),
left(4),
left(5),
left(6),
left(7),
left(8),
left(9),
left(10),
left(11),
left(12),
left(13),
left(14),
left(15),
left(16),
left(17),
left(18),
left(19),
(a, b, c, d, e, f, g, h, i, j, k, l, m, n, o, p, q, r, s, t) => 20,
);
}