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

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: flutter/flutter#173700
Change-Id: I3167a96e3ead67fd1d5c763ef2a7309d82c1a7c6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/445540
Commit-Queue: Nate Biggs <[email protected]>
Reviewed-by: Nicholas Shahan <[email protected]>

Author: natebiggs

pkg/dev_compiler/lib/src/kernel/compiler_new.dart

@@ -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,23 +7691,43 @@ 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,
         node.arguments,
         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(