diff options
author | Andy Ayers <andya@microsoft.com> | 2016-10-12 18:00:13 -0700 |
---|---|---|
committer | Andy Ayers <andya@microsoft.com> | 2016-10-19 12:17:35 -0700 |
commit | 1e0359ce3e5d25ba4dbbe72336653ea1c94113ac (patch) | |
tree | 31da0d21c2a1464936f4a14ee7f6fe374b99f37e | |
parent | c764a9585625d88922ca92ea0798cd1bbce316c0 (diff) | |
download | coreclr-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.cpp | 41 | ||||
-rw-r--r-- | src/jit/inline.def | 1 | ||||
-rw-r--r-- | src/jit/inlinepolicy.cpp | 21 | ||||
-rw-r--r-- | tests/src/JIT/Performance/CodeQuality/Inlining/InlineGCStruct.cs | 121 | ||||
-rw-r--r-- | tests/src/JIT/Performance/CodeQuality/Inlining/InlineGCStruct.csproj | 44 |
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> |