Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JIT: enable reading PGO data when switching to optimized #66618

Merged

Conversation

AndyAyersMS
Copy link
Member

When QuickJitForLoops=0 (current default) and the JIT sees a Tier0 method with
a loop, it will switch to optimizing the method instead.

When this happens we should also have set BBOPT so that the JIT will read and
incorporate PGO data for the method and/or its inliees. But we were not doing that.

This change sets BBOPT.

When QuickJitForLoops=0 (current default) and the JIT sees a Tier0 method with
a loop, it will switch to optimizing the method instead.

When this happens we should also have set `BBOPT` so that the JIT will read and
incorporate PGO data for the method and/or its inliees. But we were not doing that.

This change sets `BBOPT`.
@ghost ghost assigned AndyAyersMS Mar 14, 2022
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Mar 14, 2022
@ghost
Copy link

ghost commented Mar 14, 2022

Tagging subscribers to this area: @JulieLeeMSFT
See info in area-owners.md if you want to be subscribed.

Issue Details

When QuickJitForLoops=0 (current default) and the JIT sees a Tier0 method with
a loop, it will switch to optimizing the method instead.

When this happens we should also have set BBOPT so that the JIT will read and
incorporate PGO data for the method and/or its inliees. But we were not doing that.

This change sets BBOPT.

Author: AndyAyersMS
Assignees: AndyAyersMS
Labels:

area-CodeGen-coreclr

Milestone: -

@AndyAyersMS
Copy link
Member Author

@EgorBo PTAL
cc @dotnet/jit-contrib

This has the potential to cause perf swings. Hopefully mostly for the better.

I know of at least one regression in microbenchmarks: in System.Numerics.Tests.Perf_BitOperations.PopCount_ulong we will stop aligning a loop because we no longer think the body is executed enough to warrant alignment. We may also see a regression in System.MathBenchmarks.Single.Min as the profile data there is oddly biased.

See #33658 (comment) for some context.

@AndyAyersMS
Copy link
Member Author

No SPMI diffs. Presumably because when we originally jitted all those methods we never asked for PGO data, so if we now start asking, there's none to be had, and we end up in the same place.

I'll run some PMI diffs locally and post the results.

@AndyAyersMS
Copy link
Member Author

AndyAyersMS commented Mar 15, 2022

I'll run some PMI diffs locally and post the results.

I had to hack jit-diff a bit, to not disable R2R or enable QJFL. So take these diffs with a grain of salt.

PMI CodeSize Diffs for System.Private.CoreLib.dll, framework assemblies [tier0] for x64 default jit

Summary of Code Size diffs:
(Lower is better)

Total bytes of base: 65442053
Total bytes of diff: 65565442
Total bytes of delta: 123389 (0.19 % of base)
Total relative delta: 103.00
    diff is a regression.
    relative diff is a regression.


Top file regressions (bytes):
       15334 : System.Private.Xml.dasm (0.35% of base)
        9756 : Microsoft.Extensions.Logging.Console.dasm (10.36% of base)
        9491 : System.Linq.Expressions.dasm (0.98% of base)
        6458 : System.Linq.Parallel.dasm (0.27% of base)
        5637 : System.Data.Common.dasm (0.34% of base)
        5467 : Microsoft.Diagnostics.Tracing.TraceEvent.dasm (0.13% of base)
        5415 : System.Net.WebSockets.dasm (4.68% of base)
        4833 : System.Net.Http.dasm (0.58% of base)
        3753 : System.Private.DataContractSerialization.dasm (0.39% of base)
        3590 : System.Text.Json.dasm (0.20% of base)
        3210 : System.Threading.Tasks.Parallel.dasm (1.33% of base)
        3088 : System.Private.CoreLib.dasm (0.12% of base)
        2782 : FSharp.Core.dasm (0.05% of base)
        2307 : Microsoft.CodeAnalysis.dasm (0.10% of base)
        2304 : Microsoft.VisualBasic.Core.dasm (0.39% of base)
        2210 : Newtonsoft.Json.dasm (0.21% of base)
        1977 : xunit.execution.dotnet.dasm (0.64% of base)
        1825 : Microsoft.CodeAnalysis.CSharp.dasm (0.03% of base)
        1732 : Microsoft.CSharp.dasm (0.38% of base)
        1602 : System.Composition.Hosting.dasm (1.36% of base)

Top file improvements (bytes):
       -1067 : Microsoft.CodeAnalysis.VisualBasic.dasm (-0.02% of base)
        -373 : System.Collections.Immutable.dasm (-0.02% of base)
        -101 : System.Windows.Extensions.dasm (-0.22% of base)
         -94 : System.Net.Requests.dasm (-0.07% of base)
         -57 : Newtonsoft.Json.Bson.dasm (-0.05% of base)
         -49 : Microsoft.Extensions.Configuration.Binder.dasm (-0.26% of base)
         -45 : Microsoft.Extensions.Logging.EventSource.dasm (-0.14% of base)
         -45 : Microsoft.Extensions.FileProviders.Physical.dasm (-0.21% of base)
         -32 : System.Security.Claims.dasm (-0.13% of base)
         -24 : System.ServiceModel.Syndication.dasm (-0.01% of base)
         -21 : System.Collections.Specialized.dasm (-0.07% of base)
         -21 : Microsoft.Extensions.Caching.Memory.dasm (-0.09% of base)
         -12 : Microsoft.Extensions.Configuration.Ini.dasm (-0.23% of base)
         -12 : Microsoft.Extensions.Configuration.EnvironmentVariables.dasm (-0.35% of base)
         -12 : System.Collections.NonGeneric.dasm (-0.04% of base)
          -9 : Microsoft.Extensions.Primitives.dasm (-0.03% of base)
          -8 : System.Threading.Tasks.Dataflow.dasm (-0.00% of base)
          -6 : System.Net.WebHeaderCollection.dasm (-0.03% of base)
          -4 : OSExtensions.dasm (-0.04% of base)
          -3 : System.Text.Encoding.CodePages.dasm (-0.00% of base)

150 total files with Code Size differences (20 improved, 130 regressed), 121 unchanged.

Top method regressions (bytes):
        9518 (35.74% of base) : Microsoft.Extensions.Logging.Console.dasm - JsonConsoleFormatter:Write(byref,IExternalScopeProvider,TextWriter):this (8 methods)
        5241 ( 9.36% of base) : System.Net.WebSockets.dasm - <ReceiveAsyncPrivate>d__63`1:MoveNext():this (8 methods)
        3256 ( 5.39% of base) : System.Data.Common.dasm - SortExpressionBuilder`1:CloneCast():SortExpressionBuilder`1:this (64 methods)
        2487 (12.36% of base) : System.Linq.Parallel.dasm - SortHelper`2:MergeSortCooperatively():this (8 methods)
        2088 (11.39% of base) : System.Threading.Tasks.Parallel.dasm - <<ForEachAsync>b__54_0>d:MoveNext():this (8 methods)
        1736 (24.33% of base) : System.Private.DataContractSerialization.dasm - DataNode`1:AddQualifiedNameAttribute(ElementData,String,String,String,String,String):this (8 methods)
        1582 ( 8.61% of base) : System.Composition.Hosting.dasm - MetadataViewProvider:GetMetadataViewProvider():Func`2 (8 methods)
        1494 (189.83% of base) : System.Private.Xml.dasm - XmlWriterSettings:GetObjectData(XmlQueryDataWriter):this
        1408 ( 8.17% of base) : Microsoft.CodeAnalysis.VisualBasic.dasm - MethodToClassRewriter`1:RewriteBlock(BoundBlock,ArrayBuilder`1,ArrayBuilder`1):BoundBlock:this (8 methods)
        1352 (11.03% of base) : System.Threading.Tasks.Parallel.dasm - <>c__DisplayClass44_0`2:<PartitionerForEachWorker>b__1(byref,int,byref):this (8 methods)
        1172 ( 6.37% of base) : FSharp.Core.dasm - Array2DModule:CopyTo(ref,int,int,ref,int,int,int,int) (8 methods)
        1168 ( 4.46% of base) : System.Text.Json.dasm - <WriteStreamAsync>d__112`1:MoveNext():this (8 methods)
        1011 (15.04% of base) : System.Threading.Channels.dasm - <WriteAsyncCore>d__4:MoveNext():this (8 methods)
        1002 (21.87% of base) : System.Private.CoreLib.dasm - String:JoinCore(ReadOnlySpan`1,IEnumerable`1):String (6 methods)
         972 (22.24% of base) : System.Private.CoreLib.dasm - Vector`1:ToString(String,IFormatProvider):String:this (6 methods)
         862 (56.67% of base) : System.Private.Xml.dasm - XmlQueryStaticData:GetObjectData(byref,byref):this
         856 (18.14% of base) : System.Private.CoreLib.dasm - String:Concat(IEnumerable`1):String (7 methods)
         788 (69.80% of base) : Microsoft.VisualBasic.Core.dasm - ConversionResolution:ClassifyPredefinedCLRConversion(Type,Type):byte
         776 (21.28% of base) : System.Linq.Expressions.dasm - Expression:Switch(Type,Expression,Expression,MethodInfo,IEnumerable`1):SwitchExpression
         760 (140.74% of base) : System.Linq.Expressions.dasm - TypeUtils:StrictHasReferenceConversionTo(Type,Type,bool):bool

Top method improvements (bytes):
       -2476 (-45.98% of base) : Microsoft.CodeAnalysis.VisualBasic.dasm - Binder:BuildBoundLambdaParameters(UnboundLambda,TargetSignature,DiagnosticBag):ImmutableArray`1:this
       -1832 (-46.85% of base) : Microsoft.CodeAnalysis.CSharp.dasm - Binder:GetRewrittenAttributeConstructorArguments(byref,MethodSymbol,ImmutableArray`1,ImmutableArray`1,AttributeSyntax,DiagnosticBag,byref):ImmutableArray`1:this
       -1404 (-38.89% of base) : Microsoft.CodeAnalysis.VisualBasic.dasm - SourceMemberContainerTypeSymbol:ReportErrorsOnPartialMethodImplementation(SourceMethodSymbol,SourceMethodSymbol,Location,DiagnosticBag):this
       -1372 (-44.07% of base) : Microsoft.CodeAnalysis.VisualBasic.dasm - Binder:ConstructAnonymousDelegateSymbol(UnboundLambda,ImmutableArray`1,TypeSymbol,DiagnosticBag):NamedTypeSymbol:this
       -1292 (-31.28% of base) : Microsoft.VisualBasic.Core.dasm - VBBinder:GetMethodsByName(Type,IReflect,String,int):ref:this
        -846 (-3.50% of base) : Microsoft.CodeAnalysis.VisualBasic.dasm - Binder:ReportOverloadResolutionFailureForASingleCandidate(VisualBasicSyntaxNode,Location,int,byref,ImmutableArray`1,ImmutableArray`1,bool,bool,bool,bool,DiagnosticBag,Symbol,bool,VisualBasicSyntaxNode,Symbol):this
        -804 (-31.79% of base) : Microsoft.CodeAnalysis.CSharp.dasm - AnonymousTypeTemplateSymbol:.ctor(AnonymousTypeManager,AnonymousTypeDescriptor):this
        -510 (-2.06% of base) : Microsoft.VisualBasic.Core.dasm - VBBinder:BindToMethod(int,ref,byref,ref,CultureInfo,ref,byref):MethodBase:this
        -482 (-10.58% of base) : Microsoft.CodeAnalysis.VisualBasic.dasm - SourceEventSymbol:ComputeType(DiagnosticBag,byref,byref):TypeSymbol:this
        -462 (-15.01% of base) : Microsoft.CodeAnalysis.CSharp.dasm - MetadataDecoder:SubstituteNoPiaLocalType(byref,bool,TypeSymbol,String,String,String,AssemblySymbol):NamedTypeSymbol
        -444 (-11.89% of base) : System.Private.Xml.dasm - DecimalFormatter:.ctor(String,DecimalFormat):this
        -416 (-18.46% of base) : Microsoft.CodeAnalysis.CSharp.dasm - Binder:BindArrayCreationWithInitializer(DiagnosticBag,ExpressionSyntax,InitializerExpressionSyntax,ArrayTypeSymbol,ImmutableArray`1,ImmutableArray`1):BoundArrayCreation:this
        -410 (-28.77% of base) : Microsoft.VisualBasic.Core.dasm - VBBinder:BindingScore(ref,ref,ref,bool,int):int:this
        -394 (-13.74% of base) : System.DirectoryServices.AccountManagement.dasm - PrincipalContext:DoLDAPDirectoryInitNoContainer():this
        -392 (-12.98% of base) : Microsoft.CodeAnalysis.VisualBasic.dasm - Binder:BuildDelegateRelaxationLambda(VisualBasicSyntaxNode,MethodSymbol,BoundMethodGroup,int,bool,bool,DiagnosticBag):BoundLambda:this
        -388 (-17.33% of base) : Microsoft.CodeAnalysis.VisualBasic.dasm - OverloadResolution:InferTypeArguments(byref,ImmutableArray`1,ImmutableArray`1,TypeSymbol,BoundNode,byref,Binder,byref):bool
        -368 (-14.63% of base) : Microsoft.CodeAnalysis.VisualBasic.dasm - LocalRewriter:BuildDelegateRelaxationLambda(VisualBasicSyntaxNode,NamedTypeSymbol,BoundLateMemberAccess,Binder,DiagnosticBag):BoundExpression
        -350 (-11.99% of base) : Microsoft.CodeAnalysis.VisualBasic.dasm - MetadataDecoder:SubstituteNoPiaLocalType(byref,bool,TypeSymbol,String,String,String,AssemblySymbol):NamedTypeSymbol
        -350 (-21.26% of base) : Microsoft.CodeAnalysis.CSharp.dasm - SourceNamedTypeSymbol:MakeTypeParameterConstraints(DiagnosticBag):ImmutableArray`1:this
        -314 (-37.12% of base) : Microsoft.CSharp.dasm - RuntimeBinder:PopulateLocalScope(ICSharpBinder,Scope,ref,ref):ref

Top method regressions (percentages):
        1494 (189.83% of base) : System.Private.Xml.dasm - XmlWriterSettings:GetObjectData(XmlQueryDataWriter):this
         100 (181.82% of base) : System.Net.Http.dasm - ContentReadStream:Dispose(bool):this
         760 (140.74% of base) : System.Linq.Expressions.dasm - TypeUtils:StrictHasReferenceConversionTo(Type,Type,bool):bool
         652 (112.61% of base) : System.Linq.Expressions.dasm - TypeUtils:IsValidInstanceType(MemberInfo,Type):bool
         306 (109.68% of base) : System.Reflection.MetadataLoadContext.dasm - Assignability:ProvablyAGcReferenceTypeHelper(Type,CoreTypes):bool
          93 (103.33% of base) : System.IO.Pipelines.dasm - StreamPipeReader:Complete(Exception):this
         212 (92.17% of base) : System.DirectoryServices.AccountManagement.dasm - ADStoreCtx:BuildExtensionPropertyList(Hashtable,Type):this
          48 (87.27% of base) : System.ComponentModel.Composition.dasm - ContractNameServices:FindArrayElementType(Type):Type
         217 (83.46% of base) : Microsoft.Extensions.DependencyInjection.Abstractions.dasm - ActivatorUtilities:TryFindPreferredConstructor(Type,ref,byref,byref):bool
         431 (82.88% of base) : Microsoft.VisualBasic.Core.dasm - Symbols:IsOrInheritsFrom(Type,Type):bool
         650 (78.60% of base) : Microsoft.VisualBasic.Core.dasm - OverloadResolution:InferTypeArgumentsFromArgument(Type,Type,ref,MethodBase,bool):bool
         195 (78.00% of base) : System.ComponentModel.Composition.Registration.dasm - PartBuilder:BuildDefaultConstructorAttributes(Type,byref)
         226 (77.93% of base) : System.Text.Json.dasm - IEnumerableConverterFactoryHelpers:GetImmutableEnumerableCreateRangeMethod(Type,Type):MethodInfo
         121 (77.56% of base) : System.Linq.Expressions.dasm - Expression:ValidateTryGetFuncActionArgs(ref):int
         113 (75.84% of base) : Microsoft.VisualBasic.Core.dasm - Symbols:AreParametersAndReturnTypesValid(ref,Type):bool
         225 (71.88% of base) : System.Text.Json.dasm - IEnumerableConverterFactoryHelpers:GetImmutableDictionaryCreateRangeMethod(Type,Type,Type):MethodInfo
         788 (69.80% of base) : Microsoft.VisualBasic.Core.dasm - ConversionResolution:ClassifyPredefinedCLRConversion(Type,Type):byte
         105 (68.63% of base) : Microsoft.VisualBasic.Core.dasm - LateBinding:NoByrefs(ref):bool
         499 (62.14% of base) : System.ComponentModel.TypeConverter.dasm - DesigntimeLicenseContextSerializer:Serialize(Stream,String,DesigntimeLicenseContext)
          51 (59.30% of base) : Microsoft.VisualBasic.Core.dasm - Method:get_HasByRefParameter():bool:this

Top method improvements (percentages):
       -1832 (-46.85% of base) : Microsoft.CodeAnalysis.CSharp.dasm - Binder:GetRewrittenAttributeConstructorArguments(byref,MethodSymbol,ImmutableArray`1,ImmutableArray`1,AttributeSyntax,DiagnosticBag,byref):ImmutableArray`1:this
       -2476 (-45.98% of base) : Microsoft.CodeAnalysis.VisualBasic.dasm - Binder:BuildBoundLambdaParameters(UnboundLambda,TargetSignature,DiagnosticBag):ImmutableArray`1:this
        -211 (-45.67% of base) : System.Runtime.Serialization.Formatters.dasm - ObjectMapInfo:IsCompatible(int,ref,ref):bool:this
       -1372 (-44.07% of base) : Microsoft.CodeAnalysis.VisualBasic.dasm - Binder:ConstructAnonymousDelegateSymbol(UnboundLambda,ImmutableArray`1,TypeSymbol,DiagnosticBag):NamedTypeSymbol:this
       -1404 (-38.89% of base) : Microsoft.CodeAnalysis.VisualBasic.dasm - SourceMemberContainerTypeSymbol:ReportErrorsOnPartialMethodImplementation(SourceMethodSymbol,SourceMethodSymbol,Location,DiagnosticBag):this
        -314 (-37.12% of base) : Microsoft.CSharp.dasm - RuntimeBinder:PopulateLocalScope(ICSharpBinder,Scope,ref,ref):ref
        -236 (-36.99% of base) : System.Reflection.MetadataLoadContext.dasm - RoAssembly:GetFiles(bool):ref:this
        -138 (-33.66% of base) : Microsoft.CodeAnalysis.CSharp.dasm - MemberRefMetadataDecoder:CustomModifiersMatch(ImmutableArray`1,ImmutableArray`1):bool
        -301 (-33.00% of base) : System.Private.Xml.dasm - <ParseNamedCharRefAsync>d__573:MoveNext():this
        -193 (-31.80% of base) : System.Management.dasm - ManagementClassGenerator:ConvertValuesToName(String):String
        -804 (-31.79% of base) : Microsoft.CodeAnalysis.CSharp.dasm - AnonymousTypeTemplateSymbol:.ctor(AnonymousTypeManager,AnonymousTypeDescriptor):this
       -1292 (-31.28% of base) : Microsoft.VisualBasic.Core.dasm - VBBinder:GetMethodsByName(Type,IReflect,String,int):ref:this
        -147 (-30.25% of base) : System.Reflection.MetadataLoadContext.dasm - PropertyPolicies:IsSuppressedByMoreDerivedMember(PropertyInfo,ref,int,int):bool:this
        -410 (-28.77% of base) : Microsoft.VisualBasic.Core.dasm - VBBinder:BindingScore(ref,ref,ref,bool,int):int:this
        -122 (-27.85% of base) : Microsoft.CodeAnalysis.dasm - MetadataHelpers:InferTypeArityFromMetadataName(String,byref):short
        -148 (-27.51% of base) : Microsoft.CodeAnalysis.dasm - ModuleExtensions:GetVTableGapSize(String):int
        -188 (-22.68% of base) : System.Diagnostics.PerformanceCounter.dasm - CategorySample:GetInstanceNamesFromIndex(int):ref:this
        -350 (-21.26% of base) : Microsoft.CodeAnalysis.CSharp.dasm - SourceNamedTypeSymbol:MakeTypeParameterConstraints(DiagnosticBag):ImmutableArray`1:this
        -256 (-20.30% of base) : System.Net.Http.dasm - HttpEnvironmentProxy:GetUriFromString(String):Uri
        -416 (-18.46% of base) : Microsoft.CodeAnalysis.CSharp.dasm - Binder:BindArrayCreationWithInitializer(DiagnosticBag,ExpressionSyntax,InitializerExpressionSyntax,ArrayTypeSymbol,ImmutableArray`1,ImmutableArray`1):BoundArrayCreation:this

4478 total methods with Code Size differences (1222 improved, 3256 regressed), 248766 unchanged.

@AndyAyersMS
Copy link
Member Author

Alpine arm64 failure in System.Text.Json.Tests is known issue: #66631.

Alpine arm64 Failure in System.Collections.Concurrent.Tests may be related...? Am going to see if it repros....

  Starting:    System.Collections.Concurrent.Tests (parallel test collections = on, max threads = 4)
./RunTests.sh: line 168:    24 Aborted                 (core dumped)

@EgorBo
Copy link
Member

EgorBo commented Mar 17, 2022

Win-arm64 improvement dotnet/perf-autofiling-issues#4143

@kunalspathak
Copy link
Member

Regression in single-min dotnet/perf-autofiling-issues#4221

@kunalspathak
Copy link
Member

Improvements in dotnet/perf-autofiling-issues#4223

@DrewScoggins
Copy link
Member

Regression in BitArrayByteArrayCtor dotnet/perf-autofiling-issues#4246

radekdoulik pushed a commit to radekdoulik/runtime that referenced this pull request Mar 30, 2022
When QuickJitForLoops=0 (current default) and the JIT sees a Tier0 method with
a loop, it will switch to optimizing the method instead.

When this happens we should also have set `BBOPT` so that the JIT will read and
incorporate PGO data for the method and/or its inliees. But we were not doing that.

This change sets `BBOPT`.
@ghost ghost locked as resolved and limited conversation to collaborators Apr 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants