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: Don't allocate string literals inside potential BBJ_THROW candidates #50112

Merged
merged 3 commits into from
Mar 26, 2021

Conversation

EgorBo
Copy link
Member

@EgorBo EgorBo commented Mar 23, 2021

Together with #48589, this PR addresses #48573 (comment)

Example:

using System;

class Prog
{
    static void Main(string[] args)
    {
        ThrowIfNull(args, nameof(args));
    }

    private static void ThrowIfNull(object arg, string argName)
    {
        if (arg is null)
            ThrowAE(argName);
    }

    private static void ThrowAE(string arg) => throw new ArgumentNullException(arg);
}

Codegen diff for Main: https://www.diffchecker.com/tZAYRdBY

As you can see, nameof(args) literal is not allocated on heap during JIT compilation, a lazy-allocate helper is emitted instead.

jit-diff (jit-utils -f --pmi):

PMI CodeSize Diffs for System.Private.CoreLib.dll, framework assemblies for  default jit

Summary of Code Size diffs:
(Lower is better)

Total bytes of base: 52999910
Total bytes of diff: 53048663
Total bytes of delta: 48753 (0.09% of base)
    diff is a regression.


Top file regressions (bytes):
       39746 : System.Collections.Immutable.dasm (3.08% of base)
        3724 : System.Collections.Concurrent.dasm (0.96% of base)
        3448 : FSharp.Core.dasm (0.09% of base)
         468 : System.Private.DataContractSerialization.dasm (0.06% of base)
         359 : System.Private.Xml.dasm (0.01% of base)
         345 : System.Net.Sockets.dasm (0.20% of base)
         244 : System.Net.Http.WinHttpHandler.dasm (0.26% of base)
         118 : Microsoft.Diagnostics.Tracing.TraceEvent.dasm (0.00% of base)
          74 : System.Data.Odbc.dasm (0.04% of base)
          59 : System.Drawing.Primitives.dasm (0.15% of base)
          50 : System.Private.CoreLib.dasm (0.00% of base)
          40 : System.Net.Http.dasm (0.01% of base)
          40 : Microsoft.Extensions.Logging.Console.dasm (0.09% of base)
          20 : System.Net.WebSockets.dasm (0.05% of base)
          18 : System.Text.RegularExpressions.dasm (0.01% of base)

15 total files with Code Size differences (0 improved, 15 regressed), 256 unchanged.

Top method regressions (bytes):
        2025 (16.17% of base) : FSharp.Core.dasm - Array2DModule:CopyTo(ref,int,int,ref,int,int,int,int) (8 methods)
         710 ( 5.09% of base) : System.Collections.Immutable.dasm - ImmutableHashSet`1:SymmetricExcept(IEnumerable`1,MutationInput):MutationResult (8 methods)
         640 (14.24% of base) : System.Collections.Immutable.dasm - ImmutableList`1:ConvertAll(Func`2):ImmutableList`1:this (64 methods)
         640 (14.24% of base) : System.Collections.Immutable.dasm - Builder:ConvertAll(Func`2):ImmutableList`1:this (64 methods)
         580 ( 7.97% of base) : System.Collections.Immutable.dasm - Node:NodeTreeFromList(IOrderedCollection`1,int,int):Node (24 methods)
         504 ( 7.04% of base) : System.Collections.Immutable.dasm - ImmutableHashSet`1:Intersect(IEnumerable`1,MutationInput):MutationResult (8 methods)
         390 ( 4.44% of base) : System.Collections.Immutable.dasm - Builder:AddRange(IEnumerable`1):this (32 methods)
         384 (25.08% of base) : System.Collections.Immutable.dasm - ImmutableSortedDictionary`2:.ctor(Node,int,IComparer`1,IEqualityComparer`1):this (8 methods)
         352 (25.79% of base) : System.Collections.Immutable.dasm - MutationInput:.ctor(SortedInt32KeyNode`1,IEqualityComparer`1,IEqualityComparer`1,int):this (8 methods)
         320 (15.62% of base) : System.Collections.Immutable.dasm - Builder:Sort(int,int,IComparer`1):this (16 methods)
         320 ( 4.75% of base) : System.Collections.Immutable.dasm - Node:CopyTo(ref,int):this (16 methods)
         320 ( 8.36% of base) : System.Collections.Immutable.dasm - Node:CopyTo(int,ref,int,int):this (8 methods)
         320 ( 3.54% of base) : System.Collections.Immutable.dasm - Node:CopyTo(Array,int):this (16 methods)
         310 ( 4.07% of base) : System.Collections.Immutable.dasm - ImmutableHashSet`1:Except(IEnumerable`1,IEqualityComparer`1,IEqualityComparer`1,SortedInt32KeyNode`1):MutationResult (8 methods)
         308 (17.94% of base) : System.Collections.Immutable.dasm - Builder:get_Origin():MutationInput:this (16 methods)
         300 ( 7.18% of base) : System.Collections.Immutable.dasm - Builder:set_KeyComparer(IEqualityComparer`1):this (16 methods)
         272 ( 8.80% of base) : System.Collections.Immutable.dasm - ImmutableSortedDictionary`2:WithComparers(IComparer`1,IEqualityComparer`1):ImmutableSortedDictionary`2:this (8 methods)
         256 (12.22% of base) : System.Collections.Immutable.dasm - ImmutableArray:Create(ref,int,int):ImmutableArray`1 (8 methods)
         256 (10.68% of base) : System.Collections.Immutable.dasm - ImmutableArray:CreateRange(ImmutableArray`1,int,int,Func`2):ImmutableArray`1 (8 methods)
         244 ( 3.75% of base) : System.Collections.Immutable.dasm - ImmutableSortedSet`1:SymmetricExcept(IEnumerable`1):ImmutableSortedSet`1:this (8 methods)

Top method regressions (percentages):
          18 (100.00% of base) : System.Collections.Concurrent.dasm - ThrowHelper:ThrowKeyNullException()
          23 (35.94% of base) : System.Private.DataContractSerialization.dasm - XmlReaderDelegator:ReadContentAsBoolean():bool:this
          23 (35.94% of base) : System.Private.DataContractSerialization.dasm - XmlReaderDelegator:ReadContentAsSingle():float:this
          23 (35.94% of base) : System.Private.DataContractSerialization.dasm - XmlReaderDelegator:ReadContentAsDouble():double:this
          23 (35.94% of base) : System.Private.DataContractSerialization.dasm - XmlReaderDelegator:ReadContentAsDecimal():Decimal:this
          23 (35.94% of base) : System.Private.DataContractSerialization.dasm - XmlReaderDelegator:ReadContentAsDateTime():DateTime:this
          23 (35.94% of base) : System.Private.DataContractSerialization.dasm - XmlReaderDelegator:ReadContentAsInt():int:this
          23 (35.94% of base) : System.Private.DataContractSerialization.dasm - XmlReaderDelegator:ReadContentAsLong():long:this
          80 (26.76% of base) : System.Collections.Immutable.dasm - ImmutableList:ToImmutableList(Builder):ImmutableList`1 (8 methods)
          80 (26.76% of base) : System.Collections.Immutable.dasm - ImmutableSortedDictionary:ToImmutableSortedDictionary(Builder):ImmutableSortedDictionary`2 (8 methods)
          80 (26.76% of base) : System.Collections.Immutable.dasm - ImmutableSortedSet:ToImmutableSortedSet(Builder):ImmutableSortedSet`1 (8 methods)
         352 (25.79% of base) : System.Collections.Immutable.dasm - MutationInput:.ctor(SortedInt32KeyNode`1,IEqualityComparer`1,IEqualityComparer`1,int):this (8 methods)
          49 (25.65% of base) : System.Drawing.Primitives.dasm - Color:FromArgb(int,int,int,int):Color
          80 (25.56% of base) : System.Collections.Immutable.dasm - ImmutableArray:ToImmutableArray(Builder):ImmutableArray`1 (8 methods)
          80 (25.40% of base) : System.Collections.Immutable.dasm - NodeEnumerable:.ctor(SortedInt32KeyNode`1):this (8 methods)
         384 (25.08% of base) : System.Collections.Immutable.dasm - ImmutableSortedDictionary`2:.ctor(Node,int,IComparer`1,IEqualityComparer`1):this (8 methods)
          80 (23.81% of base) : System.Collections.Immutable.dasm - ImmutableHashSet`1:Union(IEnumerable`1):ImmutableHashSet`1:this (8 methods)
          80 (23.81% of base) : System.Collections.Immutable.dasm - ImmutableDictionary`2:AddRange(IEnumerable`1):ImmutableDictionary`2:this (8 methods)
          80 (23.26% of base) : System.Collections.Immutable.dasm - ImmutableArrayExtensions:Any(Builder):bool (8 methods)
          80 (23.26% of base) : System.Collections.Concurrent.dasm - IDictionaryDebugView`2:.ctor(IDictionary`2):this (8 methods)

1191 total methods with Code Size differences (0 improved, 1191 regressed), 267367 unchanged.

Quite a size regression (for obvious reasons) but each regression is basically a saved GC allocation.
~ +10 bytes of machine code per each literal (in the cold sections)

PS: 67% of the regression is in System.Collections.Immutable.dasm

@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 23, 2021
@EgorBo
Copy link
Member Author

EgorBo commented Mar 23, 2021

@dotnet/jit-contrib PTAL

@AndyAyersMS
Copy link
Member

67% of the regression is in System.Collections.Immutable

Is there a pattern to the methods in this assembly that we might want to reconsider?

src/coreclr/jit/morph.cpp Outdated Show resolved Hide resolved
src/coreclr/jit/morph.cpp Outdated Show resolved Hide resolved
@EgorBo
Copy link
Member Author

EgorBo commented Mar 24, 2021

@BruceForstall you're right, I've changed the logic to just scan a top-level node of the current statement in fgMorphConst and got almost the same jit-diff with basically zero overhead for JIT.

@AndyAyersMS from my understanding, System.Collections.Immutable has a lot of generic types with multiple generic arguments so all diffs are multiplied by some big value (PMI mode)

@BruceForstall
Copy link
Member

@AndyAyersMS from my understanding, System.Collections.Immutable has a lot of generic types with multiple generic arguments so all diffs are multiplied by some big value (PMI mode)

I believe you're correct. However, have you tried crossgen/crossgen2 libraries SuperPMI asm diffs, or using "jit-diff" without "--pmi", to see if that's the case?

@EgorBo
Copy link
Member Author

EgorBo commented Mar 26, 2021

@AndyAyersMS from my understanding, System.Collections.Immutable has a lot of generic types with multiple generic arguments so all diffs are multiplied by some big value (PMI mode)

I believe you're correct. However, have you tried crossgen/crossgen2 libraries SuperPMI asm diffs, or using "jit-diff" without "--pmi", to see if that's the case?

yeah with crossgen it's 20x smaller:

Total bytes of delta: 2241 (0.01% of base)
    diff is a regression.


Top file regressions (bytes):
        1676 : System.Collections.Immutable.dasm (0.74% of base)
         201 : FSharp.Core.dasm (0.02% of base)
         121 : System.Net.Sockets.dasm (0.08% of base)
         102 : System.Collections.Concurrent.dasm (0.22% of base)
          72 : System.Net.Http.WinHttpHandler.dasm (0.11% of base)
          20 : System.Drawing.Primitives.dasm (0.06% of base)
          15 : System.Private.CoreLib.dasm (0.00% of base)
          14 : System.Net.WebSockets.dasm (0.03% of base)
           8 : System.Private.DataContractSerialization.dasm (0.00% of base)
           6 : System.Net.Http.dasm (0.00% of base)
           6 : System.Private.Xml.dasm (0.00% of base)

@EgorBo EgorBo merged commit 92b5d34 into dotnet:main Mar 26, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Apr 25, 2021
@karelz karelz added this to the 6.0.0 milestone May 20, 2021
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.

4 participants