[vm/compiler] Minor heuristic change to reduce code size

aartbik · commit-bot@chromium.org · commit b0968395f13e · 2019-08-12T23:17:43.000Z
Rationale: Inlining constant constructors/mixin is practically always profitable. So the heuristics have been changed to at least consider these even at higher depth (note that they are of course still subject to actual heuristics while inlining). This brings back flutter gallery size to where it was before mixins were introduced. head: VMIsolate(CodeSize): 4737 Isolate(CodeSize): 2005123 101% ReadOnlyData(CodeSize): 2212408 103% Instructions(CodeSize): 7006928 100% Total(CodeSize): 11229196 101% improved heuristic: VMIsolate(CodeSize): 4737 Isolate(CodeSize): 1985691 ReadOnlyData(CodeSize): 2152880 Instructions(CodeSize): 6987616 Total(CodeSize): 11130924 #37126 Change-Id: I28de0fa6c92a785bbc47e9fa09ed55ae68593c0a Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/112758 Commit-Queue: Aart Bik <ajcbik@google.com> Reviewed-by: Alexander Markov <alexmarkov@google.com>
diff --git a/runtime/vm/compiler/backend/inliner.cc b/runtime/vm/compiler/backend/inliner.cc
@@ -433,10 +433,10 @@ class CallSites : public ValueObject {
       return;
     }
 
-    // Recognized methods are not treated as normal calls. They don't have
-    // calls in themselves, so we keep adding those even when at the threshold.
-    const bool inline_only_recognized_methods =
-        (depth == inlining_depth_threshold_);
+    // At the maximum inlining depth, only profitable methods
+    // are further considered for inlining.
+    const bool inline_only_profitable_methods =
+        (depth >= inlining_depth_threshold_);
 
     // In AOT, compute loop hierarchy.
     if (FLAG_precompiled_mode) {
@@ -454,13 +454,15 @@ class CallSites : public ValueObject {
         if (current->IsPolymorphicInstanceCall()) {
           PolymorphicInstanceCallInstr* instance_call =
               current->AsPolymorphicInstanceCall();
-          if (!inline_only_recognized_methods ||
+          if (!inline_only_profitable_methods ||
               instance_call->IsSureToCallSingleRecognizedTarget() ||
               instance_call->HasOnlyDispatcherOrImplicitAccessorTargets()) {
+            // Consider instance call for further inlining. Note that it will
+            // still be subject to all the inlining heuristics.
             instance_calls_.Add(InstanceCallInfo(instance_call, graph, depth));
           } else {
-            // Method not inlined because inlining too deep and method
-            // not recognized.
+            // No longer consider the instance call because inlining is too
+            // deep and the method is not deemed profitable by other criteria.
             if (FLAG_print_inlining_tree) {
               const Function* caller = &graph->function();
               const Function* target = &instance_call->targets().FirstTarget();
@@ -470,13 +472,16 @@ class CallSites : public ValueObject {
           }
         } else if (current->IsStaticCall()) {
           StaticCallInstr* static_call = current->AsStaticCall();
-          if (!inline_only_recognized_methods ||
-              static_call->function().IsRecognized() ||
-              static_call->function().IsDispatcherOrImplicitAccessor()) {
+          const Function& function = static_call->function();
+          if (!inline_only_profitable_methods || function.IsRecognized() ||
+              function.IsDispatcherOrImplicitAccessor() ||
+              (function.is_const() && function.IsGenerativeConstructor())) {
+            // Consider static call for further inlining. Note that it will
+            // still be subject to all the inlining heuristics.
             static_calls_.Add(StaticCallInfo(static_call, graph, depth));
           } else {
-            // Method not inlined because inlining too deep and method
-            // not recognized.
+            // No longer consider the static call because inlining is too
+            // deep and the method is not deemed profitable by other criteria.
             if (FLAG_print_inlining_tree) {
               const Function* caller = &graph->function();
               const Function* target = &static_call->function();
@@ -485,9 +490,13 @@ class CallSites : public ValueObject {
             }
           }
         } else if (current->IsClosureCall()) {
-          if (!inline_only_recognized_methods) {
+          if (!inline_only_profitable_methods) {
+            // Consider closure for further inlining. Note that it will
+            // still be subject to all the inlining heuristics.
             ClosureCallInstr* closure_call = current->AsClosureCall();
             closure_calls_.Add(ClosureCallInfo(closure_call, graph));
+          } else {
+            // No longer consider the closure because inlining is too deep.
           }
         }
       }
diff --git a/tests/language_2/vm/mixin_test.dart b/tests/language_2/vm/mixin_test.dart
@@ -0,0 +1,37 @@
+// Copyright (c) 2019, 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.
+
+// Illustrates inlining heuristic issue of
+// https://github.com/dart-lang/sdk/issues/37126
+// (mixins introduce one extra depth of inlining).
+
+// VMOptions=--deterministic
+
+import "package:expect/expect.dart";
+
+class X {
+  const X();
+  int foo() {
+    return 1;
+  }
+}
+
+mixin YMixin {
+  int bar() {
+    return 2;
+  }
+}
+
+class Y with YMixin {
+  const Y();
+}
+
+@pragma("vm:never-inline")
+int foobar() {
+  return new X().foo() + new Y().bar();
+}
+
+main() {
+  Expect.equals(3, foobar());
+}
