Tweak Florian's dart2js inlining heuristic change

pkg/compiler/lib/src/ssa/builder.dart

 // Copyright (c) 2012, 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.
 
 import 'dart:collection';
 
 import 'package:js_runtime/shared/embedded_names.dart';
 
 import '../closure.dart';
 import '../common.dart';
@@ skipping 455 matching lines @@
           isReachable = false;
           return false;
         }
       }
 
       return true;
     }
 
     bool doesNotContainCode() {
       // A function with size 1 does not contain any code.
-      return InlineWeeder.canBeInlined(functionResolvedAst, 1, true,
+      return InlineWeeder.canBeInlined(functionResolvedAst, 1,
           enableUserAssertions: compiler.options.enableUserAssertions);
     }
 
     bool reductiveHeuristic() {
       // The call is on a path which is executed rarely, so inline only if it
       // does not make the program larger.
       if (isCalledOnce(function)) {
-        return InlineWeeder.canBeInlined(functionResolvedAst, -1, false,
+        return InlineWeeder.canBeInlined(functionResolvedAst, null,
             enableUserAssertions: compiler.options.enableUserAssertions);
       }
       // TODO(sra): Measure if inlining would 'reduce' the size.  One desirable
       // case we miss by doing nothing is inlining very simple constructors
       // where all fields are initialized with values from the arguments at this
       // call site.  The code is slightly larger (`new Foo(1)` vs `Foo$(1)`) but
       // that usually means the factory constructor is left unused and not
       // emitted.
       // We at least inline bodies that are empty (and thus have a size of 1).
       return doesNotContainCode();
@@ skipping 17 matching lines @@
       }
 
       // Do not inline code that is rarely executed unless it reduces size.
       if (inExpressionOfThrow || inLazyInitializerExpression) {
         return reductiveHeuristic();
       }
 
       if (cachedCanBeInlined == true) {
         // We may have forced the inlining of some methods. Therefore check
         // if we can inline this method regardless of size.
-        assert(InlineWeeder.canBeInlined(functionResolvedAst, -1, false,
+        assert(InlineWeeder.canBeInlined(functionResolvedAst, null,
             allowLoops: true,
             enableUserAssertions: compiler.options.enableUserAssertions));
         return true;
       }
 
       int numParameters = function.functionSignature.parameterCount;
       int maxInliningNodes;
-      bool useMaxInliningNodes = true;
       if (insideLoop) {
         maxInliningNodes = InlineWeeder.INLINING_NODES_INSIDE_LOOP +
             InlineWeeder.INLINING_NODES_INSIDE_LOOP_ARG_FACTOR * numParameters;
       } else {
         maxInliningNodes = InlineWeeder.INLINING_NODES_OUTSIDE_LOOP +
             InlineWeeder.INLINING_NODES_OUTSIDE_LOOP_ARG_FACTOR * numParameters;
       }
 
       // If a method is called only once, and all the methods in the
       // inlining stack are called only once as well, we know we will
       // save on output size by inlining this method.
       if (isCalledOnce(function)) {
-        useMaxInliningNodes = false;
+        maxInliningNodes = null;
       }
-      bool canInline;
-      canInline = InlineWeeder.canBeInlined(
-          functionResolvedAst, maxInliningNodes, useMaxInliningNodes,
+      bool canInline = InlineWeeder.canBeInlined(
+          functionResolvedAst, maxInliningNodes,
           enableUserAssertions: compiler.options.enableUserAssertions);
       if (canInline) {
         backend.inlineCache.markAsInlinable(function, insideLoop: insideLoop);
       } else {
         backend.inlineCache
             .markAsNonInlinable(function, insideLoop: insideLoop);
       }
       return canInline;
     }
 
@@ skipping 6022 matching lines @@
 class InlineWeeder extends ast.Visitor {
   // Invariant: *INSIDE_LOOP* > *OUTSIDE_LOOP*
   static const INLINING_NODES_OUTSIDE_LOOP = 18;
   static const INLINING_NODES_OUTSIDE_LOOP_ARG_FACTOR = 3;
   static const INLINING_NODES_INSIDE_LOOP = 42;
   static const INLINING_NODES_INSIDE_LOOP_ARG_FACTOR = 4;
 
   bool seenReturn = false;
   bool tooDifficult = false;
   int nodeCount = 0;
-  final int maxInliningNodes;
-  final bool useMaxInliningNodes;
+  final int maxInliningNodes; // `null` for unbounded.
   final bool allowLoops;
   final bool enableUserAssertions;
+  final TreeElements elements;
 
-  InlineWeeder(this.maxInliningNodes, this.useMaxInliningNodes, this.allowLoops,
+  InlineWeeder._(this.elements, this.maxInliningNodes, this.allowLoops,
       this.enableUserAssertions);
 
-  static bool canBeInlined(
-      ResolvedAst resolvedAst, int maxInliningNodes, bool useMaxInliningNodes,
+  static bool canBeInlined(ResolvedAst resolvedAst, int maxInliningNodes,
       {bool allowLoops: false, bool enableUserAssertions: null}) {
     assert(enableUserAssertions is bool); // Ensure we passed it.
     if (resolvedAst.elements.containsTryStatement) return false;
 
-    InlineWeeder weeder = new InlineWeeder(maxInliningNodes,
-        useMaxInliningNodes, allowLoops, enableUserAssertions);
+    InlineWeeder weeder = new InlineWeeder._(resolvedAst.elements,
+        maxInliningNodes, allowLoops, enableUserAssertions);
     ast.FunctionExpression functionExpression = resolvedAst.node;
+
     weeder.visit(functionExpression.initializers);
     weeder.visit(functionExpression.body);
     weeder.visit(functionExpression.asyncModifier);
     return !weeder.tooDifficult;
   }
 
   bool registerNode() {
-    if (!useMaxInliningNodes) return true;
+    if (maxInliningNodes == null) return true;
     if (nodeCount++ > maxInliningNodes) {
       tooDifficult = true;
       return false;
     } else {
       return true;
     }
   }
 
   void visit(ast.Node node) {
     if (node != null) node.accept(this);
@@ skipping 26 matching lines @@
     if (!registerNode()) return;
     tooDifficult = true;
   }
 
   void visitFunctionDeclaration(ast.Node node) {
     if (!registerNode()) return;
     tooDifficult = true;
   }
 
   void visitSend(ast.Send node) {
+    // TODO(sra): Investigate following, and possibly count occurrences, since
+    // repeated references might cause a temporary to be assigned.
+    //
+    //     Element element = elements[node];
+    //     if (element != null && element.isParameter) {
+    //       // Don't count as additional node, since it's likely that passing
+    //       // the argument would cost us as much space as we inline.
+    //       return;
+    //     }
     if (!registerNode()) return;
     node.visitChildren(this);
   }
 
   visitLoop(ast.Node node) {
     // It's actually not difficult to inline a method with a loop, but
     // our measurements show that it's currently better to not inline a
     // method that contains a loop.
     if (!allowLoops) tooDifficult = true;
   }
 
   void visitRedirectingFactoryBody(ast.RedirectingFactoryBody node) {
     if (!registerNode()) return;
     tooDifficult = true;
   }
 
+  void visitConditional(ast.Conditional node) {
+    // Heuristic: In "parameter ? A : B" there is a high probability that
+    // parameter is a constant. Assuming the parameter is constant, we can

[floitsch, 2017/03/01 08:50:15]

In theory we could even look at the parameter. However, then we would need to
change our caching strategy for "isInlinable".

+    // compute a count that is bounded by the largest arm rather than the sum of
+    // both arms.
+    visit(node.condition);
+    if (tooDifficult) return;
+    int commonPrefixCount = nodeCount;
+
+    visit(node.thenExpression);
+    if (tooDifficult) return;
+    int thenCount = nodeCount - commonPrefixCount;
+
+    nodeCount = commonPrefixCount;
+    visit(node.elseExpression);
+    if (tooDifficult) return;
+    int elseCount = nodeCount - commonPrefixCount;
+
+    nodeCount = commonPrefixCount + thenCount + elseCount;
+    if (node.condition.asSend() != null &&
+        elements[node.condition]?.isParameter == true) {
+      nodeCount =
+          commonPrefixCount + (thenCount > elseCount ? thenCount : elseCount);
+    }
+    // This is last so that [tooDifficult] is always updated.
+    if (!registerNode()) return;
+  }
+
   void visitRethrow(ast.Rethrow node) {
     if (!registerNode()) return;
     tooDifficult = true;
   }
 
   void visitReturn(ast.Return node) {
     if (!registerNode()) return;
     if (seenReturn || identical(node.beginToken.stringValue, 'native')) {
       tooDifficult = true;
       return;
@@ skipping 40 matching lines @@
       this.oldReturnLocal,
       this.oldReturnType,
       this.oldResolvedAst,
       this.oldStack,
       this.oldLocalsHandler,
       this.inTryStatement,
       this.allFunctionsCalledOnce,
       this.oldElementInferenceResults)
       : super(function);
 }