summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAndy Ayers <andya@microsoft.com>2016-10-12 18:00:13 -0700
committerAndy Ayers <andya@microsoft.com>2016-10-19 12:17:35 -0700
commit1e0359ce3e5d25ba4dbbe72336653ea1c94113ac (patch)
tree31da0d21c2a1464936f4a14ee7f6fe374b99f37e
parentc764a9585625d88922ca92ea0798cd1bbce316c0 (diff)
downloadcoreclr-1e0359ce3e5d25ba4dbbe72336653ea1c94113ac.tar.gz
coreclr-1e0359ce3e5d25ba4dbbe72336653ea1c94113ac.tar.bz2
coreclr-1e0359ce3e5d25ba4dbbe72336653ea1c94113ac.zip
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.
-rw-r--r--src/jit/importer.cpp41
-rw-r--r--src/jit/inline.def1
-rw-r--r--src/jit/inlinepolicy.cpp21
-rw-r--r--tests/src/JIT/Performance/CodeQuality/Inlining/InlineGCStruct.cs121
-rw-r--r--tests/src/JIT/Performance/CodeQuality/Inlining/InlineGCStruct.csproj44
5 files changed, 227 insertions, 1 deletions
diff --git a/src/jit/importer.cpp b/src/jit/importer.cpp
index 53e856ce87..1c6d76101c 100644
--- 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 794acd3f3f..71d441056a 100644
--- 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 f80f3a5ec0..20d18c4e4d 100644
--- 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
index 0000000000..dee73cbd4f
--- /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
index 0000000000..cb3381638c
--- /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>