Fix issue 21159: prevent endless inlining of field dispatchers.

runtime/vm/flow_graph_inliner.cc

 // Copyright (c) 2013, 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.
 
 #include "vm/flow_graph_inliner.h"
 
 #include "vm/block_scheduler.h"
 #include "vm/compiler.h"
 #include "vm/flags.h"
 #include "vm/flow_graph.h"
@@ skipping 29 matching lines @@
     "Stop inlining once caller reaches the threshold.");
 DEFINE_FLAG(int, inlining_constant_arguments_count, 1,
     "Inline function calls with sufficient constant arguments "
     "and up to the increased threshold on instructions");
 DEFINE_FLAG(int, inlining_constant_arguments_size_threshold, 60,
     "Inline function calls with sufficient constant arguments "
     "and up to the increased threshold on instructions");
 DEFINE_FLAG(int, inlining_hotness, 10,
     "Inline only hotter calls, in percents (0 .. 100); "
     "default 10%: calls above-equal 10% of max-count are inlined.");
-DEFINE_FLAG(bool, inline_recursive, true,
-    "Inline recursive calls.");
+DEFINE_FLAG(bool, inline_recursive, false, "Inline recursive calls.");
 DEFINE_FLAG(int, max_inlined_per_depth, 500,
     "Max. number of inlined calls per depth");
 DEFINE_FLAG(bool, print_inlining_tree, false, "Print inlining tree");
 
 DECLARE_FLAG(bool, compiler_stats);
 DECLARE_FLAG(bool, enable_type_checks);
 DECLARE_FLAG(int, deoptimization_counter_threshold);
 DECLARE_FLAG(bool, print_flow_graph);
 DECLARE_FLAG(bool, print_flow_graph_optimized);
 DECLARE_FLAG(bool, verify_compiler);
@@ skipping 546 matching lines @@
                                function.optimized_call_site_count(),
                                constant_arguments));
       PRINT_INLINING_TREE("Early heuristic",
           &call_data->caller, &function, call_data->call);
       return false;
     }
 
     // Abort if this is a recursive occurrence.
     Definition* call = call_data->call;
     if (!FLAG_inline_recursive && IsCallRecursive(unoptimized_code, call)) {
       function.set_is_inlinable(false);

[Florian Schneider, 2014/10/20 14:01:57]

Maybe that should not mark the whole function as not inlinable in this case:

I found that for example the Integer.compareTo could still be inlined into its
caller, but the recursive call-site inside compareTo (which may not be reachable
after constant propagation) should not be inlined. 

       TRACE_INLINING(OS::Print("     Bailout: recursive function\n"));
       PRINT_INLINING_TREE("Recursive function",
           &call_data->caller, &function, call_data->call);
       return false;
     }
 
     // Save and clear deopt id.
     const intptr_t prev_deopt_id = isolate()->deopt_id();
     isolate()->set_deopt_id(0);
     // Install bailout jump.
@@ skipping 128 matching lines @@
                                  "code size:  %" Pd ", "
                                  "call sites: %" Pd ", "
                                  "const args: %" Pd "\n",
                                  size,
                                  call_site_count,
                                  constants_count));
         PRINT_INLINING_TREE("Heuristic fail",
             &call_data->caller, &function, call_data->call);
         return false;
       }
-
       if (function.IsInvokeFieldDispatcher() ||
           function.IsNoSuchMethodDispatcher()) {
         // Append call sites to the currently processed list so that dispatcher
         // methods get inlined regardless of the current depth.
+        // Need a throttling mechanism for recursive inlining.
+        ASSERT(!FLAG_inline_recursive);
         inlining_call_sites_->FindCallSites(callee_graph,
                                             0,

[zerny-google, 2014/10/20 11:28:21]

DBC: It seems this issue is caused by ignoring the inlining depth. We could
separate the general depth threshold from the recursive depth threshold (I
recall talking about this, but not implementing it, since, at the time, the
general depth achieved mostly the same thing.)

[Florian Schneider, 2014/10/20 14:01:56]

I agree that a separate threshold for recursive inlining would be the best if we
do want to inline recursive call-sites. Dispatchers should always be inlined,
since they are so small. 

                                             &inlined_info_);
       } else {
         collected_call_sites_->FindCallSites(callee_graph,
                                              inlining_depth_,
                                              &inlined_info_);
       }
 
       // Add the function to the cache.
       if (!in_cache) {
         function_cache_.Add(parsed_function);
@@ skipping 919 matching lines @@
         OS::Print("After Inlining of %s\n", flow_graph_->
                   parsed_function().function().ToFullyQualifiedCString());
         FlowGraphPrinter printer(*flow_graph_);
         printer.PrintBlocks();
       }
     }
   }
 }
 
 }  // namespace dart