Inliner: avoid inlining callees with GC structs on cold paths

If an inline introduces a local or temp GC struct, the initialization
of that struct is done in the root method prolog. This can hurt
performance if the inline call site is cold. Detect this during
importation and update the inline policy to back out of the inline if
the inline is an always or discretionary candidate.

Jit diffs shows size wins in the core library with no regressions.
```
Total bytes of diff: -8789 (-0.29 % of base)
    diff is an improvement.
Total byte diff includes 0 bytes from reconciling methods
        Base had    0 unique methods,        0 unique bytes
        Diff had    0 unique methods,        0 unique bytes
Top file improvements by size (bytes):
       -8789 : System.Private.CoreLib.dasm (-0.29 % of base)
1 total files with size differences.
Top method improvements by size (bytes):
        -320 : System.Private.CoreLib.dasm - FrameSecurityDescriptor:CheckDemand2(ref,ref,long,bool):bool:this
        -224 : System.Private.CoreLib.dasm - RuntimeConstructorInfo:CheckCanCreateInstance(ref,bool)
        -214 : System.Private.CoreLib.dasm - RuntimeType:GetMethodBase(ref,long):ref
        -150 : System.Private.CoreLib.dasm - CultureInfo:GetCultureInfoByIetfLanguageTag(ref):ref
        -146 : System.Private.CoreLib.dasm - GC:RegisterForFullGCNotification(int,int)
103 total methods with size differences.
```
Desktop testing shows similar wins in a number of places and only
a few sparse small regressions.

Added a perf test. Thanks to @hypersw for noticing this.

Closes #7569.

diff --git a/src/jit/importer.cpp b/src/jit/importer.cpp
index 53e856ce87dbc08129cd68a8c641c72c1cf3ca16..1c6d76101c7135eb8335a2c6e2ca374b3d5da92b 100644 (file)
--- a/src/jit/importer.cpp
+++ b/src/jit/importer.cpp
@@ -12359,6 +12359,26 @@ void Compiler::impImportBlockCode(BasicBlock* block)
                     // The lookup of the code pointer will be handled by CALL in this case
                     if (clsFlags & CORINFO_FLG_VALUECLASS)
                     {
+                        if (compIsForInlining())
+                        {
+                            if (impInlineInfo->iciBlock->isRunRarely())
+                            {
+                                // If value class has GC fields, inform
+                                // the inliner. It may choose to bail out
+                                // on the inline, if the call site is
+                                // rare.
+                                DWORD typeFlags = info.compCompHnd->getClassAttribs(resolvedToken.hClass);
+                                if ((typeFlags & CORINFO_FLG_CONTAINS_GC_PTR) != 0)
+                                {
+                                    compInlineResult->Note(InlineObservation::CALLSITE_RARE_GC_STRUCT);
+                                    if (compInlineResult->IsFailure())
+                                    {
+                                        return;
+                                    }
+                                }
+                            }
+                        }
+
                         CorInfoType jitTyp = info.compCompHnd->asCorInfoType(resolvedToken.hClass);
                         unsigned    size   = info.compCompHnd->getClassSize(resolvedToken.hClass);
 
@@ -17248,6 +17268,7 @@ void Compiler::impInlineInitVars(InlineInfo* pInlineInfo)
         var_types sigType = (var_types)eeGetArgType(argLst, &methInfo->args);
 
         lclVarInfo[i].lclVerTypeInfo = verParseArgSigToTypeInfo(&methInfo->args, argLst);
+
 #ifdef FEATURE_SIMD
         if ((!foundSIMDType || (sigType == TYP_STRUCT)) && isSIMDClass(&(lclVarInfo[i].lclVerTypeInfo)))
         {
@@ -17384,6 +17405,26 @@ void Compiler::impInlineInitVars(InlineInfo* pInlineInfo)
 
         lclVarInfo[i + argCnt].lclVerTypeInfo = verParseArgSigToTypeInfo(&methInfo->locals, localsSig);
 
+        // If this local is a struct type with GC fields, inform the
+        // inliner. It may choose to bail out on the inline, if the call
+        // site is rare.
+        if (pInlineInfo->iciBlock->isRunRarely())
+        {
+            if (type == TYP_STRUCT)
+            {
+                CORINFO_CLASS_HANDLE lclHandle = lclVarInfo[i + argCnt].lclVerTypeInfo.GetClassHandle();
+                DWORD                typeFlags = info.compCompHnd->getClassAttribs(lclHandle);
+                if ((typeFlags & CORINFO_FLG_CONTAINS_GC_PTR) != 0)
+                {
+                    inlineResult->Note(InlineObservation::CALLSITE_RARE_GC_STRUCT);
+                    if (inlineResult->IsFailure())
+                    {
+                        return;
+                    }
+                }
+            }
+        }
+
         localsSig = info.compCompHnd->getArgNext(localsSig);
 
 #ifdef FEATURE_SIMD

diff --git a/src/jit/inline.def b/src/jit/inline.def
index 794acd3f3f19adcc33ba01ecf8943f8188280839..71d441056a2527cce12a6d5a82225c62da1bf2b9 100644 (file)
--- a/src/jit/inline.def
+++ b/src/jit/inline.def
@@ -156,6 +156,7 @@ INLINE_OBSERVATION(TOO_MANY_LOCALS,           bool,   "too many locals",
 
 // ------ Call Site Performance ------- 
 
+INLINE_OBSERVATION(RARE_GC_STRUCT,            bool,   "rarely called, has gc struct",  FATAL,       CALLSITE)
 
 // ------ Call Site Information ------- 
 

diff --git a/src/jit/inlinepolicy.cpp b/src/jit/inlinepolicy.cpp
index f80f3a5ec06cf95f6abd6d5a448190e5d01b74ce..20d18c4e4d9376537f80668194fc0c467c34cac1 100644 (file)
--- a/src/jit/inlinepolicy.cpp
+++ b/src/jit/inlinepolicy.cpp
@@ -842,7 +842,7 @@ int LegacyPolicy::CodeSizeEstimate()
 // NoteBool: handle a boolean observation with non-fatal impact
 //
 // Arguments:
-//    obs      - the current obsevation
+//    obs      - the current observation
 //    value    - the value of the observation
 
 void EnhancedLegacyPolicy::NoteBool(InlineObservation obs, bool value)
@@ -854,6 +854,25 @@ void EnhancedLegacyPolicy::NoteBool(InlineObservation obs, bool value)
             m_IsNoReturnKnown = true;
             break;
 
+        case InlineObservation::CALLSITE_RARE_GC_STRUCT:
+            // If this is a discretionary or always inline candidate
+            // with a gc struct, we may change our mind about inlining
+            // if the call site is rare, to avoid costs associated with
+            // zeroing the GC struct up in the root prolog.
+            if (m_Observation == InlineObservation::CALLEE_BELOW_ALWAYS_INLINE_SIZE)
+            {
+                assert(m_CallsiteFrequency == InlineCallsiteFrequency::UNUSED);
+                SetFailure(obs);
+                return;
+            }
+            else if (m_Observation == InlineObservation::CALLEE_IS_DISCRETIONARY_INLINE)
+            {
+                assert(m_CallsiteFrequency == InlineCallsiteFrequency::RARE);
+                SetFailure(obs);
+                return;
+            }
+            break;
+
         default:
             // Pass all other information to the legacy policy
             LegacyPolicy::NoteBool(obs, value);

diff --git a/tests/src/JIT/Performance/CodeQuality/Inlining/InlineGCStruct.cs b/tests/src/JIT/Performance/CodeQuality/Inlining/InlineGCStruct.cs
new file mode 100644 (file)
index 0000000..dee73cb
--- /dev/null
+++ b/tests/src/JIT/Performance/CodeQuality/Inlining/InlineGCStruct.cs
@@ -0,0 +1,121 @@
+// 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.
+
+using Microsoft.Xunit.Performance;
+using System;
+using System.Runtime.CompilerServices;
+using Xunit;
+
+[assembly: OptimizeForBenchmarks]
+[assembly: MeasureInstructionsRetired]
+
+public class InlineGCStruct
+{
+
+#if DEBUG
+    public const int Iterations = 1;
+#else
+    public const int Iterations = 2500000;
+#endif
+
+[MethodImpl(MethodImplOptions.NoInlining)]
+public static int FastFunctionNotCallingStringFormat(int param)
+{
+    if (param < 0) {
+        throw new Exception(String.Format("We do not like the value {0:N0}.", param));
+    }
+
+    if (param == int.MaxValue) {
+        throw new Exception(String.Format("{0:N0} is maxed out.", param));
+    }
+
+    if (param > int.MaxValue / 2) {
+        throw new Exception(String.Format("We do not like the value {0:N0} either.", param));
+    }
+
+    return param * 2;
+}
+
+[MethodImpl(MethodImplOptions.NoInlining)]
+public static int FastFunctionNotHavingStringFormat(int param)
+{
+    if (param < 0) {
+        throw new ArgumentOutOfRangeException("param", "We do not like this value.");
+    }
+
+    if (param == int.MaxValue) {
+        throw new ArgumentOutOfRangeException("param", "Maxed out.");
+    }
+
+    if (param > int.MaxValue / 2) {
+        throw new ArgumentOutOfRangeException("param", "We do not like this value either.");
+    }
+
+    return param * 2;
+}
+
+[Benchmark]
+public static bool WithFormat()
+{
+    int result = 0;
+
+    foreach (var iteration in Benchmark.Iterations) {
+        using (iteration.StartMeasurement()) {
+            for (int i = 0; i < Iterations; i++) {
+                result |= FastFunctionNotCallingStringFormat(11);
+            }
+        }
+    }
+
+    return (result == 22);
+}
+
+[Benchmark]
+public static bool WithoutFormat()
+{
+    int result = 0;
+
+    foreach (var iteration in Benchmark.Iterations) {
+        using (iteration.StartMeasurement()) {
+            for (int i = 0; i < Iterations; i++) {
+                result |= FastFunctionNotHavingStringFormat(11);
+            }
+        }
+    }
+
+    return (result == 22);
+}
+
+public static bool WithoutFormatBase()
+{
+    int result = 0;
+
+    for (int i = 0; i < Iterations; i++) {
+        result |= FastFunctionNotHavingStringFormat(11);
+    }
+
+    return (result == 22);
+}
+
+public static bool WithFormatBase()
+{
+    int result = 0;
+
+    for (int i = 0; i < Iterations; i++) {
+        result |= FastFunctionNotCallingStringFormat(11);
+    }
+
+    return (result == 22);
+}
+
+public static int Main()
+{
+    bool withFormat = WithFormatBase();
+    bool withoutFormat = WithoutFormatBase();
+
+    return (withFormat && withoutFormat ? 100 : -1);
+}
+
+}
+

diff --git a/tests/src/JIT/Performance/CodeQuality/Inlining/InlineGCStruct.csproj b/tests/src/JIT/Performance/CodeQuality/Inlining/InlineGCStruct.csproj
new file mode 100644 (file)
index 0000000..cb33816
--- /dev/null
+++ b/tests/src/JIT/Performance/CodeQuality/Inlining/InlineGCStruct.csproj
@@ -0,0 +1,44 @@
+<?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>
+    <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\11.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' ">
+    <DebugType>pdbonly</DebugType>
+    <Optimize>true</Optimize>
+  </PropertyGroup>
+  <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Release|AnyCPU' ">
+    <DebugType>pdbonly</DebugType>
+    <Optimize>true</Optimize>
+  </PropertyGroup>
+  <ItemGroup>
+    <CodeAnalysisDependentAssemblyPaths Condition=" '$(VS100COMNTOOLS)' != '' " Include="$(VS100COMNTOOLS)..\IDE\PrivateAssemblies">
+      <Visible>False</Visible>
+    </CodeAnalysisDependentAssemblyPaths>
+  </ItemGroup>
+  <ItemGroup>
+    <Service Include="{82A7F48D-3B50-4B1E-B82E-3ADA8210C358}" />
+  </ItemGroup>
+  <ItemGroup>
+    <Compile Include="InlineGCStruct.cs" />
+  </ItemGroup>
+  <PropertyGroup>
+    <ProjectJson>$(JitPackagesConfigFileDirectory)benchmark\project.json</ProjectJson>
+    <ProjectLockJson>$(JitPackagesConfigFileDirectory)benchmark\project.lock.json</ProjectLockJson>
+  </PropertyGroup>
+  <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />
+  <PropertyGroup Condition=" '$(MsBuildProjectDirOverride)' != '' ">
+  </PropertyGroup>
+</Project>