Support for inlining small methods (independent of they're called inside a loop or not)

sdk/lib/_internal/compiler/implementation/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.
 
 part of ssa;
 
 /**
  * A special element for the extra parameter taken by intercepted
  * methods. We need to override [Element.computeType] because our
  * optimizers may look at its declared type.
@@ skipping 911 matching lines @@
       activationVariables = new Map<Element, HLocalValue>(),
       jumpTargets = new Map<TargetElement, JumpHandler>(),
       parameters = new Map<Element, HInstruction>(),
       sourceElementStack = <Element>[work.element],
       inliningStack = <InliningState>[],
       rti = builder.backend.rti,
       super(work.resolutionTree) {
     localsHandler = new LocalsHandler(this);
   }
 
-  static const MAX_INLINING_DEPTH = 3;
-  static const MAX_INLINING_NODES = 46;
-
   List<InliningState> inliningStack;
 
   Element returnElement;
   DartType returnType;
 
   bool inTryStatement = false;
   int loopNesting = 0;
 
   HBasicBlock get current => _current;
   void set current(c) {
@@ skipping 273 matching lines @@
   bool tryInlineMethod(Element element,
                        Selector selector,
                        List<HInstruction> providedArguments,
                        Node currentNode) {
     backend.registerStaticUse(element, compiler.enqueuer.codegen);
 
     // Ensure that [element] is an implementation element.
     element = element.implementation;
     FunctionElement function = element;
 
-    bool cachedCanBeInlined = backend.canBeInlined[function];
+    bool insideLoop = loopNesting > 0 || graph.calledInLoop;
+
+    // Bail out early if the inlining decision is in the cache and we can't
+    // inline (no need to check the hard constraints).
+    bool cachedCanBeInlined =
+        backend.inlineCache.canInline(function, insideLoop: insideLoop);
     if (cachedCanBeInlined == false) return false;
 
     bool meetsHardConstraints() {
       // We cannot inline a method from a deferred library into a method
       // which isn't deferred.
       // TODO(ahe): But we should still inline into the same
       // connected-component of the deferred library.
       if (compiler.deferredLoadTask.isDeferred(element)) return false;
       if (compiler.disableInlining) return false;
 
@@ skipping 13 matching lines @@
 
       // Don't inline if the return type was inferred to be non-null empty. This
       // means that the function always throws an exception.
       TypeMask returnType =
           compiler.typesTask.getGuaranteedReturnTypeOfElement(element);
       if (returnType != null && returnType.isEmpty && !returnType.isNullable) {
         isReachable = false;
         return false;
       }
 
+      // Don't inline recursivly

[ngeoffray, 2013/07/16 17:17:37]

As discussed, removing the inlining depth is a heuristic, and that is why I
would remove this hard limitation into a heuristic too.

[kustermann, 2013/07/17 09:46:42]

Done.

+      if (inliningStack.any((entry) => entry.function == function)) {
+        return false;
+      }
+
       return true;
     }
 
-    bool heuristicsSayGoodToGo(FunctionExpression functionExpression,
-                                TreeElements newElements) {
-      if (loopNesting == 0 && !graph.calledInLoop) return false;
+    bool heuristicSayGoodToGo(FunctionExpression functionExpression) {
+      if (cachedCanBeInlined == true) return cachedCanBeInlined;
 
-      int maxDepth = (loopNesting > 0) ? MAX_INLINING_DEPTH : 1;
-      if (inliningStack.length >= maxDepth) return false;
-
-      if (cachedCanBeInlined == null) {
-        var canBeInlined =
-            InlineWeeder.canBeInlined(functionExpression, newElements);
-        backend.canBeInlined[function] = canBeInlined;
-        return canBeInlined;
+      int numParameters = function.functionSignature.parameterCount;
+      int maxInliningNodes;
+      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;
       }
-      return cachedCanBeInlined;
+      bool canBeInlined = InlineWeeder.canBeInlined(
+          functionExpression, maxInliningNodes);
+      if (canBeInlined) {
+        backend.inlineCache.markAsInlinable(element, insideLoop: insideLoop);
+      } else {
+        backend.inlineCache.markAsNonInlinable(element, insideLoop: insideLoop);
+      }
+      return canBeInlined;
     }
 
     void doInlining(FunctionExpression functionExpression) {
       // Add an explicit null check on the receiver before doing the
       // inlining. We use [element] to get the same name in the
       // NoSuchMethodError message as if we had called it.
       if (element.isInstanceMember()
           && !element.isGenerativeConstructorBody()
           && (selector.mask == null || selector.mask.isNullable)) {
         addWithPosition(
@@ skipping 11 matching lines @@
         });
         element.isGenerativeConstructor()
             ? buildFactory(element)
             : functionExpression.body.accept(this);
       });
       leaveInlinedMethod(state);
     }
 
     if (meetsHardConstraints()) {
       FunctionExpression functionExpression = function.parseNode(compiler);
-      TreeElements newElements =
-          compiler.enqueuer.resolution.getCachedElements(function);
-      if (newElements == null) {
-        compiler.internalError("Element not resolved: $function");
-      }
 
-      if (heuristicsSayGoodToGo(functionExpression, newElements)) {
+      if (heuristicSayGoodToGo(functionExpression)) {
         doInlining(functionExpression);
         return true;
       }
     }
 
     return false;
   }
 
   inlinedFrom(Element element, f()) {
     assert(element is FunctionElement || element is VariableElement);
@@ skipping 3791 matching lines @@
     builder.add(instruction);
     return instruction;
   }
 }
 
 /**
  * This class visits the method that is a candidate for inlining and
  * finds whether it is too difficult to inline.
  */
 class InlineWeeder extends Visitor {
-  final TreeElements elements;
+  // 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;
 
-  InlineWeeder(this.elements);
+  InlineWeeder(this.maxInliningNodes);
 
   static bool canBeInlined(FunctionExpression functionExpression,
-                           TreeElements elements) {
-    InlineWeeder weeder = new InlineWeeder(elements);
+                           int maxInliningNodes) {
+    InlineWeeder weeder = new InlineWeeder(maxInliningNodes);
     weeder.visit(functionExpression.initializers);
     weeder.visit(functionExpression.body);
-    if (weeder.tooDifficult) return false;
-    return true;
+    return !weeder.tooDifficult;
   }
 
   bool registerNode() {
-    if (nodeCount++ > SsaBuilder.MAX_INLINING_NODES) {
+    if (nodeCount++ > maxInliningNodes) {
       tooDifficult = true;
       return false;
     } else {
       return true;
     }
   }
 
   void visit(Node node) {
     if (node != null) node.accept(this);
   }
@@ skipping 310 matching lines @@
           new HSubGraphBlockInformation(elseBranch.graph));
 
     HBasicBlock conditionStartBlock = conditionBranch.block;
     conditionStartBlock.setBlockFlow(info, joinBlock);
     SubGraph conditionGraph = conditionBranch.graph;
     HIf branch = conditionGraph.end.last;
     assert(branch is HIf);
     branch.blockInformation = conditionStartBlock.blockFlow;
   }
 }