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

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

Index: pkg/compiler/lib/src/ssa/builder.dart
diff --git a/pkg/compiler/lib/src/ssa/builder.dart b/pkg/compiler/lib/src/ssa/builder.dart
index e4b5c8bfb39c7b17683b2ac749881d580c3209d6..99122b112a9e225a530949af20408fa5111e4e97 100644
--- a/pkg/compiler/lib/src/ssa/builder.dart
+++ b/pkg/compiler/lib/src/ssa/builder.dart
@@ -1293,12 +1293,6 @@ class SsaBuilder extends NewResolvedVisitor {
     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.
-      if (!compiler.deferredLoadTask
-          .inSameOutputUnit(element,compiler.currentElement)) {
-        return false;
-      }
       if (compiler.disableInlining) return false;
 
       assert(selector != null
@@ -1348,12 +1342,18 @@ class SsaBuilder extends NewResolvedVisitor {
         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 doesNotContainCode() {

[floitsch, 2015/05/05 17:58:15]

I would probably inline this method, but ok as is.

[herhut, 2015/05/06 08:37:09]

Acknowledged.

+      // A function with size 1 does not contain any code.
+      return InlineWeeder.canBeInlined(function, 1, true);
     }
 
     bool heuristicSayGoodToGo() {
@@ -1364,6 +1364,16 @@ class SsaBuilder extends NewResolvedVisitor {
 
       if (element.isSynthesized) return true;
 
+      // Don't inline across deferred import to prevent leaking code. The only
+      // exception is an empty function (which does not contain code).
+      bool hasNonDeferredImportPath = !compiler.deferredLoadTask

[floitsch, 2015/05/05 17:58:15]

The variable names are wrong:
the boolean should be: hasDeferredImportPath, and the function on
deferredLoadTask should be 'hasOnlyNonDeferredImportPaths'.

If you want to keep the local, negate it only in the conditional of the if (and
rename it accordingly). Currently the "!" disappears slightly.

[herhut, 2015/05/06 08:37:09]

Done.

+          .hasNonDeferredImportPath(compiler.currentElement, element);
+
+      if (hasNonDeferredImportPath) {
+        return doesNotContainCode();
+      }
+
+      // Do not inline code that is rarely executed unless it reduces size.
       if (inExpressionOfThrow || inLazyInitializerExpression) {
         return reductiveHeuristic();
       }