dart2js: Inline across deferred output units if that leads to reduced size.

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.
 
 part of ssa;
 
 class SsaFunctionCompiler implements FunctionCompiler {
   SsaCodeGeneratorTask generator;
   SsaBuilderTask builder;
   SsaOptimizerTask optimizer;
@@ skipping 1275 matching lines @@
     FunctionElement function = element;
     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() {
-      // Don't inline from one output unit to another. If something is deferred
-      // it is to save space in the loading code.

[floitsch, 2015/05/04 23:32:58]

this comment is incomplete: we must not inline code from deferred libraries,
because they might contain confidential code (like experiments).
However, we are allowed to inline over output-unit boundaries (as long as they
don't cross a deferred library-prefix).

I would reduce this test to: "!inSameOutputUnit &&
element_is_accessed_through_a_deferred_prefix".
I'm not sure this is something we can know here, though.

[herhut, 2015/05/05 14:19:12]

I changed it to not inline across deferred imports independent of output unit,
which I think is what you were asking for.

-      if (!compiler.deferredLoadTask
-          .inSameOutputUnit(element,compiler.currentElement)) {
-        return false;
-      }
       if (compiler.disableInlining) return false;
 
       assert(selector != null
              || Elements.isStaticOrTopLevel(element)
              || element.isGenerativeConstructorBody);
       if (selector != null && !selector.applies(function, compiler.world)) {
         return false;
       }
 
       // Don't inline operator== methods if the parameter can be null.
@@ skipping 25 matching lines @@
           return false;
         }
       }
 
       return true;
     }
 
     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(element)) {

[floitsch, 2015/05/04 23:34:44]

I guess we could inline empty methods from deferred libraries without too much
risk of leaking information, but the "isCalledOnce" inlining would lead to
trouble.

[herhut, 2015/05/05 14:19:12]

Acknowledged.

         return InlineWeeder.canBeInlined(function, -1, false);
       }
       // TODO(sra): Measure if inlining would 'reduce' the size.  One desirable
-      // case we miss my doing nothing is inlining very simple constructors
+      // 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.
-      return false;
+      // We at least inline bodies that are empty (and thus have a size of 1).
+      return InlineWeeder.canBeInlined(function, 1, true);
     }
 
     bool heuristicSayGoodToGo() {
       // Don't inline recursively
       if (inliningStack.any((entry) => entry.function == function)) {
         return false;
       }
 
       if (element.isSynthesized) return true;
 
-      if (inExpressionOfThrow || inLazyInitializerExpression) {
+      // Don't inline from one output unit to another, unless it reduces size.
+      // Similarly do not inline code that is rarely executed.
+      bool fromDifferentOutputUnit = !compiler.deferredLoadTask
+          .inSameOutputUnit(element,compiler.currentElement);
+
+      if (inExpressionOfThrow ||
+          inLazyInitializerExpression ||
+          fromDifferentOutputUnit) {
         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(function, -1, false,
                                          allowLoops: true));
         return true;
       }
@@ skipping 6209 matching lines @@
     if (unaliased is TypedefType) throw 'unable to unalias $type';
     unaliased.accept(this, builder);
   }
 
   void visitDynamicType(DynamicType type, SsaBuilder builder) {
     JavaScriptBackend backend = builder.compiler.backend;
     ClassElement cls = backend.findHelper('DynamicRuntimeType');
     builder.push(new HDynamicType(type, new TypeMask.exact(cls, classWorld)));
   }
 }