Inliner: use time budget to avoid excessive inlining

Use the time budget and time estimates to stop inlining once the
overall jit time increase estimate for the method is 10x the initial
jit time estimate.

This is implemented as part of `LegacyPolicy` and so can impact
the current inline behavior.

The budget is intentionally set quite high so that it only kicks in
for very rare cases where the call tree below the root is deep and wide
with many small methods. In extended testing on desktop this limit
fires in exactly two cases, both HFA tests.

In CoreCLR tests 12 of the HFA tests hit this limit. I've added
a directed test case here that came from the original bug report.

Closes dotnet/coreclr#2472.

Commit migrated from https://github.com/dotnet/coreclr/commit/597f78e55cc77dc75f25c5aa2471ea0e82e988cd

diff --git a/src/coreclr/src/jit/inline.cpp b/src/coreclr/src/jit/inline.cpp
index 188d21c..4477fd2 100644 (file)
--- a/src/coreclr/src/jit/inline.cpp
+++ b/src/coreclr/src/jit/inline.cpp
@@ -663,6 +663,9 @@ InlineStrategy::InlineStrategy(Compiler* compiler)
 //------------------------------------------------------------------------
 // GetRootContext: get the InlineContext for the root method
 //
+// Return Value:
+//    Root context; describes the method being jitted.
+//
 // Note:
 //    Also initializes the jit time estimate and budget.
 
@@ -698,26 +701,68 @@ void InlineStrategy::NoteCandidate()
 
 //------------------------------------------------------------------------
 // EstimateTime: estimate impact of this inline on the method jit time
+//
+// Arguments:
+//     context - context describing this inline
+//
+// Return Value:
+//    Nominal estimate of  jit time.
 
 int InlineStrategy::EstimateTime(InlineContext* context)
 {
-    unsigned ILSize = context->GetCodeSize();
-
     // Simple linear models based on observations
+    // show time is fairly well predicted by IL size.
+    unsigned ilSize = context->GetCodeSize();
 
+    // Prediction varies for root and inlines.
     if (context == m_RootContext)
     {
-        // Root method
-        return 60 + 3 * ILSize;
+        return EstimateRootTime(ilSize);
     }
     else
     {
-        // Inline
-        return -14 + 2 * ILSize;
+        return EstimateInlineTime(ilSize);
     }
 }
 
 //------------------------------------------------------------------------
+// EstimteRootTime: estimate jit time for method of this size with
+// no inlining.
+//
+// Arguments:
+//    ilSize - size of the method's IL
+//
+// Return Value:
+//    Nominal estimate of jit time.
+//
+// Notes:
+//    Based on observational data. Time is nominally microseconds.
+
+int InlineStrategy::EstimateRootTime(unsigned ilSize)
+{
+    return 60 + 3 * ilSize;
+}
+
+//------------------------------------------------------------------------
+// EstimteInlineTime: estimate time impact on jitting for an inline
+// of this size.
+//
+// Arguments:
+//    ilSize - size of the method's IL
+//
+// Return Value:
+//    Nominal increase in jit time.
+//
+// Notes:
+//    Based on observational data. Time is nominally microseconds.
+//    Small inlines will make the jit a bit faster.
+
+int InlineStrategy::EstimateInlineTime(unsigned ilSize)
+{
+    return -14 + 2 * ilSize;
+}
+
+//------------------------------------------------------------------------
 // NoteOutcome: do bookkeeping for an inline
 //
 // Arguments:
@@ -789,6 +834,22 @@ void InlineStrategy::NoteOutcome(InlineContext* context)
 }
 
 //------------------------------------------------------------------------
+// BudgetCheck: return true if as inline of this size would exceed the
+// jit time budget for this method
+//
+// Arguments:
+//     ilSize - size of the method's IL
+//
+// Return Value:
+//     true if the inline would go over budget
+
+bool InlineStrategy::BudgetCheck(unsigned ilSize)
+{
+    int timeDelta = EstimateInlineTime(ilSize);
+    return (timeDelta + m_CurrentTimeEstimate > m_CurrentTimeBudget);
+}
+
+//------------------------------------------------------------------------
 // NewRoot: construct an InlineContext for the root method
 //
 // Return Value:

diff --git a/src/coreclr/src/jit/inline.def b/src/coreclr/src/jit/inline.def
index 03979b1..6684891 100644 (file)
--- a/src/coreclr/src/jit/inline.def
+++ b/src/coreclr/src/jit/inline.def
@@ -138,6 +138,7 @@ INLINE_OBSERVATION(LDFLD_NEEDS_HELPER,        bool,   "ldfld needs helper",
 INLINE_OBSERVATION(LDVIRTFN_ON_NON_VIRTUAL,   bool,   "ldvirtfn on non-virtual",       FATAL,       CALLSITE)
 INLINE_OBSERVATION(NOT_CANDIDATE,             bool,   "not inline candidate",          FATAL,       CALLSITE)
 INLINE_OBSERVATION(NOT_PROFITABLE_INLINE,     bool,   "unprofitable inline",           FATAL,       CALLSITE)
+INLINE_OBSERVATION(OVER_BUDGET,               bool,   "inline exceeds budget",         FATAL,       CALLSITE)
 INLINE_OBSERVATION(OVER_INLINE_LIMIT,         bool,   "limited by JitInlineLimit",     FATAL,       CALLSITE)
 INLINE_OBSERVATION(RANDOM_REJECT,             bool,   "random reject",                 FATAL,       CALLSITE)
 INLINE_OBSERVATION(REQUIRES_SAME_THIS,        bool,   "requires same this",            FATAL,       CALLSITE)

diff --git a/src/coreclr/src/jit/inline.h b/src/coreclr/src/jit/inline.h
index 8dfed32..efc2db4 100644 (file)
--- a/src/coreclr/src/jit/inline.h
+++ b/src/coreclr/src/jit/inline.h
@@ -660,6 +660,10 @@ public:
     // Inform strategy that there's a new inline candidate.
     void NoteCandidate();
 
+    // See if an inline of this size would fit within the current jit
+    // time budget.
+    bool BudgetCheck(unsigned ilSize);
+
 #if defined(DEBUG) || defined(INLINE_DATA)
 
     // Dump textual description of inlines done so far.
@@ -687,6 +691,10 @@ private:
     // Estimate the jit time change because of this inline.
     int EstimateTime(InlineContext* context);
 
+    // EstimateTime helpers
+    int EstimateRootTime(unsigned ilSize);
+    int EstimateInlineTime(unsigned ilSize);
+
 #if defined(DEBUG) || defined(INLINE_DATA)
     static bool    s_DumpDataHeader;
 #endif // defined(DEBUG) || defined(INLINE_DATA)

diff --git a/src/coreclr/src/jit/inlinepolicy.cpp b/src/coreclr/src/jit/inlinepolicy.cpp
index bc13855..d27a21c 100644 (file)
--- a/src/coreclr/src/jit/inlinepolicy.cpp
+++ b/src/coreclr/src/jit/inlinepolicy.cpp
@@ -321,6 +321,37 @@ void LegacyPolicy::NoteBool(InlineObservation obs, bool value)
                     m_MethodIsMostlyLoadStore = true;
                 }
 
+                // Budget check.
+                //
+                // Conceptually this should happen when we
+                // observe the candidate's IL size.
+                //
+                // However, we do this here to avoid potential
+                // inconsistency between the state of the budget
+                // during candidate scan and the state when the IL is
+                // being scanned.
+                //
+                // Consider the case where we're just below the budget
+                // during candidate scan, and we have three possible
+                // inlines, any two of which put us over budget. We
+                // allow them all to become candidates. We then move
+                // on to inlining and the first two get inlined and
+                // put us over budget. Now the third can't be inlined
+                // anymore, but we have a policy that when we replay
+                // the candidate IL size during the inlining pass it
+                // "reestablishes" candidacy rather than alters
+                // candidacy ... so instead we bail out here.
+
+                if (!m_IsPrejitRoot)
+                {
+                    InlineStrategy* strategy = m_RootCompiler->m_inlineStrategy;
+                    bool overBudget = strategy->BudgetCheck(m_CodeSize);
+                    if (overBudget)
+                    {
+                        SetFailure(InlineObservation::CALLSITE_OVER_BUDGET);
+                    }
+                }
+
                 break;
             }
 

diff --git a/src/coreclr/tests/src/JIT/opt/Inline/tests/LotsOfInlines.cs b/src/coreclr/tests/src/JIT/opt/Inline/tests/LotsOfInlines.cs
new file mode 100644 (file)
index 0000000..3a8b4d2
--- /dev/null
+++ b/src/coreclr/tests/src/JIT/opt/Inline/tests/LotsOfInlines.cs
@@ -0,0 +1,63 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+//
+// Thanks to Alexander Speshilov (spechuric @ github).
+
+using System;
+
+namespace Repro
+{
+    public class Program
+    {
+        public static int Main(string[] args)
+        {
+            Tst a = new Tst();
+            a.f30();
+            long expected = 1 << 30;
+            Console.WriteLine("result = {0} expected = {1}", a.i, expected);
+            return (a.i == expected ? 100 : -1);
+        }
+    }
+
+    public class Tst
+    {
+        public long i = 0;
+
+        // NOTE: this method executes 2^N times
+        public void f00() { i++; }
+
+        public void f01() { f00(); f00(); }
+        public void f02() { f01(); f01(); }
+        public void f03() { f02(); f02(); }
+        public void f04() { f03(); f03(); }
+        public void f05() { f04(); f04(); }
+        public void f06() { f05(); f05(); }
+        public void f07() { f06(); f06(); }
+        public void f08() { f07(); f07(); }
+        public void f09() { f08(); f08(); }
+        public void f10() { f09(); f09(); }
+
+        public void f11() { f10(); f10(); }
+        public void f12() { f11(); f11(); }
+        public void f13() { f12(); f12(); }
+        public void f14() { f13(); f13(); }
+        public void f15() { f14(); f14(); }
+        public void f16() { f15(); f15(); }
+        public void f17() { f16(); f16(); }
+        public void f18() { f17(); f17(); }
+        public void f19() { f18(); f18(); }
+        public void f20() { f19(); f19(); }
+
+        public void f21() { f20(); f20(); }
+        public void f22() { f21(); f21(); }
+        public void f23() { f22(); f22(); }
+        public void f24() { f23(); f23(); }
+        public void f25() { f24(); f24(); }
+        public void f26() { f25(); f25(); }
+        public void f27() { f26(); f26(); }
+        public void f28() { f27(); f27(); }
+        public void f29() { f28(); f28(); }
+        public void f30() { f29(); f29(); }
+    }
+}

diff --git a/src/coreclr/tests/src/JIT/opt/Inline/tests/LotsOfInlines.csproj b/src/coreclr/tests/src/JIT/opt/Inline/tests/LotsOfInlines.csproj
new file mode 100644 (file)
index 0000000..ae4ef5d
--- /dev/null
+++ b/src/coreclr/tests/src/JIT/opt/Inline/tests/LotsOfInlines.csproj
@@ -0,0 +1,53 @@
+<?xml version="1.0" encoding="utf-8"?>
+<Project ToolsVersion="12.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
+  <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.props))\dir.props" />
+  <PropertyGroup>
+    <Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration>
+    <Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform>
+    <AssemblyName>$(MSBuildProjectName)</AssemblyName>
+    <SchemaVersion>2.0</SchemaVersion>
+    <ProjectGuid>{95DFC527-4DC1-495E-97D7-E94EE1F7140D}</ProjectGuid>
+    <OutputType>Exe</OutputType>
+    <AppDesignerFolder>Properties</AppDesignerFolder>
+    <FileAlignment>512</FileAlignment>
+    <ProjectTypeGuids>{786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}</ProjectTypeGuids>
+    <ReferencePath>$(ProgramFiles)\Common Files\microsoft shared\VSTT  .0\UITestExtensionPackages</ReferencePath>
+    <SolutionDir Condition="$(SolutionDir) == '' Or $(SolutionDir) == '*Undefined*'">..\..\</SolutionDir>
+    <NuGetPackageImportStamp>7a9bfb7d</NuGetPackageImportStamp>
+  </PropertyGroup>
+  <!-- Default configurations to help VS understand the configurations -->
+  <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' ">
+  </PropertyGroup>
+  <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Release|AnyCPU' ">
+  </PropertyGroup>
+  <ItemGroup>
+    <CodeAnalysisDependentAssemblyPaths Condition=" '$(VS100COMNTOOLS)' != '' " Include="$(VS100COMNTOOLS)..\IDE\PrivateAssemblies">
+      <Visible>False</Visible>
+    </CodeAnalysisDependentAssemblyPaths>
+  </ItemGroup>
+  <PropertyGroup>
+    <!-- Set to 'Full' if the Debug? column is marked in the spreadsheet. Leave blank otherwise. -->
+    <DebugType>None</DebugType>
+    <NoLogo>True</NoLogo>
+    <NoStandardLib>True</NoStandardLib>
+    <Noconfig>True</Noconfig>
+    <DefineConstants>$(DefineConstants);CORECLR</DefineConstants>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="LotsOfInlines.cs" />
+  </ItemGroup>
+  <ItemGroup>
+    <None Include="$(JitPackagesConfigFileDirectory)minimal\project.json" />
+    <None Include="app.config" />
+  </ItemGroup>
+  <ItemGroup>
+    <Service Include="{82A7F48D-3B50-4B1E-B82E-3ADA8210C358}" />
+  </ItemGroup>
+  <PropertyGroup>
+    <ProjectJson>$(JitPackagesConfigFileDirectory)minimal\project.json</ProjectJson>
+    <ProjectLockJson>$(JitPackagesConfigFileDirectory)minimal\project.lock.json</ProjectLockJson>
+  </PropertyGroup>
+  <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />
+  <PropertyGroup Condition=" '$(MsBuildProjectDirOverride)' != '' ">
+  </PropertyGroup> 
+</Project>