Enable inlining of shared generics code within same type - #38229

Merged
jkotas merged 11 commits into
dotnet:masterfrom
jkotas:generics-inlining
Jun 27, 2020
Merged

Enable inlining of shared generics code within same type#38229
jkotas merged 11 commits into
dotnet:masterfrom
jkotas:generics-inlining

Conversation

@jkotas

@jkotasjkotas commented Jun 22, 2020

Copy link
Copy Markdown
Member

This change allows inlining of generic dictionary lookups when the caller and callee are the same exact type and instantiation. It is a simpler version of #31833.

Motivation for this change is to unblock #37941.

Example:

class G<T>
{
static bool M1() => typeof(T);
object M2() => new T[1];
bool M3() => Unsafe.SizeOf<T>();
static string M()
{
// Assume that T is string
// All M1, M2, M3 are inlined after this change, but were not inlined before this change
M1(); M2(); M3();
// Still not inlined - different instantiation, not the same exact type
G<List<T>>.M1();
// Still not inlined - different type.
OtherG<T>.M();
}
}

@jkotas
jkotas marked this pull request as draft June 22, 2020 19:22
@jkotas
jkotasforce-pushed the generics-inlining branch from 9645520 to dd93ffaCompareJune 23, 2020 06:58
@jaredparjaredpar mentioned this pull request Jun 23, 2020
@jkotas
jkotasforce-pushed the generics-inlining branch 3 times, most recently from 8cbb058 to 3770ae3CompareJune 24, 2020 16:20
Comment threadsrc/coreclr/src/ToolBox/superpmi/superpmi-shared/methodcontext.cpp Outdated
@jkotas
jkotasforce-pushed the generics-inlining branch from 3819aa0 to d09090eCompareJune 24, 2020 23:22
@jkotas

Copy link
Copy Markdown
MemberAuthor

Asm diffs

Total bytes of diff: 2738 (0.083% of base)
diff is a regression.
Top file regressions (bytes):
2738 : System.Private.CoreLib.dasm (0.083% of base)
1 total files with Code Size differences (0 improved, 1 regressed), 0 unchanged.
Top method regressions (bytes):
291 (41.044% of base) : System.Private.CoreLib.dasm - ValueTuple`3:System.IComparable.CompareTo(Object):int:this (3 methods)
260 (67.010% of base) : System.Private.CoreLib.dasm - ValueTuple`3:Equals(Object):bool:this (3 methods)
184 (3.340% of base) : System.Private.CoreLib.dasm - GenericArraySortHelper`1:IntroSort(Span`1,int) (12 methods)
173 (28.978% of base) : System.Private.CoreLib.dasm - ValueTask`1:ToString():String:this (3 methods)
159 (15.071% of base) : System.Private.CoreLib.dasm - Memory`1:CopyTo(Memory`1):this (4 methods)
159 (14.171% of base) : System.Private.CoreLib.dasm - Memory`1:TryCopyTo(Memory`1):bool:this (4 methods)
157 (69.778% of base) : System.Private.CoreLib.dasm - ValueTuple`4:Equals(Object):bool:this (2 methods)
118 (6.833% of base) : System.Private.CoreLib.dasm - Vector128`1:Equals(Object):bool:this (11 methods)
118 (3.562% of base) : System.Private.CoreLib.dasm - GenericArraySortHelper`1:PickPivotAndPartition(Span`1):int (12 methods)
94 (8.818% of base) : System.Private.CoreLib.dasm - ReadOnlyMemory`1:CopyTo(Memory`1):this (4 methods)
94 (8.297% of base) : System.Private.CoreLib.dasm - ReadOnlyMemory`1:TryCopyTo(Memory`1):bool:this (4 methods)
87 (13.344% of base) : System.Private.CoreLib.dasm - Memory`1:ToArray():ref:this (4 methods)
87 (14.846% of base) : System.Private.CoreLib.dasm - ReadOnlyMemory`1:ToArray():ref:this (4 methods)
76 (8.736% of base) : System.Private.CoreLib.dasm - List`1:System.Collections.IList.IndexOf(Object):int:this (8 methods)
75 (202.703% of base) : System.Private.CoreLib.dasm - List`1:Contains(KeyValuePair`2):bool:this
72 (39.560% of base) : System.Private.CoreLib.dasm - Lazy`1:CreateValue():__Canon:this
66 (129.412% of base) : System.Private.CoreLib.dasm - List`1:Remove(KeyValuePair`2):bool:this
63 (75.000% of base) : System.Private.CoreLib.dasm - TaskCompletionSource`1:.ctor(Object):this (2 methods)
62 (16.712% of base) : System.Private.CoreLib.dasm - ValueTuple`2:Equals(Object):bool:this (3 methods)
60 (8.310% of base) : System.Private.CoreLib.dasm - ValueTuple`2:System.IComparable.CompareTo(Object):int:this (3 methods)
Top method improvements (bytes):
-94 (-14.506% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Implicit(ref):ReadOnlySpan`1 (18 methods)
-73 (-8.120% of base) : System.Private.CoreLib.dasm - Memory`1:Equals(Object):bool:this (4 methods)
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector128`1:get_DisplayString():String:this
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector256`1:get_DisplayString():String:this
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector64`1:get_DisplayString():String:this
-44 (-6.790% of base) : System.Private.CoreLib.dasm - Span`1:op_Implicit(ref):Span`1 (18 methods)
-26 (-3.641% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Inequality(ReadOnlySpan`1,ReadOnlySpan`1):bool (18 methods)
-26 (-3.641% of base) : System.Private.CoreLib.dasm - Span`1:op_Inequality(Span`1,Span`1):bool (18 methods)
-15 (-0.957% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Implicit(ArraySegment`1):ReadOnlySpan`1 (18 methods)
-4 (-0.702% of base) : System.Private.CoreLib.dasm - AsyncValueTaskMethodBuilder`1:get_Task():ValueTask`1:this (2 methods)
-2 (-0.217% of base) : System.Private.CoreLib.dasm - ArraySegment`1:op_Inequality(ArraySegment`1,ArraySegment`1):bool (18 methods)
-2 (-0.930% of base) : System.Private.CoreLib.dasm - Memory`1:op_Implicit(ref):Memory`1 (4 methods)
Top method regressions (percentages):
47 (293.750% of base) : System.Private.CoreLib.dasm - SingleProducerSingleConsumerQueue`1:System.Collections.IEnumerable.GetEnumerator():IEnumerator:this
75 (202.703% of base) : System.Private.CoreLib.dasm - List`1:Contains(KeyValuePair`2):bool:this
66 (129.412% of base) : System.Private.CoreLib.dasm - List`1:Remove(KeyValuePair`2):bool:this
46 (124.324% of base) : System.Private.CoreLib.dasm - List`1:Contains(__Canon):bool:this
63 (75.000% of base) : System.Private.CoreLib.dasm - TaskCompletionSource`1:.ctor(Object):this (2 methods)
37 (72.549% of base) : System.Private.CoreLib.dasm - List`1:Remove(__Canon):bool:this
36 (72.000% of base) : System.Private.CoreLib.dasm - AsyncTaskMethodBuilder`1:SetNotificationForWaitCompletion(bool):this
59 (70.238% of base) : System.Private.CoreLib.dasm - TaskCompletionSource`1:.ctor(int):this (2 methods)
157 (69.778% of base) : System.Private.CoreLib.dasm - ValueTuple`4:Equals(Object):bool:this (2 methods)
260 (67.010% of base) : System.Private.CoreLib.dasm - ValueTuple`3:Equals(Object):bool:this (3 methods)
25 (43.103% of base) : System.Private.CoreLib.dasm - HashSet`1:System.Collections.Generic.IEnumerable<T>.GetEnumerator():IEnumerator`1:this
25 (43.103% of base) : System.Private.CoreLib.dasm - HashSet`1:System.Collections.IEnumerable.GetEnumerator():IEnumerator:this
291 (41.044% of base) : System.Private.CoreLib.dasm - ValueTuple`3:System.IComparable.CompareTo(Object):int:this (3 methods)
72 (39.560% of base) : System.Private.CoreLib.dasm - Lazy`1:CreateValue():__Canon:this
173 (28.978% of base) : System.Private.CoreLib.dasm - ValueTask`1:ToString():String:this (3 methods)
47 (27.485% of base) : System.Private.CoreLib.dasm - Lazy`1:ExecutionAndPublication(LazyHelper,bool):this
48 (19.753% of base) : System.Private.CoreLib.dasm - AsyncValueTaskMethodBuilder`1:SetException(Exception):this (2 methods)
18 (19.149% of base) : System.Private.CoreLib.dasm - Collection`1:System.Collections.IList.Contains(Object):bool:this
18 (18.947% of base) : System.Private.CoreLib.dasm - Collection`1:System.Collections.IList.IndexOf(Object):int:this
62 (16.712% of base) : System.Private.CoreLib.dasm - ValueTuple`2:Equals(Object):bool:this (3 methods)
Top method improvements (percentages):
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector128`1:get_DisplayString():String:this
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector256`1:get_DisplayString():String:this
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector64`1:get_DisplayString():String:this
-94 (-14.506% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Implicit(ref):ReadOnlySpan`1 (18 methods)
-73 (-8.120% of base) : System.Private.CoreLib.dasm - Memory`1:Equals(Object):bool:this (4 methods)
-44 (-6.790% of base) : System.Private.CoreLib.dasm - Span`1:op_Implicit(ref):Span`1 (18 methods)
-26 (-3.641% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Inequality(ReadOnlySpan`1,ReadOnlySpan`1):bool (18 methods)
-26 (-3.641% of base) : System.Private.CoreLib.dasm - Span`1:op_Inequality(Span`1,Span`1):bool (18 methods)
-15 (-0.957% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Implicit(ArraySegment`1):ReadOnlySpan`1 (18 methods)
-2 (-0.930% of base) : System.Private.CoreLib.dasm - Memory`1:op_Implicit(ref):Memory`1 (4 methods)
-4 (-0.702% of base) : System.Private.CoreLib.dasm - AsyncValueTaskMethodBuilder`1:get_Task():ValueTask`1:this (2 methods)
-2 (-0.217% of base) : System.Private.CoreLib.dasm - ArraySegment`1:op_Inequality(ArraySegment`1,ArraySegment`1):bool (18 methods)
57 total methods with Code Size differences (12 improved, 45 regressed), 20772 unchanged.

@jkotas

jkotas commented Jun 25, 2020

Copy link
Copy Markdown
MemberAuthor

Example of code-size regression - inlining tends to increase code size on average:

List`1:Contains(KeyValuePair`2):bool:this

Baseline

G_M57469_IG01:
sub rsp, 40
;; bbWeight=1 PerfScore 0.25
G_M57469_IG02:
cmp dword ptr [rcx+16], 0
je SHORT G_M57469_IG05
;; bbWeight=1 PerfScore 3.00
G_M57469_IG03:
call [List`1:IndexOf(KeyValuePair`2):int:this]
cmp eax, -1
setne al
movzx rax, al
;; bbWeight=0.50 PerfScore 2.25
G_M57469_IG04:
add rsp, 40
ret ;; bbWeight=0.50 PerfScore 0.63
G_M57469_IG05:
xor eax, eax
;; bbWeight=0.50 PerfScore 0.13
G_M57469_IG06:
add rsp, 40
ret 

Diff

G_M57469_IG01:
push rdi
push rsi
push rbx
sub rsp, 64
xor rax, rax
mov qword ptr [rsp+28H], rax
mov qword ptr [rsp+38H], rcx
mov rsi, rcx
;; bbWeight=1 PerfScore 5.75
G_M57469_IG02:
cmp dword ptr [rsi+16], 0
je SHORT G_M57469_IG05
;; bbWeight=1 PerfScore 3.00
G_M57469_IG03:
mov rdi, gword ptr [rdx]
mov ebx, dword ptr [rdx+8]
mov rcx, qword ptr [rsi]
call [CORINFO_HELP_READYTORUN_GENERIC_HANDLE]
mov rcx, rax
mov rdx, gword ptr [rsi+8]
lea r8, bword ptr [rsp+28H]
mov gword ptr [r8], rdi
mov dword ptr [r8+8], ebx
mov r8d, dword ptr [rsi+16]
mov dword ptr [rsp+20H], r8d
lea r8, bword ptr [rsp+28H]
xor r9d, r9d
call [Array:IndexOf(ref,KeyValuePair`2,int,int):int]
cmp eax, -1
setne al
movzx rax, al
;; bbWeight=0.50 PerfScore 11.00
G_M57469_IG04:
add rsp, 64
pop rbx
pop rsi
pop rdi
ret ;; bbWeight=0.50 PerfScore 1.38
G_M57469_IG05:
xor eax, eax
;; bbWeight=0.50 PerfScore 0.13
G_M57469_IG06:
add rsp, 64
pop rbx
pop rsi
pop rdi
ret 

@jkotas

Copy link
Copy Markdown
MemberAuthor

Example of code-size improvement - inlining reduces code size when it enables additional constant-prop:

Vector128`1:get_DisplayString

Baseline:

G_M30183_IG01:
push rdi
push rsi
sub rsp, 40
mov qword ptr [rsp+20H], rdx
mov rdi, rcx
mov rsi, rdx
;; bbWeight=1 PerfScore 3.75
G_M30183_IG02:
mov rcx, rsi
call [CORINFO_HELP_READYTORUN_GENERIC_HANDLE]
mov rcx, rax
call [Vector128`1:get_IsSupported():bool]
test al, al
je SHORT G_M30183_IG05
;; bbWeight=1 PerfScore 7.75
G_M30183_IG03:
mov rcx, rsi
call [CORINFO_HELP_READYTORUN_GENERIC_HANDLE]
mov rcx, rdi
call rax
nop ;; bbWeight=0.50 PerfScore 3.38
G_M30183_IG04:
add rsp, 40
pop rsi
pop rdi
ret ;; bbWeight=0.50 PerfScore 1.13
G_M30183_IG05:
mov rcx, qword ptr [(reloc)]
mov rcx, gword ptr [rcx]
mov rdx, qword ptr [(reloc)]
mov rdx, gword ptr [rdx]
call [SR:GetResourceString(String,String):String]
nop ;; bbWeight=0.50 PerfScore 5.63
G_M30183_IG06:
add rsp, 40
pop rsi
pop rdi
ret ;; bbWeight=0.50 PerfScore 1.13

Diff:

G_M30183_IG01:
sub rsp, 40
;; bbWeight=1 PerfScore 0.25
G_M30183_IG02:
mov rcx, qword ptr [(reloc)]
mov rcx, gword ptr [rcx]
mov rdx, qword ptr [(reloc)]
mov rdx, gword ptr [rdx]
call [SR:GetResourceString(String,String):String]
nop ;; bbWeight=1 PerfScore 11.25
G_M30183_IG03:
add rsp, 40
ret 

@nietras

Copy link
Copy Markdown
Contributor

The example improvements are exactly the kind I am hoping for 👍

@jkotas
jkotas marked this pull request as ready for review June 26, 2020 05:52
@jkotas

Copy link
Copy Markdown
MemberAuthor

@dotnet/crossgen-contrib Any crossgen2 specific legs you would recommend to run on this?

@dotnet/jit-contrib Any JIT specific legs you would recommend to run on this?

Comment threadsrc/coreclr/src/tools/Common/Sorting/MergeSortCore.cs
Comment threadsrc/coreclr/src/vm/jitinterface.cpp
@jkotasjkotas changed the title WIP: Enable inlining of shared generics code within same typeEnable inlining of shared generics code within same typeJun 26, 2020

@MichalStrehovskyMichalStrehovsky left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Crossgen2 side looks good to me.

I echo Manish's question about the assert - I think that code might be reachable.

Not commenting on the parallel sort thing because I didn't look at that code before.

@mangod9

Copy link
Copy Markdown
Member

@dotnet/crossgen-contrib Any crossgen2 specific legs you would recommend to run on this?

@dotnet/jit-contrib Any JIT specific legs you would recommend to run on this?

for crossgen2 would be good to run the runtime-coreclr crossgen2 pipeline for validation.

@jkotas

Copy link
Copy Markdown
MemberAuthor

/azp run runtime-coreclr crossgen2

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

@jkotas

Copy link
Copy Markdown
MemberAuthor

for crossgen2 would be good to run the runtime-coreclr crossgen2 pipeline for validation.

@mangod9 What is the right way to trigger it? I have tried to trigger it above, but it does not seem to work.

@mangod9

Copy link
Copy Markdown
Member

for crossgen2 would be good to run the runtime-coreclr crossgen2 pipeline for validation.

@mangod9 What is the right way to trigger it? I have tried to trigger it above, but it does not seem to work.

Hmm.. not sure why that pipeline command fails (probably some config for the pipeline @trylek ?). For now I have manually triggered it via AzDo portal

@BruceForstall

Copy link
Copy Markdown
Contributor

I thought (almost) no triggers were enabled, due to issues with accidentally triggering too many builds. Also, #32777

@davidwrighton

Copy link
Copy Markdown
Member

@jkotas, you can no longer use any of the pipelines from the AzDo command line. You need to find the pipeline in AzDo and manually start a build.

Comment threadsrc/coreclr/src/tools/Common/Sorting/MergeSortCore.cs

@AndyAyersMSAndyAyersMS left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Were those diffs from crossgen or PMI?

Might want to run a wider set of diffs (pass -f to jit-diff) and/or look diffs from both prejitting and jitting.

Comment threadsrc/coreclr/src/jit/importer.cpp
Comment threadsrc/coreclr/src/jit/importer.cpp

@davidwrightondavidwrighton left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like you to split out the work to address the static field address issue into a separate PR, and to provide a better description of under what conditions an inlined shared generic lookup will work and where it won't work.

@jkotasjkotas mentioned this pull request Jun 26, 2020
@jkotas

Copy link
Copy Markdown
MemberAuthor

I'd like you to split out the work to address the static field address issue into a separate PR, and to provide a better description of under what conditions an inlined shared generic lookup will work and where it won't work.

@davidwrighton Done. The updated description is at the top.

@BruceForstall

Copy link
Copy Markdown
Contributor

@dotnet/jit-contrib Any JIT specific legs you would recommend to run on this?

Seems like it would be worthwhile to runtime-coreclr jitstress (https://dev.azure.com/dnceng/public/_build?definitionId=658) to run Pri-1 tests with JitStress=1/2, which would include inlining stress.

@jkotas

Copy link
Copy Markdown
MemberAuthor

Might want to run a wider set of diffs

-f crossgen diffs:

Found 25 files with textual diffs.
Summary of Code Size diffs:
(Lower is better)
Total bytes of diff: 28141 (0.086% of base)
diff is a regression.
Total byte diff includes -111 bytes from reconciling methods
Base had 2 unique methods, 111 unique bytes
Diff had 0 unique methods, 0 unique bytes
Top file regressions (bytes):
12800 : System.Collections.Immutable.dasm (5.998% of base)
5146 : System.Memory.dasm (6.358% of base)
2738 : System.Private.CoreLib.dasm (0.083% of base)
2735 : System.Collections.dasm (2.930% of base)
714 : System.Collections.Concurrent.dasm (1.559% of base)
587 : Microsoft.CodeAnalysis.dasm (0.078% of base)
473 : System.Reflection.Metadata.dasm (0.147% of base)
468 : System.Linq.dasm (0.353% of base)
298 : System.Threading.Tasks.Dataflow.dasm (0.217% of base)
282 : System.Linq.Expressions.dasm (0.010% of base)
266 : System.ObjectModel.dasm (1.353% of base)
202 : Microsoft.CSharp.dasm (0.062% of base)
179 : System.DirectoryServices.AccountManagement.dasm (0.072% of base)
171 : Microsoft.Diagnostics.Tracing.TraceEvent.dasm (0.005% of base)
153 : Newtonsoft.Json.dasm (0.025% of base)
138 : Microsoft.Extensions.Logging.Abstractions.dasm (0.565% of base)
135 : Microsoft.Diagnostics.FastSerialization.dasm (0.401% of base)
126 : System.Private.Xml.dasm (0.004% of base)
116 : System.Linq.Parallel.dasm (0.020% of base)
108 : Microsoft.CodeAnalysis.CSharp.dasm (0.005% of base)
25 total files with Code Size differences (0 improved, 25 regressed), 239 unchanged.
Top method regressions (bytes):
554 (45.975% of base) : System.Memory.dasm - ReadOnlySequence`1:TryGet(byref,byref,bool):bool:this (3 methods)
543 (35.560% of base) : System.Memory.dasm - SequenceReader`1:TryReadTo(byref,__Canon,__Canon,bool):bool:this (2 methods)
437 (108.168% of base) : System.Collections.Immutable.dasm - Builder:System.Collections.IEnumerable.GetEnumerator():IEnumerator:this (8 methods)
422 (28.964% of base) : System.Memory.dasm - SequenceReader`1:TryReadToSlow(byref,__Canon,__Canon,int,bool):bool:this (2 methods)
347 (10.534% of base) : System.Collections.Immutable.dasm - Enumerator:MoveNext():bool:this (15 methods)
344 (148.276% of base) : System.Collections.Immutable.dasm - Node:System.Collections.IEnumerable.GetEnumerator():IEnumerator:this (4 methods)
336 (13.925% of base) : System.Memory.dasm - ReadOnlySequence`1:Slice(long,long):ReadOnlySequence`1:this (3 methods)
330 (23.827% of base) : System.Collections.Immutable.dasm - Enumerator:Reset():this (11 methods)
323 (21.824% of base) : System.Collections.Immutable.dasm - Builder:AddRange(IEnumerable`1):this (6 methods)
291 (31.699% of base) : System.Memory.dasm - ReadOnlySequence`1:Slice(long):ReadOnlySequence`1:this (3 methods)
291 (41.044% of base) : System.Private.CoreLib.dasm - ValueTuple`3:System.IComparable.CompareTo(Object):int:this (3 methods)
287 (38.013% of base) : System.Reflection.Metadata.dasm - SignatureDecoder`2:DecodeType(byref,bool,int):__Canon:this
286 (54.476% of base) : System.Memory.dasm - ReadOnlySequence`1:get_FirstSpan():ReadOnlySpan`1:this (3 methods)
281 (19.957% of base) : System.Memory.dasm - SequenceReader`1:TryReadTo(byref,ReadOnlySpan`1,bool):bool:this (2 methods)
275 (158.046% of base) : System.Collections.Immutable.dasm - Node:System.Collections.Generic.IEnumerable<T>.GetEnumerator():IEnumerator`1:this (3 methods)
268 (52.652% of base) : System.Memory.dasm - ReadOnlySequence`1:get_First():ReadOnlyMemory`1:this (3 methods)
268 (41.358% of base) : System.Memory.dasm - ReadOnlySequence`1:GetPosition(long,SequencePosition):SequencePosition:this (3 methods)
267 (41.654% of base) : System.Memory.dasm - ReadOnlySequence`1:GetPosition(long):SequencePosition:this (3 methods)
266 (95.341% of base) : System.Collections.Immutable.dasm - Builder:System.Collections.Generic.IEnumerable<T>.GetEnumerator():IEnumerator`1:this (6 methods)
265 (24.953% of base) : System.Collections.Immutable.dasm - Node:CopyTo(ref,int):this (3 methods)
Top method improvements (bytes):
-169 (-26.365% of base) : System.Collections.Immutable.dasm - ImmutableArray`1:InsertRange(int,ImmutableArray`1):ImmutableArray`1:this (2 methods)
-145 (-27.672% of base) : System.Collections.Immutable.dasm - Node:GetEnumerator():Enumerator:this (4 base, 3 diff methods)
-94 (-14.506% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Implicit(ref):ReadOnlySpan`1 (18 methods)
-82 (-100.000% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:CreatePair(__Canon,Pair`2):Pair`2 (1 base, 0 diff methods)
-73 (-8.120% of base) : System.Private.CoreLib.dasm - Memory`1:Equals(Object):bool:this (4 methods)
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector128`1:get_DisplayString():String:this
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector256`1:get_DisplayString():String:this
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector64`1:get_DisplayString():String:this
-56 (-29.474% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:Add(__Canon,int):bool:this
-52 (-27.083% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:Add(__Canon,__Canon):bool:this
-44 (-6.790% of base) : System.Private.CoreLib.dasm - Span`1:op_Implicit(ref):Span`1 (18 methods)
-42 (-7.420% of base) : System.Collections.Immutable.dasm - ImmutableArray`1:RemoveAll(Predicate`1):ImmutableArray`1:this (2 methods)
-32 (-46.377% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:.ctor(__Canon,int):this
-29 (-100.000% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:CreatePair(__Canon,int):Pair`2 (1 base, 0 diff methods)
-26 (-3.641% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Inequality(ReadOnlySpan`1,ReadOnlySpan`1):bool (18 methods)
-26 (-3.641% of base) : System.Private.CoreLib.dasm - Span`1:op_Inequality(Span`1,Span`1):bool (18 methods)
-22 (-31.884% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:.ctor(__Canon,__Canon):this
-21 (-19.444% of base) : System.Linq.Parallel.dasm - OrderedGroupJoinHashLookup:CreateValuePair(GroupKeyData):Pair`2:this
-19 (-11.515% of base) : Microsoft.CodeAnalysis.CSharp.dasm - SyntaxList`1:Any(ushort):bool:this
-19 (-13.475% of base) : System.Linq.dasm - SparseArrayBuilder`1:Reserve(int):this
Top method regressions (percentages):
126 (787.500% of base) : System.DirectoryServices.AccountManagement.dasm - TrackedCollection`1:System.Collections.IEnumerable.GetEnumerator():IEnumerator:this
92 (575.000% of base) : System.Collections.Immutable.dasm - ImmutableList`1:System.Collections.Immutable.IImmutableList<T>.Remove(KeyValuePair`2,IEqualityComparer`1):IImmutableList`1:this
63 (393.750% of base) : System.Collections.dasm - SortedList`2:get_Keys():IList`1:this
63 (393.750% of base) : System.Collections.dasm - SortedList`2:System.Collections.Generic.IDictionary<TKey,TValue>.get_Keys():ICollection`1:this
63 (393.750% of base) : System.Collections.dasm - SortedList`2:System.Collections.IDictionary.get_Keys():ICollection:this
63 (393.750% of base) : System.Collections.dasm - SortedList`2:System.Collections.Generic.IReadOnlyDictionary<TKey,TValue>.get_Keys():IEnumerable`1:this
63 (393.750% of base) : System.Collections.dasm - SortedList`2:get_Values():IList`1:this
63 (393.750% of base) : System.Collections.dasm - SortedList`2:System.Collections.Generic.IDictionary<TKey,TValue>.get_Values():ICollection`1:this
63 (393.750% of base) : System.Collections.dasm - SortedList`2:System.Collections.IDictionary.get_Values():ICollection:this
63 (393.750% of base) : System.Collections.dasm - SortedList`2:System.Collections.Generic.IReadOnlyDictionary<TKey,TValue>.get_Values():IEnumerable`1:this
62 (387.500% of base) : System.Collections.dasm - SortedDictionary`2:System.Collections.Generic.IDictionary<TKey,TValue>.get_Keys():ICollection`1:this
62 (387.500% of base) : System.Collections.dasm - SortedDictionary`2:System.Collections.Generic.IReadOnlyDictionary<TKey,TValue>.get_Keys():IEnumerable`1:this
62 (387.500% of base) : System.Collections.dasm - SortedDictionary`2:System.Collections.Generic.IDictionary<TKey,TValue>.get_Values():ICollection`1:this
62 (387.500% of base) : System.Collections.dasm - SortedDictionary`2:System.Collections.Generic.IReadOnlyDictionary<TKey,TValue>.get_Values():IEnumerable`1:this
62 (387.500% of base) : System.Collections.dasm - SortedDictionary`2:System.Collections.IDictionary.get_Keys():ICollection:this
62 (387.500% of base) : System.Collections.dasm - SortedDictionary`2:System.Collections.IDictionary.get_Values():ICollection:this
61 (381.250% of base) : Newtonsoft.Json.dasm - EnumerableDictionaryWrapper`2:System.Collections.IEnumerable.GetEnumerator():IEnumerator:this
61 (381.250% of base) : System.Net.Http.dasm - HttpHeaderValueCollection`1:System.Collections.IEnumerable.GetEnumerator():IEnumerator:this
60 (375.000% of base) : System.Collections.Immutable.dasm - ImmutableList`1:System.Collections.Immutable.IImmutableList<T>.Remove(__Canon,IEqualityComparer`1):IImmutableList`1:this
60 (375.000% of base) : System.Collections.Immutable.dasm - ImmutableStack`1:System.Collections.Immutable.IImmutableStack<T>.Push(__Canon):IImmutableStack`1:this
Top method improvements (percentages):
-82 (-100.000% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:CreatePair(__Canon,Pair`2):Pair`2 (1 base, 0 diff methods)
-29 (-100.000% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:CreatePair(__Canon,int):Pair`2 (1 base, 0 diff methods)
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector128`1:get_DisplayString():String:this
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector256`1:get_DisplayString():String:this
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector64`1:get_DisplayString():String:this
-32 (-46.377% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:.ctor(__Canon,int):this
-22 (-31.884% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:.ctor(__Canon,__Canon):this
-56 (-29.474% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:Add(__Canon,int):bool:this
-145 (-27.672% of base) : System.Collections.Immutable.dasm - Node:GetEnumerator():Enumerator:this (4 base, 3 diff methods)
-52 (-27.083% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:Add(__Canon,__Canon):bool:this
-169 (-26.365% of base) : System.Collections.Immutable.dasm - ImmutableArray`1:InsertRange(int,ImmutableArray`1):ImmutableArray`1:this (2 methods)
-21 (-19.444% of base) : System.Linq.Parallel.dasm - OrderedGroupJoinHashLookup:CreateValuePair(GroupKeyData):Pair`2:this
-94 (-14.506% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Implicit(ref):ReadOnlySpan`1 (18 methods)
-19 (-13.475% of base) : System.Linq.dasm - SparseArrayBuilder`1:Reserve(int):this
-18 (-12.950% of base) : System.Collections.Immutable.dasm - ImmutableDictionary`2:TryGetValue(__Canon,byref):bool:this
-18 (-12.950% of base) : System.Collections.Immutable.dasm - ImmutableDictionary`2:TryGetKey(__Canon,byref):bool:this
-9 (-12.857% of base) : Microsoft.CodeAnalysis.VisualBasic.dasm - _Closure$__17`1:.cctor()
-13 (-12.745% of base) : System.Collections.Immutable.dasm - ImmutableDictionary`2:Contains(KeyValuePair`2):bool:this
-19 (-11.515% of base) : Microsoft.CodeAnalysis.CSharp.dasm - SyntaxList`1:Any(ushort):bool:this
-13 (-10.484% of base) : System.Collections.Immutable.dasm - ImmutableDictionary`2:ContainsKey(__Canon):bool:this
406 total methods with Code Size differences (43 improved, 363 regressed), 186975 unchanged.

@jkotas

Copy link
Copy Markdown
MemberAuthor
  • ReadOnlySequence size regressions is caused by AggresiveInlining on ~70 lines method
  • ImmutableCollections size regression is a lots of small methods that are getting inlined into each other now

@jkotas

Copy link
Copy Markdown
MemberAuthor

JIT stress failure is known issue #36681
Installer Build and Test coreclr failure is known issue #35068
oreCLR Pri0 Runtime Tests Run Windows_NT x86 checked is known issue #38284

@jkotas
jkotas merged commit 31699de into dotnet:masterJun 27, 2020
@jkotas
jkotas deleted the generics-inlining branch June 27, 2020 13:36
@nietras

Copy link
Copy Markdown
Contributor

Great to see this merged. 👍

when the caller and callee are the same exact type and instantiation.

This is quite clear but to be sure, I assume this means this won't work for base class methods for example? E. g. called in a derived class?

@jkotas

Copy link
Copy Markdown
MemberAuthor

I assume this means this won't work for base class methods for example? E. g. called in a derived class?

Correct.

@ghostghost locked as resolved and limited conversation to collaborators Dec 8, 2020
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

11 participants

@jkotas@nietras@mangod9@BruceForstall@davidwrighton@EgorBo@stephentoub@jakobbotsch@AndyAyersMS@MichalStrehovsky@Dotnet-GitSync-Bot
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

Enable inlining of shared generics code within same type - #38229

Merged
jkotas merged 11 commits into
dotnet:masterfrom
jkotas:generics-inlining
Jun 27, 2020
Merged

Enable inlining of shared generics code within same type#38229
jkotas merged 11 commits into
dotnet:masterfrom
jkotas:generics-inlining

Conversation

@jkotas

@jkotasjkotas commented Jun 22, 2020

Copy link
Copy Markdown
Member

This change allows inlining of generic dictionary lookups when the caller and callee are the same exact type and instantiation. It is a simpler version of #31833.

Motivation for this change is to unblock #37941.

Example:

class G<T>
{
static bool M1() => typeof(T);
object M2() => new T[1];
bool M3() => Unsafe.SizeOf<T>();
static string M()
{
// Assume that T is string
// All M1, M2, M3 are inlined after this change, but were not inlined before this change
M1(); M2(); M3();
// Still not inlined - different instantiation, not the same exact type
G<List<T>>.M1();
// Still not inlined - different type.
OtherG<T>.M();
}
}

@jkotas
jkotas marked this pull request as draft June 22, 2020 19:22
@jkotas
jkotasforce-pushed the generics-inlining branch from 9645520 to dd93ffaCompareJune 23, 2020 06:58
@jaredparjaredpar mentioned this pull request Jun 23, 2020
@jkotas
jkotasforce-pushed the generics-inlining branch 3 times, most recently from 8cbb058 to 3770ae3CompareJune 24, 2020 16:20
Comment threadsrc/coreclr/src/ToolBox/superpmi/superpmi-shared/methodcontext.cpp Outdated
@jkotas
jkotasforce-pushed the generics-inlining branch from 3819aa0 to d09090eCompareJune 24, 2020 23:22
@jkotas

Copy link
Copy Markdown
MemberAuthor

Asm diffs

Total bytes of diff: 2738 (0.083% of base)
diff is a regression.
Top file regressions (bytes):
2738 : System.Private.CoreLib.dasm (0.083% of base)
1 total files with Code Size differences (0 improved, 1 regressed), 0 unchanged.
Top method regressions (bytes):
291 (41.044% of base) : System.Private.CoreLib.dasm - ValueTuple`3:System.IComparable.CompareTo(Object):int:this (3 methods)
260 (67.010% of base) : System.Private.CoreLib.dasm - ValueTuple`3:Equals(Object):bool:this (3 methods)
184 (3.340% of base) : System.Private.CoreLib.dasm - GenericArraySortHelper`1:IntroSort(Span`1,int) (12 methods)
173 (28.978% of base) : System.Private.CoreLib.dasm - ValueTask`1:ToString():String:this (3 methods)
159 (15.071% of base) : System.Private.CoreLib.dasm - Memory`1:CopyTo(Memory`1):this (4 methods)
159 (14.171% of base) : System.Private.CoreLib.dasm - Memory`1:TryCopyTo(Memory`1):bool:this (4 methods)
157 (69.778% of base) : System.Private.CoreLib.dasm - ValueTuple`4:Equals(Object):bool:this (2 methods)
118 (6.833% of base) : System.Private.CoreLib.dasm - Vector128`1:Equals(Object):bool:this (11 methods)
118 (3.562% of base) : System.Private.CoreLib.dasm - GenericArraySortHelper`1:PickPivotAndPartition(Span`1):int (12 methods)
94 (8.818% of base) : System.Private.CoreLib.dasm - ReadOnlyMemory`1:CopyTo(Memory`1):this (4 methods)
94 (8.297% of base) : System.Private.CoreLib.dasm - ReadOnlyMemory`1:TryCopyTo(Memory`1):bool:this (4 methods)
87 (13.344% of base) : System.Private.CoreLib.dasm - Memory`1:ToArray():ref:this (4 methods)
87 (14.846% of base) : System.Private.CoreLib.dasm - ReadOnlyMemory`1:ToArray():ref:this (4 methods)
76 (8.736% of base) : System.Private.CoreLib.dasm - List`1:System.Collections.IList.IndexOf(Object):int:this (8 methods)
75 (202.703% of base) : System.Private.CoreLib.dasm - List`1:Contains(KeyValuePair`2):bool:this
72 (39.560% of base) : System.Private.CoreLib.dasm - Lazy`1:CreateValue():__Canon:this
66 (129.412% of base) : System.Private.CoreLib.dasm - List`1:Remove(KeyValuePair`2):bool:this
63 (75.000% of base) : System.Private.CoreLib.dasm - TaskCompletionSource`1:.ctor(Object):this (2 methods)
62 (16.712% of base) : System.Private.CoreLib.dasm - ValueTuple`2:Equals(Object):bool:this (3 methods)
60 (8.310% of base) : System.Private.CoreLib.dasm - ValueTuple`2:System.IComparable.CompareTo(Object):int:this (3 methods)
Top method improvements (bytes):
-94 (-14.506% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Implicit(ref):ReadOnlySpan`1 (18 methods)
-73 (-8.120% of base) : System.Private.CoreLib.dasm - Memory`1:Equals(Object):bool:this (4 methods)
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector128`1:get_DisplayString():String:this
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector256`1:get_DisplayString():String:this
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector64`1:get_DisplayString():String:this
-44 (-6.790% of base) : System.Private.CoreLib.dasm - Span`1:op_Implicit(ref):Span`1 (18 methods)
-26 (-3.641% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Inequality(ReadOnlySpan`1,ReadOnlySpan`1):bool (18 methods)
-26 (-3.641% of base) : System.Private.CoreLib.dasm - Span`1:op_Inequality(Span`1,Span`1):bool (18 methods)
-15 (-0.957% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Implicit(ArraySegment`1):ReadOnlySpan`1 (18 methods)
-4 (-0.702% of base) : System.Private.CoreLib.dasm - AsyncValueTaskMethodBuilder`1:get_Task():ValueTask`1:this (2 methods)
-2 (-0.217% of base) : System.Private.CoreLib.dasm - ArraySegment`1:op_Inequality(ArraySegment`1,ArraySegment`1):bool (18 methods)
-2 (-0.930% of base) : System.Private.CoreLib.dasm - Memory`1:op_Implicit(ref):Memory`1 (4 methods)
Top method regressions (percentages):
47 (293.750% of base) : System.Private.CoreLib.dasm - SingleProducerSingleConsumerQueue`1:System.Collections.IEnumerable.GetEnumerator():IEnumerator:this
75 (202.703% of base) : System.Private.CoreLib.dasm - List`1:Contains(KeyValuePair`2):bool:this
66 (129.412% of base) : System.Private.CoreLib.dasm - List`1:Remove(KeyValuePair`2):bool:this
46 (124.324% of base) : System.Private.CoreLib.dasm - List`1:Contains(__Canon):bool:this
63 (75.000% of base) : System.Private.CoreLib.dasm - TaskCompletionSource`1:.ctor(Object):this (2 methods)
37 (72.549% of base) : System.Private.CoreLib.dasm - List`1:Remove(__Canon):bool:this
36 (72.000% of base) : System.Private.CoreLib.dasm - AsyncTaskMethodBuilder`1:SetNotificationForWaitCompletion(bool):this
59 (70.238% of base) : System.Private.CoreLib.dasm - TaskCompletionSource`1:.ctor(int):this (2 methods)
157 (69.778% of base) : System.Private.CoreLib.dasm - ValueTuple`4:Equals(Object):bool:this (2 methods)
260 (67.010% of base) : System.Private.CoreLib.dasm - ValueTuple`3:Equals(Object):bool:this (3 methods)
25 (43.103% of base) : System.Private.CoreLib.dasm - HashSet`1:System.Collections.Generic.IEnumerable<T>.GetEnumerator():IEnumerator`1:this
25 (43.103% of base) : System.Private.CoreLib.dasm - HashSet`1:System.Collections.IEnumerable.GetEnumerator():IEnumerator:this
291 (41.044% of base) : System.Private.CoreLib.dasm - ValueTuple`3:System.IComparable.CompareTo(Object):int:this (3 methods)
72 (39.560% of base) : System.Private.CoreLib.dasm - Lazy`1:CreateValue():__Canon:this
173 (28.978% of base) : System.Private.CoreLib.dasm - ValueTask`1:ToString():String:this (3 methods)
47 (27.485% of base) : System.Private.CoreLib.dasm - Lazy`1:ExecutionAndPublication(LazyHelper,bool):this
48 (19.753% of base) : System.Private.CoreLib.dasm - AsyncValueTaskMethodBuilder`1:SetException(Exception):this (2 methods)
18 (19.149% of base) : System.Private.CoreLib.dasm - Collection`1:System.Collections.IList.Contains(Object):bool:this
18 (18.947% of base) : System.Private.CoreLib.dasm - Collection`1:System.Collections.IList.IndexOf(Object):int:this
62 (16.712% of base) : System.Private.CoreLib.dasm - ValueTuple`2:Equals(Object):bool:this (3 methods)
Top method improvements (percentages):
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector128`1:get_DisplayString():String:this
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector256`1:get_DisplayString():String:this
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector64`1:get_DisplayString():String:this
-94 (-14.506% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Implicit(ref):ReadOnlySpan`1 (18 methods)
-73 (-8.120% of base) : System.Private.CoreLib.dasm - Memory`1:Equals(Object):bool:this (4 methods)
-44 (-6.790% of base) : System.Private.CoreLib.dasm - Span`1:op_Implicit(ref):Span`1 (18 methods)
-26 (-3.641% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Inequality(ReadOnlySpan`1,ReadOnlySpan`1):bool (18 methods)
-26 (-3.641% of base) : System.Private.CoreLib.dasm - Span`1:op_Inequality(Span`1,Span`1):bool (18 methods)
-15 (-0.957% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Implicit(ArraySegment`1):ReadOnlySpan`1 (18 methods)
-2 (-0.930% of base) : System.Private.CoreLib.dasm - Memory`1:op_Implicit(ref):Memory`1 (4 methods)
-4 (-0.702% of base) : System.Private.CoreLib.dasm - AsyncValueTaskMethodBuilder`1:get_Task():ValueTask`1:this (2 methods)
-2 (-0.217% of base) : System.Private.CoreLib.dasm - ArraySegment`1:op_Inequality(ArraySegment`1,ArraySegment`1):bool (18 methods)
57 total methods with Code Size differences (12 improved, 45 regressed), 20772 unchanged.

@jkotas

jkotas commented Jun 25, 2020

Copy link
Copy Markdown
MemberAuthor

Example of code-size regression - inlining tends to increase code size on average:

List`1:Contains(KeyValuePair`2):bool:this

Baseline

G_M57469_IG01:
sub rsp, 40
;; bbWeight=1 PerfScore 0.25
G_M57469_IG02:
cmp dword ptr [rcx+16], 0
je SHORT G_M57469_IG05
;; bbWeight=1 PerfScore 3.00
G_M57469_IG03:
call [List`1:IndexOf(KeyValuePair`2):int:this]
cmp eax, -1
setne al
movzx rax, al
;; bbWeight=0.50 PerfScore 2.25
G_M57469_IG04:
add rsp, 40
ret ;; bbWeight=0.50 PerfScore 0.63
G_M57469_IG05:
xor eax, eax
;; bbWeight=0.50 PerfScore 0.13
G_M57469_IG06:
add rsp, 40
ret 

Diff

G_M57469_IG01:
push rdi
push rsi
push rbx
sub rsp, 64
xor rax, rax
mov qword ptr [rsp+28H], rax
mov qword ptr [rsp+38H], rcx
mov rsi, rcx
;; bbWeight=1 PerfScore 5.75
G_M57469_IG02:
cmp dword ptr [rsi+16], 0
je SHORT G_M57469_IG05
;; bbWeight=1 PerfScore 3.00
G_M57469_IG03:
mov rdi, gword ptr [rdx]
mov ebx, dword ptr [rdx+8]
mov rcx, qword ptr [rsi]
call [CORINFO_HELP_READYTORUN_GENERIC_HANDLE]
mov rcx, rax
mov rdx, gword ptr [rsi+8]
lea r8, bword ptr [rsp+28H]
mov gword ptr [r8], rdi
mov dword ptr [r8+8], ebx
mov r8d, dword ptr [rsi+16]
mov dword ptr [rsp+20H], r8d
lea r8, bword ptr [rsp+28H]
xor r9d, r9d
call [Array:IndexOf(ref,KeyValuePair`2,int,int):int]
cmp eax, -1
setne al
movzx rax, al
;; bbWeight=0.50 PerfScore 11.00
G_M57469_IG04:
add rsp, 64
pop rbx
pop rsi
pop rdi
ret ;; bbWeight=0.50 PerfScore 1.38
G_M57469_IG05:
xor eax, eax
;; bbWeight=0.50 PerfScore 0.13
G_M57469_IG06:
add rsp, 64
pop rbx
pop rsi
pop rdi
ret 

@jkotas

Copy link
Copy Markdown
MemberAuthor

Example of code-size improvement - inlining reduces code size when it enables additional constant-prop:

Vector128`1:get_DisplayString

Baseline:

G_M30183_IG01:
push rdi
push rsi
sub rsp, 40
mov qword ptr [rsp+20H], rdx
mov rdi, rcx
mov rsi, rdx
;; bbWeight=1 PerfScore 3.75
G_M30183_IG02:
mov rcx, rsi
call [CORINFO_HELP_READYTORUN_GENERIC_HANDLE]
mov rcx, rax
call [Vector128`1:get_IsSupported():bool]
test al, al
je SHORT G_M30183_IG05
;; bbWeight=1 PerfScore 7.75
G_M30183_IG03:
mov rcx, rsi
call [CORINFO_HELP_READYTORUN_GENERIC_HANDLE]
mov rcx, rdi
call rax
nop ;; bbWeight=0.50 PerfScore 3.38
G_M30183_IG04:
add rsp, 40
pop rsi
pop rdi
ret ;; bbWeight=0.50 PerfScore 1.13
G_M30183_IG05:
mov rcx, qword ptr [(reloc)]
mov rcx, gword ptr [rcx]
mov rdx, qword ptr [(reloc)]
mov rdx, gword ptr [rdx]
call [SR:GetResourceString(String,String):String]
nop ;; bbWeight=0.50 PerfScore 5.63
G_M30183_IG06:
add rsp, 40
pop rsi
pop rdi
ret ;; bbWeight=0.50 PerfScore 1.13

Diff:

G_M30183_IG01:
sub rsp, 40
;; bbWeight=1 PerfScore 0.25
G_M30183_IG02:
mov rcx, qword ptr [(reloc)]
mov rcx, gword ptr [rcx]
mov rdx, qword ptr [(reloc)]
mov rdx, gword ptr [rdx]
call [SR:GetResourceString(String,String):String]
nop ;; bbWeight=1 PerfScore 11.25
G_M30183_IG03:
add rsp, 40
ret 

@nietras

Copy link
Copy Markdown
Contributor

The example improvements are exactly the kind I am hoping for 👍

@jkotas
jkotas marked this pull request as ready for review June 26, 2020 05:52
@jkotas

Copy link
Copy Markdown
MemberAuthor

@dotnet/crossgen-contrib Any crossgen2 specific legs you would recommend to run on this?

@dotnet/jit-contrib Any JIT specific legs you would recommend to run on this?

Comment threadsrc/coreclr/src/tools/Common/Sorting/MergeSortCore.cs
Comment threadsrc/coreclr/src/vm/jitinterface.cpp
@jkotasjkotas changed the title WIP: Enable inlining of shared generics code within same typeEnable inlining of shared generics code within same typeJun 26, 2020

@MichalStrehovskyMichalStrehovsky left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Crossgen2 side looks good to me.

I echo Manish's question about the assert - I think that code might be reachable.

Not commenting on the parallel sort thing because I didn't look at that code before.

@mangod9

Copy link
Copy Markdown
Member

@dotnet/crossgen-contrib Any crossgen2 specific legs you would recommend to run on this?

@dotnet/jit-contrib Any JIT specific legs you would recommend to run on this?

for crossgen2 would be good to run the runtime-coreclr crossgen2 pipeline for validation.

@jkotas

Copy link
Copy Markdown
MemberAuthor

/azp run runtime-coreclr crossgen2

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

@jkotas

Copy link
Copy Markdown
MemberAuthor

for crossgen2 would be good to run the runtime-coreclr crossgen2 pipeline for validation.

@mangod9 What is the right way to trigger it? I have tried to trigger it above, but it does not seem to work.

@mangod9

Copy link
Copy Markdown
Member

for crossgen2 would be good to run the runtime-coreclr crossgen2 pipeline for validation.

@mangod9 What is the right way to trigger it? I have tried to trigger it above, but it does not seem to work.

Hmm.. not sure why that pipeline command fails (probably some config for the pipeline @trylek ?). For now I have manually triggered it via AzDo portal

@BruceForstall

Copy link
Copy Markdown
Contributor

I thought (almost) no triggers were enabled, due to issues with accidentally triggering too many builds. Also, #32777

@davidwrighton

Copy link
Copy Markdown
Member

@jkotas, you can no longer use any of the pipelines from the AzDo command line. You need to find the pipeline in AzDo and manually start a build.

Comment threadsrc/coreclr/src/tools/Common/Sorting/MergeSortCore.cs

@AndyAyersMSAndyAyersMS left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Were those diffs from crossgen or PMI?

Might want to run a wider set of diffs (pass -f to jit-diff) and/or look diffs from both prejitting and jitting.

Comment threadsrc/coreclr/src/jit/importer.cpp
Comment threadsrc/coreclr/src/jit/importer.cpp

@davidwrightondavidwrighton left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like you to split out the work to address the static field address issue into a separate PR, and to provide a better description of under what conditions an inlined shared generic lookup will work and where it won't work.

@jkotasjkotas mentioned this pull request Jun 26, 2020
@jkotas

Copy link
Copy Markdown
MemberAuthor

I'd like you to split out the work to address the static field address issue into a separate PR, and to provide a better description of under what conditions an inlined shared generic lookup will work and where it won't work.

@davidwrighton Done. The updated description is at the top.

@BruceForstall

Copy link
Copy Markdown
Contributor

@dotnet/jit-contrib Any JIT specific legs you would recommend to run on this?

Seems like it would be worthwhile to runtime-coreclr jitstress (https://dev.azure.com/dnceng/public/_build?definitionId=658) to run Pri-1 tests with JitStress=1/2, which would include inlining stress.

@jkotas

Copy link
Copy Markdown
MemberAuthor

Might want to run a wider set of diffs

-f crossgen diffs:

Found 25 files with textual diffs.
Summary of Code Size diffs:
(Lower is better)
Total bytes of diff: 28141 (0.086% of base)
diff is a regression.
Total byte diff includes -111 bytes from reconciling methods
Base had 2 unique methods, 111 unique bytes
Diff had 0 unique methods, 0 unique bytes
Top file regressions (bytes):
12800 : System.Collections.Immutable.dasm (5.998% of base)
5146 : System.Memory.dasm (6.358% of base)
2738 : System.Private.CoreLib.dasm (0.083% of base)
2735 : System.Collections.dasm (2.930% of base)
714 : System.Collections.Concurrent.dasm (1.559% of base)
587 : Microsoft.CodeAnalysis.dasm (0.078% of base)
473 : System.Reflection.Metadata.dasm (0.147% of base)
468 : System.Linq.dasm (0.353% of base)
298 : System.Threading.Tasks.Dataflow.dasm (0.217% of base)
282 : System.Linq.Expressions.dasm (0.010% of base)
266 : System.ObjectModel.dasm (1.353% of base)
202 : Microsoft.CSharp.dasm (0.062% of base)
179 : System.DirectoryServices.AccountManagement.dasm (0.072% of base)
171 : Microsoft.Diagnostics.Tracing.TraceEvent.dasm (0.005% of base)
153 : Newtonsoft.Json.dasm (0.025% of base)
138 : Microsoft.Extensions.Logging.Abstractions.dasm (0.565% of base)
135 : Microsoft.Diagnostics.FastSerialization.dasm (0.401% of base)
126 : System.Private.Xml.dasm (0.004% of base)
116 : System.Linq.Parallel.dasm (0.020% of base)
108 : Microsoft.CodeAnalysis.CSharp.dasm (0.005% of base)
25 total files with Code Size differences (0 improved, 25 regressed), 239 unchanged.
Top method regressions (bytes):
554 (45.975% of base) : System.Memory.dasm - ReadOnlySequence`1:TryGet(byref,byref,bool):bool:this (3 methods)
543 (35.560% of base) : System.Memory.dasm - SequenceReader`1:TryReadTo(byref,__Canon,__Canon,bool):bool:this (2 methods)
437 (108.168% of base) : System.Collections.Immutable.dasm - Builder:System.Collections.IEnumerable.GetEnumerator():IEnumerator:this (8 methods)
422 (28.964% of base) : System.Memory.dasm - SequenceReader`1:TryReadToSlow(byref,__Canon,__Canon,int,bool):bool:this (2 methods)
347 (10.534% of base) : System.Collections.Immutable.dasm - Enumerator:MoveNext():bool:this (15 methods)
344 (148.276% of base) : System.Collections.Immutable.dasm - Node:System.Collections.IEnumerable.GetEnumerator():IEnumerator:this (4 methods)
336 (13.925% of base) : System.Memory.dasm - ReadOnlySequence`1:Slice(long,long):ReadOnlySequence`1:this (3 methods)
330 (23.827% of base) : System.Collections.Immutable.dasm - Enumerator:Reset():this (11 methods)
323 (21.824% of base) : System.Collections.Immutable.dasm - Builder:AddRange(IEnumerable`1):this (6 methods)
291 (31.699% of base) : System.Memory.dasm - ReadOnlySequence`1:Slice(long):ReadOnlySequence`1:this (3 methods)
291 (41.044% of base) : System.Private.CoreLib.dasm - ValueTuple`3:System.IComparable.CompareTo(Object):int:this (3 methods)
287 (38.013% of base) : System.Reflection.Metadata.dasm - SignatureDecoder`2:DecodeType(byref,bool,int):__Canon:this
286 (54.476% of base) : System.Memory.dasm - ReadOnlySequence`1:get_FirstSpan():ReadOnlySpan`1:this (3 methods)
281 (19.957% of base) : System.Memory.dasm - SequenceReader`1:TryReadTo(byref,ReadOnlySpan`1,bool):bool:this (2 methods)
275 (158.046% of base) : System.Collections.Immutable.dasm - Node:System.Collections.Generic.IEnumerable<T>.GetEnumerator():IEnumerator`1:this (3 methods)
268 (52.652% of base) : System.Memory.dasm - ReadOnlySequence`1:get_First():ReadOnlyMemory`1:this (3 methods)
268 (41.358% of base) : System.Memory.dasm - ReadOnlySequence`1:GetPosition(long,SequencePosition):SequencePosition:this (3 methods)
267 (41.654% of base) : System.Memory.dasm - ReadOnlySequence`1:GetPosition(long):SequencePosition:this (3 methods)
266 (95.341% of base) : System.Collections.Immutable.dasm - Builder:System.Collections.Generic.IEnumerable<T>.GetEnumerator():IEnumerator`1:this (6 methods)
265 (24.953% of base) : System.Collections.Immutable.dasm - Node:CopyTo(ref,int):this (3 methods)
Top method improvements (bytes):
-169 (-26.365% of base) : System.Collections.Immutable.dasm - ImmutableArray`1:InsertRange(int,ImmutableArray`1):ImmutableArray`1:this (2 methods)
-145 (-27.672% of base) : System.Collections.Immutable.dasm - Node:GetEnumerator():Enumerator:this (4 base, 3 diff methods)
-94 (-14.506% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Implicit(ref):ReadOnlySpan`1 (18 methods)
-82 (-100.000% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:CreatePair(__Canon,Pair`2):Pair`2 (1 base, 0 diff methods)
-73 (-8.120% of base) : System.Private.CoreLib.dasm - Memory`1:Equals(Object):bool:this (4 methods)
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector128`1:get_DisplayString():String:this
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector256`1:get_DisplayString():String:this
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector64`1:get_DisplayString():String:this
-56 (-29.474% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:Add(__Canon,int):bool:this
-52 (-27.083% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:Add(__Canon,__Canon):bool:this
-44 (-6.790% of base) : System.Private.CoreLib.dasm - Span`1:op_Implicit(ref):Span`1 (18 methods)
-42 (-7.420% of base) : System.Collections.Immutable.dasm - ImmutableArray`1:RemoveAll(Predicate`1):ImmutableArray`1:this (2 methods)
-32 (-46.377% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:.ctor(__Canon,int):this
-29 (-100.000% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:CreatePair(__Canon,int):Pair`2 (1 base, 0 diff methods)
-26 (-3.641% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Inequality(ReadOnlySpan`1,ReadOnlySpan`1):bool (18 methods)
-26 (-3.641% of base) : System.Private.CoreLib.dasm - Span`1:op_Inequality(Span`1,Span`1):bool (18 methods)
-22 (-31.884% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:.ctor(__Canon,__Canon):this
-21 (-19.444% of base) : System.Linq.Parallel.dasm - OrderedGroupJoinHashLookup:CreateValuePair(GroupKeyData):Pair`2:this
-19 (-11.515% of base) : Microsoft.CodeAnalysis.CSharp.dasm - SyntaxList`1:Any(ushort):bool:this
-19 (-13.475% of base) : System.Linq.dasm - SparseArrayBuilder`1:Reserve(int):this
Top method regressions (percentages):
126 (787.500% of base) : System.DirectoryServices.AccountManagement.dasm - TrackedCollection`1:System.Collections.IEnumerable.GetEnumerator():IEnumerator:this
92 (575.000% of base) : System.Collections.Immutable.dasm - ImmutableList`1:System.Collections.Immutable.IImmutableList<T>.Remove(KeyValuePair`2,IEqualityComparer`1):IImmutableList`1:this
63 (393.750% of base) : System.Collections.dasm - SortedList`2:get_Keys():IList`1:this
63 (393.750% of base) : System.Collections.dasm - SortedList`2:System.Collections.Generic.IDictionary<TKey,TValue>.get_Keys():ICollection`1:this
63 (393.750% of base) : System.Collections.dasm - SortedList`2:System.Collections.IDictionary.get_Keys():ICollection:this
63 (393.750% of base) : System.Collections.dasm - SortedList`2:System.Collections.Generic.IReadOnlyDictionary<TKey,TValue>.get_Keys():IEnumerable`1:this
63 (393.750% of base) : System.Collections.dasm - SortedList`2:get_Values():IList`1:this
63 (393.750% of base) : System.Collections.dasm - SortedList`2:System.Collections.Generic.IDictionary<TKey,TValue>.get_Values():ICollection`1:this
63 (393.750% of base) : System.Collections.dasm - SortedList`2:System.Collections.IDictionary.get_Values():ICollection:this
63 (393.750% of base) : System.Collections.dasm - SortedList`2:System.Collections.Generic.IReadOnlyDictionary<TKey,TValue>.get_Values():IEnumerable`1:this
62 (387.500% of base) : System.Collections.dasm - SortedDictionary`2:System.Collections.Generic.IDictionary<TKey,TValue>.get_Keys():ICollection`1:this
62 (387.500% of base) : System.Collections.dasm - SortedDictionary`2:System.Collections.Generic.IReadOnlyDictionary<TKey,TValue>.get_Keys():IEnumerable`1:this
62 (387.500% of base) : System.Collections.dasm - SortedDictionary`2:System.Collections.Generic.IDictionary<TKey,TValue>.get_Values():ICollection`1:this
62 (387.500% of base) : System.Collections.dasm - SortedDictionary`2:System.Collections.Generic.IReadOnlyDictionary<TKey,TValue>.get_Values():IEnumerable`1:this
62 (387.500% of base) : System.Collections.dasm - SortedDictionary`2:System.Collections.IDictionary.get_Keys():ICollection:this
62 (387.500% of base) : System.Collections.dasm - SortedDictionary`2:System.Collections.IDictionary.get_Values():ICollection:this
61 (381.250% of base) : Newtonsoft.Json.dasm - EnumerableDictionaryWrapper`2:System.Collections.IEnumerable.GetEnumerator():IEnumerator:this
61 (381.250% of base) : System.Net.Http.dasm - HttpHeaderValueCollection`1:System.Collections.IEnumerable.GetEnumerator():IEnumerator:this
60 (375.000% of base) : System.Collections.Immutable.dasm - ImmutableList`1:System.Collections.Immutable.IImmutableList<T>.Remove(__Canon,IEqualityComparer`1):IImmutableList`1:this
60 (375.000% of base) : System.Collections.Immutable.dasm - ImmutableStack`1:System.Collections.Immutable.IImmutableStack<T>.Push(__Canon):IImmutableStack`1:this
Top method improvements (percentages):
-82 (-100.000% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:CreatePair(__Canon,Pair`2):Pair`2 (1 base, 0 diff methods)
-29 (-100.000% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:CreatePair(__Canon,int):Pair`2 (1 base, 0 diff methods)
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector128`1:get_DisplayString():String:this
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector256`1:get_DisplayString():String:this
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector64`1:get_DisplayString():String:this
-32 (-46.377% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:.ctor(__Canon,int):this
-22 (-31.884% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:.ctor(__Canon,__Canon):this
-56 (-29.474% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:Add(__Canon,int):bool:this
-145 (-27.672% of base) : System.Collections.Immutable.dasm - Node:GetEnumerator():Enumerator:this (4 base, 3 diff methods)
-52 (-27.083% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:Add(__Canon,__Canon):bool:this
-169 (-26.365% of base) : System.Collections.Immutable.dasm - ImmutableArray`1:InsertRange(int,ImmutableArray`1):ImmutableArray`1:this (2 methods)
-21 (-19.444% of base) : System.Linq.Parallel.dasm - OrderedGroupJoinHashLookup:CreateValuePair(GroupKeyData):Pair`2:this
-94 (-14.506% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Implicit(ref):ReadOnlySpan`1 (18 methods)
-19 (-13.475% of base) : System.Linq.dasm - SparseArrayBuilder`1:Reserve(int):this
-18 (-12.950% of base) : System.Collections.Immutable.dasm - ImmutableDictionary`2:TryGetValue(__Canon,byref):bool:this
-18 (-12.950% of base) : System.Collections.Immutable.dasm - ImmutableDictionary`2:TryGetKey(__Canon,byref):bool:this
-9 (-12.857% of base) : Microsoft.CodeAnalysis.VisualBasic.dasm - _Closure$__17`1:.cctor()
-13 (-12.745% of base) : System.Collections.Immutable.dasm - ImmutableDictionary`2:Contains(KeyValuePair`2):bool:this
-19 (-11.515% of base) : Microsoft.CodeAnalysis.CSharp.dasm - SyntaxList`1:Any(ushort):bool:this
-13 (-10.484% of base) : System.Collections.Immutable.dasm - ImmutableDictionary`2:ContainsKey(__Canon):bool:this
406 total methods with Code Size differences (43 improved, 363 regressed), 186975 unchanged.

@jkotas

Copy link
Copy Markdown
MemberAuthor
  • ReadOnlySequence size regressions is caused by AggresiveInlining on ~70 lines method
  • ImmutableCollections size regression is a lots of small methods that are getting inlined into each other now

@jkotas

Copy link
Copy Markdown
MemberAuthor

JIT stress failure is known issue #36681
Installer Build and Test coreclr failure is known issue #35068
oreCLR Pri0 Runtime Tests Run Windows_NT x86 checked is known issue #38284

@jkotas
jkotas merged commit 31699de into dotnet:masterJun 27, 2020
@jkotas
jkotas deleted the generics-inlining branch June 27, 2020 13:36
@nietras

Copy link
Copy Markdown
Contributor

Great to see this merged. 👍

when the caller and callee are the same exact type and instantiation.

This is quite clear but to be sure, I assume this means this won't work for base class methods for example? E. g. called in a derived class?

@jkotas

Copy link
Copy Markdown
MemberAuthor

I assume this means this won't work for base class methods for example? E. g. called in a derived class?

Correct.

@ghostghost locked as resolved and limited conversation to collaborators Dec 8, 2020
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

11 participants

@jkotas@nietras@mangod9@BruceForstall@davidwrighton@EgorBo@stephentoub@jakobbotsch@AndyAyersMS@MichalStrehovsky@Dotnet-GitSync-Bot
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Enable inlining of shared generics code within same type - #38229

Merged
jkotas merged 11 commits into
dotnet:masterfrom
jkotas:generics-inlining
Jun 27, 2020
Merged

Enable inlining of shared generics code within same type#38229
jkotas merged 11 commits into
dotnet:masterfrom
jkotas:generics-inlining

Conversation

@jkotas

@jkotasjkotas commented Jun 22, 2020

Copy link
Copy Markdown
Member

This change allows inlining of generic dictionary lookups when the caller and callee are the same exact type and instantiation. It is a simpler version of #31833.

Motivation for this change is to unblock #37941.

Example:

class G<T>
{
static bool M1() => typeof(T);
object M2() => new T[1];
bool M3() => Unsafe.SizeOf<T>();
static string M()
{
// Assume that T is string
// All M1, M2, M3 are inlined after this change, but were not inlined before this change
M1(); M2(); M3();
// Still not inlined - different instantiation, not the same exact type
G<List<T>>.M1();
// Still not inlined - different type.
OtherG<T>.M();
}
}

@jkotas
jkotas marked this pull request as draft June 22, 2020 19:22
@jkotas
jkotasforce-pushed the generics-inlining branch from 9645520 to dd93ffaCompareJune 23, 2020 06:58
@jaredparjaredpar mentioned this pull request Jun 23, 2020
@jkotas
jkotasforce-pushed the generics-inlining branch 3 times, most recently from 8cbb058 to 3770ae3CompareJune 24, 2020 16:20
Comment threadsrc/coreclr/src/ToolBox/superpmi/superpmi-shared/methodcontext.cpp Outdated
@jkotas
jkotasforce-pushed the generics-inlining branch from 3819aa0 to d09090eCompareJune 24, 2020 23:22
@jkotas

Copy link
Copy Markdown
MemberAuthor

Asm diffs

Total bytes of diff: 2738 (0.083% of base)
diff is a regression.
Top file regressions (bytes):
2738 : System.Private.CoreLib.dasm (0.083% of base)
1 total files with Code Size differences (0 improved, 1 regressed), 0 unchanged.
Top method regressions (bytes):
291 (41.044% of base) : System.Private.CoreLib.dasm - ValueTuple`3:System.IComparable.CompareTo(Object):int:this (3 methods)
260 (67.010% of base) : System.Private.CoreLib.dasm - ValueTuple`3:Equals(Object):bool:this (3 methods)
184 (3.340% of base) : System.Private.CoreLib.dasm - GenericArraySortHelper`1:IntroSort(Span`1,int) (12 methods)
173 (28.978% of base) : System.Private.CoreLib.dasm - ValueTask`1:ToString():String:this (3 methods)
159 (15.071% of base) : System.Private.CoreLib.dasm - Memory`1:CopyTo(Memory`1):this (4 methods)
159 (14.171% of base) : System.Private.CoreLib.dasm - Memory`1:TryCopyTo(Memory`1):bool:this (4 methods)
157 (69.778% of base) : System.Private.CoreLib.dasm - ValueTuple`4:Equals(Object):bool:this (2 methods)
118 (6.833% of base) : System.Private.CoreLib.dasm - Vector128`1:Equals(Object):bool:this (11 methods)
118 (3.562% of base) : System.Private.CoreLib.dasm - GenericArraySortHelper`1:PickPivotAndPartition(Span`1):int (12 methods)
94 (8.818% of base) : System.Private.CoreLib.dasm - ReadOnlyMemory`1:CopyTo(Memory`1):this (4 methods)
94 (8.297% of base) : System.Private.CoreLib.dasm - ReadOnlyMemory`1:TryCopyTo(Memory`1):bool:this (4 methods)
87 (13.344% of base) : System.Private.CoreLib.dasm - Memory`1:ToArray():ref:this (4 methods)
87 (14.846% of base) : System.Private.CoreLib.dasm - ReadOnlyMemory`1:ToArray():ref:this (4 methods)
76 (8.736% of base) : System.Private.CoreLib.dasm - List`1:System.Collections.IList.IndexOf(Object):int:this (8 methods)
75 (202.703% of base) : System.Private.CoreLib.dasm - List`1:Contains(KeyValuePair`2):bool:this
72 (39.560% of base) : System.Private.CoreLib.dasm - Lazy`1:CreateValue():__Canon:this
66 (129.412% of base) : System.Private.CoreLib.dasm - List`1:Remove(KeyValuePair`2):bool:this
63 (75.000% of base) : System.Private.CoreLib.dasm - TaskCompletionSource`1:.ctor(Object):this (2 methods)
62 (16.712% of base) : System.Private.CoreLib.dasm - ValueTuple`2:Equals(Object):bool:this (3 methods)
60 (8.310% of base) : System.Private.CoreLib.dasm - ValueTuple`2:System.IComparable.CompareTo(Object):int:this (3 methods)
Top method improvements (bytes):
-94 (-14.506% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Implicit(ref):ReadOnlySpan`1 (18 methods)
-73 (-8.120% of base) : System.Private.CoreLib.dasm - Memory`1:Equals(Object):bool:this (4 methods)
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector128`1:get_DisplayString():String:this
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector256`1:get_DisplayString():String:this
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector64`1:get_DisplayString():String:this
-44 (-6.790% of base) : System.Private.CoreLib.dasm - Span`1:op_Implicit(ref):Span`1 (18 methods)
-26 (-3.641% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Inequality(ReadOnlySpan`1,ReadOnlySpan`1):bool (18 methods)
-26 (-3.641% of base) : System.Private.CoreLib.dasm - Span`1:op_Inequality(Span`1,Span`1):bool (18 methods)
-15 (-0.957% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Implicit(ArraySegment`1):ReadOnlySpan`1 (18 methods)
-4 (-0.702% of base) : System.Private.CoreLib.dasm - AsyncValueTaskMethodBuilder`1:get_Task():ValueTask`1:this (2 methods)
-2 (-0.217% of base) : System.Private.CoreLib.dasm - ArraySegment`1:op_Inequality(ArraySegment`1,ArraySegment`1):bool (18 methods)
-2 (-0.930% of base) : System.Private.CoreLib.dasm - Memory`1:op_Implicit(ref):Memory`1 (4 methods)
Top method regressions (percentages):
47 (293.750% of base) : System.Private.CoreLib.dasm - SingleProducerSingleConsumerQueue`1:System.Collections.IEnumerable.GetEnumerator():IEnumerator:this
75 (202.703% of base) : System.Private.CoreLib.dasm - List`1:Contains(KeyValuePair`2):bool:this
66 (129.412% of base) : System.Private.CoreLib.dasm - List`1:Remove(KeyValuePair`2):bool:this
46 (124.324% of base) : System.Private.CoreLib.dasm - List`1:Contains(__Canon):bool:this
63 (75.000% of base) : System.Private.CoreLib.dasm - TaskCompletionSource`1:.ctor(Object):this (2 methods)
37 (72.549% of base) : System.Private.CoreLib.dasm - List`1:Remove(__Canon):bool:this
36 (72.000% of base) : System.Private.CoreLib.dasm - AsyncTaskMethodBuilder`1:SetNotificationForWaitCompletion(bool):this
59 (70.238% of base) : System.Private.CoreLib.dasm - TaskCompletionSource`1:.ctor(int):this (2 methods)
157 (69.778% of base) : System.Private.CoreLib.dasm - ValueTuple`4:Equals(Object):bool:this (2 methods)
260 (67.010% of base) : System.Private.CoreLib.dasm - ValueTuple`3:Equals(Object):bool:this (3 methods)
25 (43.103% of base) : System.Private.CoreLib.dasm - HashSet`1:System.Collections.Generic.IEnumerable<T>.GetEnumerator():IEnumerator`1:this
25 (43.103% of base) : System.Private.CoreLib.dasm - HashSet`1:System.Collections.IEnumerable.GetEnumerator():IEnumerator:this
291 (41.044% of base) : System.Private.CoreLib.dasm - ValueTuple`3:System.IComparable.CompareTo(Object):int:this (3 methods)
72 (39.560% of base) : System.Private.CoreLib.dasm - Lazy`1:CreateValue():__Canon:this
173 (28.978% of base) : System.Private.CoreLib.dasm - ValueTask`1:ToString():String:this (3 methods)
47 (27.485% of base) : System.Private.CoreLib.dasm - Lazy`1:ExecutionAndPublication(LazyHelper,bool):this
48 (19.753% of base) : System.Private.CoreLib.dasm - AsyncValueTaskMethodBuilder`1:SetException(Exception):this (2 methods)
18 (19.149% of base) : System.Private.CoreLib.dasm - Collection`1:System.Collections.IList.Contains(Object):bool:this
18 (18.947% of base) : System.Private.CoreLib.dasm - Collection`1:System.Collections.IList.IndexOf(Object):int:this
62 (16.712% of base) : System.Private.CoreLib.dasm - ValueTuple`2:Equals(Object):bool:this (3 methods)
Top method improvements (percentages):
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector128`1:get_DisplayString():String:this
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector256`1:get_DisplayString():String:this
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector64`1:get_DisplayString():String:this
-94 (-14.506% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Implicit(ref):ReadOnlySpan`1 (18 methods)
-73 (-8.120% of base) : System.Private.CoreLib.dasm - Memory`1:Equals(Object):bool:this (4 methods)
-44 (-6.790% of base) : System.Private.CoreLib.dasm - Span`1:op_Implicit(ref):Span`1 (18 methods)
-26 (-3.641% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Inequality(ReadOnlySpan`1,ReadOnlySpan`1):bool (18 methods)
-26 (-3.641% of base) : System.Private.CoreLib.dasm - Span`1:op_Inequality(Span`1,Span`1):bool (18 methods)
-15 (-0.957% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Implicit(ArraySegment`1):ReadOnlySpan`1 (18 methods)
-2 (-0.930% of base) : System.Private.CoreLib.dasm - Memory`1:op_Implicit(ref):Memory`1 (4 methods)
-4 (-0.702% of base) : System.Private.CoreLib.dasm - AsyncValueTaskMethodBuilder`1:get_Task():ValueTask`1:this (2 methods)
-2 (-0.217% of base) : System.Private.CoreLib.dasm - ArraySegment`1:op_Inequality(ArraySegment`1,ArraySegment`1):bool (18 methods)
57 total methods with Code Size differences (12 improved, 45 regressed), 20772 unchanged.

@jkotas

jkotas commented Jun 25, 2020

Copy link
Copy Markdown
MemberAuthor

Example of code-size regression - inlining tends to increase code size on average:

List`1:Contains(KeyValuePair`2):bool:this

Baseline

G_M57469_IG01:
sub rsp, 40
;; bbWeight=1 PerfScore 0.25
G_M57469_IG02:
cmp dword ptr [rcx+16], 0
je SHORT G_M57469_IG05
;; bbWeight=1 PerfScore 3.00
G_M57469_IG03:
call [List`1:IndexOf(KeyValuePair`2):int:this]
cmp eax, -1
setne al
movzx rax, al
;; bbWeight=0.50 PerfScore 2.25
G_M57469_IG04:
add rsp, 40
ret ;; bbWeight=0.50 PerfScore 0.63
G_M57469_IG05:
xor eax, eax
;; bbWeight=0.50 PerfScore 0.13
G_M57469_IG06:
add rsp, 40
ret 

Diff

G_M57469_IG01:
push rdi
push rsi
push rbx
sub rsp, 64
xor rax, rax
mov qword ptr [rsp+28H], rax
mov qword ptr [rsp+38H], rcx
mov rsi, rcx
;; bbWeight=1 PerfScore 5.75
G_M57469_IG02:
cmp dword ptr [rsi+16], 0
je SHORT G_M57469_IG05
;; bbWeight=1 PerfScore 3.00
G_M57469_IG03:
mov rdi, gword ptr [rdx]
mov ebx, dword ptr [rdx+8]
mov rcx, qword ptr [rsi]
call [CORINFO_HELP_READYTORUN_GENERIC_HANDLE]
mov rcx, rax
mov rdx, gword ptr [rsi+8]
lea r8, bword ptr [rsp+28H]
mov gword ptr [r8], rdi
mov dword ptr [r8+8], ebx
mov r8d, dword ptr [rsi+16]
mov dword ptr [rsp+20H], r8d
lea r8, bword ptr [rsp+28H]
xor r9d, r9d
call [Array:IndexOf(ref,KeyValuePair`2,int,int):int]
cmp eax, -1
setne al
movzx rax, al
;; bbWeight=0.50 PerfScore 11.00
G_M57469_IG04:
add rsp, 64
pop rbx
pop rsi
pop rdi
ret ;; bbWeight=0.50 PerfScore 1.38
G_M57469_IG05:
xor eax, eax
;; bbWeight=0.50 PerfScore 0.13
G_M57469_IG06:
add rsp, 64
pop rbx
pop rsi
pop rdi
ret 

@jkotas

Copy link
Copy Markdown
MemberAuthor

Example of code-size improvement - inlining reduces code size when it enables additional constant-prop:

Vector128`1:get_DisplayString

Baseline:

G_M30183_IG01:
push rdi
push rsi
sub rsp, 40
mov qword ptr [rsp+20H], rdx
mov rdi, rcx
mov rsi, rdx
;; bbWeight=1 PerfScore 3.75
G_M30183_IG02:
mov rcx, rsi
call [CORINFO_HELP_READYTORUN_GENERIC_HANDLE]
mov rcx, rax
call [Vector128`1:get_IsSupported():bool]
test al, al
je SHORT G_M30183_IG05
;; bbWeight=1 PerfScore 7.75
G_M30183_IG03:
mov rcx, rsi
call [CORINFO_HELP_READYTORUN_GENERIC_HANDLE]
mov rcx, rdi
call rax
nop ;; bbWeight=0.50 PerfScore 3.38
G_M30183_IG04:
add rsp, 40
pop rsi
pop rdi
ret ;; bbWeight=0.50 PerfScore 1.13
G_M30183_IG05:
mov rcx, qword ptr [(reloc)]
mov rcx, gword ptr [rcx]
mov rdx, qword ptr [(reloc)]
mov rdx, gword ptr [rdx]
call [SR:GetResourceString(String,String):String]
nop ;; bbWeight=0.50 PerfScore 5.63
G_M30183_IG06:
add rsp, 40
pop rsi
pop rdi
ret ;; bbWeight=0.50 PerfScore 1.13

Diff:

G_M30183_IG01:
sub rsp, 40
;; bbWeight=1 PerfScore 0.25
G_M30183_IG02:
mov rcx, qword ptr [(reloc)]
mov rcx, gword ptr [rcx]
mov rdx, qword ptr [(reloc)]
mov rdx, gword ptr [rdx]
call [SR:GetResourceString(String,String):String]
nop ;; bbWeight=1 PerfScore 11.25
G_M30183_IG03:
add rsp, 40
ret 

@nietras

Copy link
Copy Markdown
Contributor

The example improvements are exactly the kind I am hoping for 👍

@jkotas
jkotas marked this pull request as ready for review June 26, 2020 05:52
@jkotas

Copy link
Copy Markdown
MemberAuthor

@dotnet/crossgen-contrib Any crossgen2 specific legs you would recommend to run on this?

@dotnet/jit-contrib Any JIT specific legs you would recommend to run on this?

Comment threadsrc/coreclr/src/tools/Common/Sorting/MergeSortCore.cs
Comment threadsrc/coreclr/src/vm/jitinterface.cpp
@jkotasjkotas changed the title WIP: Enable inlining of shared generics code within same typeEnable inlining of shared generics code within same typeJun 26, 2020

@MichalStrehovskyMichalStrehovsky left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Crossgen2 side looks good to me.

I echo Manish's question about the assert - I think that code might be reachable.

Not commenting on the parallel sort thing because I didn't look at that code before.

@mangod9

Copy link
Copy Markdown
Member

@dotnet/crossgen-contrib Any crossgen2 specific legs you would recommend to run on this?

@dotnet/jit-contrib Any JIT specific legs you would recommend to run on this?

for crossgen2 would be good to run the runtime-coreclr crossgen2 pipeline for validation.

@jkotas

Copy link
Copy Markdown
MemberAuthor

/azp run runtime-coreclr crossgen2

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

@jkotas

Copy link
Copy Markdown
MemberAuthor

for crossgen2 would be good to run the runtime-coreclr crossgen2 pipeline for validation.

@mangod9 What is the right way to trigger it? I have tried to trigger it above, but it does not seem to work.

@mangod9

Copy link
Copy Markdown
Member

for crossgen2 would be good to run the runtime-coreclr crossgen2 pipeline for validation.

@mangod9 What is the right way to trigger it? I have tried to trigger it above, but it does not seem to work.

Hmm.. not sure why that pipeline command fails (probably some config for the pipeline @trylek ?). For now I have manually triggered it via AzDo portal

@BruceForstall

Copy link
Copy Markdown
Contributor

I thought (almost) no triggers were enabled, due to issues with accidentally triggering too many builds. Also, #32777

@davidwrighton

Copy link
Copy Markdown
Member

@jkotas, you can no longer use any of the pipelines from the AzDo command line. You need to find the pipeline in AzDo and manually start a build.

Comment threadsrc/coreclr/src/tools/Common/Sorting/MergeSortCore.cs

@AndyAyersMSAndyAyersMS left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Were those diffs from crossgen or PMI?

Might want to run a wider set of diffs (pass -f to jit-diff) and/or look diffs from both prejitting and jitting.

Comment threadsrc/coreclr/src/jit/importer.cpp
Comment threadsrc/coreclr/src/jit/importer.cpp

@davidwrightondavidwrighton left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like you to split out the work to address the static field address issue into a separate PR, and to provide a better description of under what conditions an inlined shared generic lookup will work and where it won't work.

@jkotasjkotas mentioned this pull request Jun 26, 2020
@jkotas

Copy link
Copy Markdown
MemberAuthor

I'd like you to split out the work to address the static field address issue into a separate PR, and to provide a better description of under what conditions an inlined shared generic lookup will work and where it won't work.

@davidwrighton Done. The updated description is at the top.

@BruceForstall

Copy link
Copy Markdown
Contributor

@dotnet/jit-contrib Any JIT specific legs you would recommend to run on this?

Seems like it would be worthwhile to runtime-coreclr jitstress (https://dev.azure.com/dnceng/public/_build?definitionId=658) to run Pri-1 tests with JitStress=1/2, which would include inlining stress.

@jkotas

Copy link
Copy Markdown
MemberAuthor

Might want to run a wider set of diffs

-f crossgen diffs:

Found 25 files with textual diffs.
Summary of Code Size diffs:
(Lower is better)
Total bytes of diff: 28141 (0.086% of base)
diff is a regression.
Total byte diff includes -111 bytes from reconciling methods
Base had 2 unique methods, 111 unique bytes
Diff had 0 unique methods, 0 unique bytes
Top file regressions (bytes):
12800 : System.Collections.Immutable.dasm (5.998% of base)
5146 : System.Memory.dasm (6.358% of base)
2738 : System.Private.CoreLib.dasm (0.083% of base)
2735 : System.Collections.dasm (2.930% of base)
714 : System.Collections.Concurrent.dasm (1.559% of base)
587 : Microsoft.CodeAnalysis.dasm (0.078% of base)
473 : System.Reflection.Metadata.dasm (0.147% of base)
468 : System.Linq.dasm (0.353% of base)
298 : System.Threading.Tasks.Dataflow.dasm (0.217% of base)
282 : System.Linq.Expressions.dasm (0.010% of base)
266 : System.ObjectModel.dasm (1.353% of base)
202 : Microsoft.CSharp.dasm (0.062% of base)
179 : System.DirectoryServices.AccountManagement.dasm (0.072% of base)
171 : Microsoft.Diagnostics.Tracing.TraceEvent.dasm (0.005% of base)
153 : Newtonsoft.Json.dasm (0.025% of base)
138 : Microsoft.Extensions.Logging.Abstractions.dasm (0.565% of base)
135 : Microsoft.Diagnostics.FastSerialization.dasm (0.401% of base)
126 : System.Private.Xml.dasm (0.004% of base)
116 : System.Linq.Parallel.dasm (0.020% of base)
108 : Microsoft.CodeAnalysis.CSharp.dasm (0.005% of base)
25 total files with Code Size differences (0 improved, 25 regressed), 239 unchanged.
Top method regressions (bytes):
554 (45.975% of base) : System.Memory.dasm - ReadOnlySequence`1:TryGet(byref,byref,bool):bool:this (3 methods)
543 (35.560% of base) : System.Memory.dasm - SequenceReader`1:TryReadTo(byref,__Canon,__Canon,bool):bool:this (2 methods)
437 (108.168% of base) : System.Collections.Immutable.dasm - Builder:System.Collections.IEnumerable.GetEnumerator():IEnumerator:this (8 methods)
422 (28.964% of base) : System.Memory.dasm - SequenceReader`1:TryReadToSlow(byref,__Canon,__Canon,int,bool):bool:this (2 methods)
347 (10.534% of base) : System.Collections.Immutable.dasm - Enumerator:MoveNext():bool:this (15 methods)
344 (148.276% of base) : System.Collections.Immutable.dasm - Node:System.Collections.IEnumerable.GetEnumerator():IEnumerator:this (4 methods)
336 (13.925% of base) : System.Memory.dasm - ReadOnlySequence`1:Slice(long,long):ReadOnlySequence`1:this (3 methods)
330 (23.827% of base) : System.Collections.Immutable.dasm - Enumerator:Reset():this (11 methods)
323 (21.824% of base) : System.Collections.Immutable.dasm - Builder:AddRange(IEnumerable`1):this (6 methods)
291 (31.699% of base) : System.Memory.dasm - ReadOnlySequence`1:Slice(long):ReadOnlySequence`1:this (3 methods)
291 (41.044% of base) : System.Private.CoreLib.dasm - ValueTuple`3:System.IComparable.CompareTo(Object):int:this (3 methods)
287 (38.013% of base) : System.Reflection.Metadata.dasm - SignatureDecoder`2:DecodeType(byref,bool,int):__Canon:this
286 (54.476% of base) : System.Memory.dasm - ReadOnlySequence`1:get_FirstSpan():ReadOnlySpan`1:this (3 methods)
281 (19.957% of base) : System.Memory.dasm - SequenceReader`1:TryReadTo(byref,ReadOnlySpan`1,bool):bool:this (2 methods)
275 (158.046% of base) : System.Collections.Immutable.dasm - Node:System.Collections.Generic.IEnumerable<T>.GetEnumerator():IEnumerator`1:this (3 methods)
268 (52.652% of base) : System.Memory.dasm - ReadOnlySequence`1:get_First():ReadOnlyMemory`1:this (3 methods)
268 (41.358% of base) : System.Memory.dasm - ReadOnlySequence`1:GetPosition(long,SequencePosition):SequencePosition:this (3 methods)
267 (41.654% of base) : System.Memory.dasm - ReadOnlySequence`1:GetPosition(long):SequencePosition:this (3 methods)
266 (95.341% of base) : System.Collections.Immutable.dasm - Builder:System.Collections.Generic.IEnumerable<T>.GetEnumerator():IEnumerator`1:this (6 methods)
265 (24.953% of base) : System.Collections.Immutable.dasm - Node:CopyTo(ref,int):this (3 methods)
Top method improvements (bytes):
-169 (-26.365% of base) : System.Collections.Immutable.dasm - ImmutableArray`1:InsertRange(int,ImmutableArray`1):ImmutableArray`1:this (2 methods)
-145 (-27.672% of base) : System.Collections.Immutable.dasm - Node:GetEnumerator():Enumerator:this (4 base, 3 diff methods)
-94 (-14.506% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Implicit(ref):ReadOnlySpan`1 (18 methods)
-82 (-100.000% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:CreatePair(__Canon,Pair`2):Pair`2 (1 base, 0 diff methods)
-73 (-8.120% of base) : System.Private.CoreLib.dasm - Memory`1:Equals(Object):bool:this (4 methods)
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector128`1:get_DisplayString():String:this
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector256`1:get_DisplayString():String:this
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector64`1:get_DisplayString():String:this
-56 (-29.474% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:Add(__Canon,int):bool:this
-52 (-27.083% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:Add(__Canon,__Canon):bool:this
-44 (-6.790% of base) : System.Private.CoreLib.dasm - Span`1:op_Implicit(ref):Span`1 (18 methods)
-42 (-7.420% of base) : System.Collections.Immutable.dasm - ImmutableArray`1:RemoveAll(Predicate`1):ImmutableArray`1:this (2 methods)
-32 (-46.377% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:.ctor(__Canon,int):this
-29 (-100.000% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:CreatePair(__Canon,int):Pair`2 (1 base, 0 diff methods)
-26 (-3.641% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Inequality(ReadOnlySpan`1,ReadOnlySpan`1):bool (18 methods)
-26 (-3.641% of base) : System.Private.CoreLib.dasm - Span`1:op_Inequality(Span`1,Span`1):bool (18 methods)
-22 (-31.884% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:.ctor(__Canon,__Canon):this
-21 (-19.444% of base) : System.Linq.Parallel.dasm - OrderedGroupJoinHashLookup:CreateValuePair(GroupKeyData):Pair`2:this
-19 (-11.515% of base) : Microsoft.CodeAnalysis.CSharp.dasm - SyntaxList`1:Any(ushort):bool:this
-19 (-13.475% of base) : System.Linq.dasm - SparseArrayBuilder`1:Reserve(int):this
Top method regressions (percentages):
126 (787.500% of base) : System.DirectoryServices.AccountManagement.dasm - TrackedCollection`1:System.Collections.IEnumerable.GetEnumerator():IEnumerator:this
92 (575.000% of base) : System.Collections.Immutable.dasm - ImmutableList`1:System.Collections.Immutable.IImmutableList<T>.Remove(KeyValuePair`2,IEqualityComparer`1):IImmutableList`1:this
63 (393.750% of base) : System.Collections.dasm - SortedList`2:get_Keys():IList`1:this
63 (393.750% of base) : System.Collections.dasm - SortedList`2:System.Collections.Generic.IDictionary<TKey,TValue>.get_Keys():ICollection`1:this
63 (393.750% of base) : System.Collections.dasm - SortedList`2:System.Collections.IDictionary.get_Keys():ICollection:this
63 (393.750% of base) : System.Collections.dasm - SortedList`2:System.Collections.Generic.IReadOnlyDictionary<TKey,TValue>.get_Keys():IEnumerable`1:this
63 (393.750% of base) : System.Collections.dasm - SortedList`2:get_Values():IList`1:this
63 (393.750% of base) : System.Collections.dasm - SortedList`2:System.Collections.Generic.IDictionary<TKey,TValue>.get_Values():ICollection`1:this
63 (393.750% of base) : System.Collections.dasm - SortedList`2:System.Collections.IDictionary.get_Values():ICollection:this
63 (393.750% of base) : System.Collections.dasm - SortedList`2:System.Collections.Generic.IReadOnlyDictionary<TKey,TValue>.get_Values():IEnumerable`1:this
62 (387.500% of base) : System.Collections.dasm - SortedDictionary`2:System.Collections.Generic.IDictionary<TKey,TValue>.get_Keys():ICollection`1:this
62 (387.500% of base) : System.Collections.dasm - SortedDictionary`2:System.Collections.Generic.IReadOnlyDictionary<TKey,TValue>.get_Keys():IEnumerable`1:this
62 (387.500% of base) : System.Collections.dasm - SortedDictionary`2:System.Collections.Generic.IDictionary<TKey,TValue>.get_Values():ICollection`1:this
62 (387.500% of base) : System.Collections.dasm - SortedDictionary`2:System.Collections.Generic.IReadOnlyDictionary<TKey,TValue>.get_Values():IEnumerable`1:this
62 (387.500% of base) : System.Collections.dasm - SortedDictionary`2:System.Collections.IDictionary.get_Keys():ICollection:this
62 (387.500% of base) : System.Collections.dasm - SortedDictionary`2:System.Collections.IDictionary.get_Values():ICollection:this
61 (381.250% of base) : Newtonsoft.Json.dasm - EnumerableDictionaryWrapper`2:System.Collections.IEnumerable.GetEnumerator():IEnumerator:this
61 (381.250% of base) : System.Net.Http.dasm - HttpHeaderValueCollection`1:System.Collections.IEnumerable.GetEnumerator():IEnumerator:this
60 (375.000% of base) : System.Collections.Immutable.dasm - ImmutableList`1:System.Collections.Immutable.IImmutableList<T>.Remove(__Canon,IEqualityComparer`1):IImmutableList`1:this
60 (375.000% of base) : System.Collections.Immutable.dasm - ImmutableStack`1:System.Collections.Immutable.IImmutableStack<T>.Push(__Canon):IImmutableStack`1:this
Top method improvements (percentages):
-82 (-100.000% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:CreatePair(__Canon,Pair`2):Pair`2 (1 base, 0 diff methods)
-29 (-100.000% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:CreatePair(__Canon,int):Pair`2 (1 base, 0 diff methods)
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector128`1:get_DisplayString():String:this
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector256`1:get_DisplayString():String:this
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector64`1:get_DisplayString():String:this
-32 (-46.377% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:.ctor(__Canon,int):this
-22 (-31.884% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:.ctor(__Canon,__Canon):this
-56 (-29.474% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:Add(__Canon,int):bool:this
-145 (-27.672% of base) : System.Collections.Immutable.dasm - Node:GetEnumerator():Enumerator:this (4 base, 3 diff methods)
-52 (-27.083% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:Add(__Canon,__Canon):bool:this
-169 (-26.365% of base) : System.Collections.Immutable.dasm - ImmutableArray`1:InsertRange(int,ImmutableArray`1):ImmutableArray`1:this (2 methods)
-21 (-19.444% of base) : System.Linq.Parallel.dasm - OrderedGroupJoinHashLookup:CreateValuePair(GroupKeyData):Pair`2:this
-94 (-14.506% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Implicit(ref):ReadOnlySpan`1 (18 methods)
-19 (-13.475% of base) : System.Linq.dasm - SparseArrayBuilder`1:Reserve(int):this
-18 (-12.950% of base) : System.Collections.Immutable.dasm - ImmutableDictionary`2:TryGetValue(__Canon,byref):bool:this
-18 (-12.950% of base) : System.Collections.Immutable.dasm - ImmutableDictionary`2:TryGetKey(__Canon,byref):bool:this
-9 (-12.857% of base) : Microsoft.CodeAnalysis.VisualBasic.dasm - _Closure$__17`1:.cctor()
-13 (-12.745% of base) : System.Collections.Immutable.dasm - ImmutableDictionary`2:Contains(KeyValuePair`2):bool:this
-19 (-11.515% of base) : Microsoft.CodeAnalysis.CSharp.dasm - SyntaxList`1:Any(ushort):bool:this
-13 (-10.484% of base) : System.Collections.Immutable.dasm - ImmutableDictionary`2:ContainsKey(__Canon):bool:this
406 total methods with Code Size differences (43 improved, 363 regressed), 186975 unchanged.

@jkotas

Copy link
Copy Markdown
MemberAuthor
  • ReadOnlySequence size regressions is caused by AggresiveInlining on ~70 lines method
  • ImmutableCollections size regression is a lots of small methods that are getting inlined into each other now

@jkotas

Copy link
Copy Markdown
MemberAuthor

JIT stress failure is known issue #36681
Installer Build and Test coreclr failure is known issue #35068
oreCLR Pri0 Runtime Tests Run Windows_NT x86 checked is known issue #38284

@jkotas
jkotas merged commit 31699de into dotnet:masterJun 27, 2020
@jkotas
jkotas deleted the generics-inlining branch June 27, 2020 13:36
@nietras

Copy link
Copy Markdown
Contributor

Great to see this merged. 👍

when the caller and callee are the same exact type and instantiation.

This is quite clear but to be sure, I assume this means this won't work for base class methods for example? E. g. called in a derived class?

@jkotas

Copy link
Copy Markdown
MemberAuthor

I assume this means this won't work for base class methods for example? E. g. called in a derived class?

Correct.

@ghostghost locked as resolved and limited conversation to collaborators Dec 8, 2020
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

11 participants

@jkotas@nietras@mangod9@BruceForstall@davidwrighton@EgorBo@stephentoub@jakobbotsch@AndyAyersMS@MichalStrehovsky@Dotnet-GitSync-Bot
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Enable inlining of shared generics code within same type - #38229

Merged
jkotas merged 11 commits into
dotnet:masterfrom
jkotas:generics-inlining
Jun 27, 2020
Merged

Enable inlining of shared generics code within same type#38229
jkotas merged 11 commits into
dotnet:masterfrom
jkotas:generics-inlining

Conversation

@jkotas

@jkotasjkotas commented Jun 22, 2020

Copy link
Copy Markdown
Member

This change allows inlining of generic dictionary lookups when the caller and callee are the same exact type and instantiation. It is a simpler version of #31833.

Motivation for this change is to unblock #37941.

Example:

class G<T>
{
static bool M1() => typeof(T);
object M2() => new T[1];
bool M3() => Unsafe.SizeOf<T>();
static string M()
{
// Assume that T is string
// All M1, M2, M3 are inlined after this change, but were not inlined before this change
M1(); M2(); M3();
// Still not inlined - different instantiation, not the same exact type
G<List<T>>.M1();
// Still not inlined - different type.
OtherG<T>.M();
}
}

@jkotas
jkotas marked this pull request as draft June 22, 2020 19:22
@jkotas
jkotasforce-pushed the generics-inlining branch from 9645520 to dd93ffaCompareJune 23, 2020 06:58
@jaredparjaredpar mentioned this pull request Jun 23, 2020
@jkotas
jkotasforce-pushed the generics-inlining branch 3 times, most recently from 8cbb058 to 3770ae3CompareJune 24, 2020 16:20
Comment threadsrc/coreclr/src/ToolBox/superpmi/superpmi-shared/methodcontext.cpp Outdated
@jkotas
jkotasforce-pushed the generics-inlining branch from 3819aa0 to d09090eCompareJune 24, 2020 23:22
@jkotas

Copy link
Copy Markdown
MemberAuthor

Asm diffs

Total bytes of diff: 2738 (0.083% of base)
diff is a regression.
Top file regressions (bytes):
2738 : System.Private.CoreLib.dasm (0.083% of base)
1 total files with Code Size differences (0 improved, 1 regressed), 0 unchanged.
Top method regressions (bytes):
291 (41.044% of base) : System.Private.CoreLib.dasm - ValueTuple`3:System.IComparable.CompareTo(Object):int:this (3 methods)
260 (67.010% of base) : System.Private.CoreLib.dasm - ValueTuple`3:Equals(Object):bool:this (3 methods)
184 (3.340% of base) : System.Private.CoreLib.dasm - GenericArraySortHelper`1:IntroSort(Span`1,int) (12 methods)
173 (28.978% of base) : System.Private.CoreLib.dasm - ValueTask`1:ToString():String:this (3 methods)
159 (15.071% of base) : System.Private.CoreLib.dasm - Memory`1:CopyTo(Memory`1):this (4 methods)
159 (14.171% of base) : System.Private.CoreLib.dasm - Memory`1:TryCopyTo(Memory`1):bool:this (4 methods)
157 (69.778% of base) : System.Private.CoreLib.dasm - ValueTuple`4:Equals(Object):bool:this (2 methods)
118 (6.833% of base) : System.Private.CoreLib.dasm - Vector128`1:Equals(Object):bool:this (11 methods)
118 (3.562% of base) : System.Private.CoreLib.dasm - GenericArraySortHelper`1:PickPivotAndPartition(Span`1):int (12 methods)
94 (8.818% of base) : System.Private.CoreLib.dasm - ReadOnlyMemory`1:CopyTo(Memory`1):this (4 methods)
94 (8.297% of base) : System.Private.CoreLib.dasm - ReadOnlyMemory`1:TryCopyTo(Memory`1):bool:this (4 methods)
87 (13.344% of base) : System.Private.CoreLib.dasm - Memory`1:ToArray():ref:this (4 methods)
87 (14.846% of base) : System.Private.CoreLib.dasm - ReadOnlyMemory`1:ToArray():ref:this (4 methods)
76 (8.736% of base) : System.Private.CoreLib.dasm - List`1:System.Collections.IList.IndexOf(Object):int:this (8 methods)
75 (202.703% of base) : System.Private.CoreLib.dasm - List`1:Contains(KeyValuePair`2):bool:this
72 (39.560% of base) : System.Private.CoreLib.dasm - Lazy`1:CreateValue():__Canon:this
66 (129.412% of base) : System.Private.CoreLib.dasm - List`1:Remove(KeyValuePair`2):bool:this
63 (75.000% of base) : System.Private.CoreLib.dasm - TaskCompletionSource`1:.ctor(Object):this (2 methods)
62 (16.712% of base) : System.Private.CoreLib.dasm - ValueTuple`2:Equals(Object):bool:this (3 methods)
60 (8.310% of base) : System.Private.CoreLib.dasm - ValueTuple`2:System.IComparable.CompareTo(Object):int:this (3 methods)
Top method improvements (bytes):
-94 (-14.506% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Implicit(ref):ReadOnlySpan`1 (18 methods)
-73 (-8.120% of base) : System.Private.CoreLib.dasm - Memory`1:Equals(Object):bool:this (4 methods)
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector128`1:get_DisplayString():String:this
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector256`1:get_DisplayString():String:this
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector64`1:get_DisplayString():String:this
-44 (-6.790% of base) : System.Private.CoreLib.dasm - Span`1:op_Implicit(ref):Span`1 (18 methods)
-26 (-3.641% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Inequality(ReadOnlySpan`1,ReadOnlySpan`1):bool (18 methods)
-26 (-3.641% of base) : System.Private.CoreLib.dasm - Span`1:op_Inequality(Span`1,Span`1):bool (18 methods)
-15 (-0.957% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Implicit(ArraySegment`1):ReadOnlySpan`1 (18 methods)
-4 (-0.702% of base) : System.Private.CoreLib.dasm - AsyncValueTaskMethodBuilder`1:get_Task():ValueTask`1:this (2 methods)
-2 (-0.217% of base) : System.Private.CoreLib.dasm - ArraySegment`1:op_Inequality(ArraySegment`1,ArraySegment`1):bool (18 methods)
-2 (-0.930% of base) : System.Private.CoreLib.dasm - Memory`1:op_Implicit(ref):Memory`1 (4 methods)
Top method regressions (percentages):
47 (293.750% of base) : System.Private.CoreLib.dasm - SingleProducerSingleConsumerQueue`1:System.Collections.IEnumerable.GetEnumerator():IEnumerator:this
75 (202.703% of base) : System.Private.CoreLib.dasm - List`1:Contains(KeyValuePair`2):bool:this
66 (129.412% of base) : System.Private.CoreLib.dasm - List`1:Remove(KeyValuePair`2):bool:this
46 (124.324% of base) : System.Private.CoreLib.dasm - List`1:Contains(__Canon):bool:this
63 (75.000% of base) : System.Private.CoreLib.dasm - TaskCompletionSource`1:.ctor(Object):this (2 methods)
37 (72.549% of base) : System.Private.CoreLib.dasm - List`1:Remove(__Canon):bool:this
36 (72.000% of base) : System.Private.CoreLib.dasm - AsyncTaskMethodBuilder`1:SetNotificationForWaitCompletion(bool):this
59 (70.238% of base) : System.Private.CoreLib.dasm - TaskCompletionSource`1:.ctor(int):this (2 methods)
157 (69.778% of base) : System.Private.CoreLib.dasm - ValueTuple`4:Equals(Object):bool:this (2 methods)
260 (67.010% of base) : System.Private.CoreLib.dasm - ValueTuple`3:Equals(Object):bool:this (3 methods)
25 (43.103% of base) : System.Private.CoreLib.dasm - HashSet`1:System.Collections.Generic.IEnumerable<T>.GetEnumerator():IEnumerator`1:this
25 (43.103% of base) : System.Private.CoreLib.dasm - HashSet`1:System.Collections.IEnumerable.GetEnumerator():IEnumerator:this
291 (41.044% of base) : System.Private.CoreLib.dasm - ValueTuple`3:System.IComparable.CompareTo(Object):int:this (3 methods)
72 (39.560% of base) : System.Private.CoreLib.dasm - Lazy`1:CreateValue():__Canon:this
173 (28.978% of base) : System.Private.CoreLib.dasm - ValueTask`1:ToString():String:this (3 methods)
47 (27.485% of base) : System.Private.CoreLib.dasm - Lazy`1:ExecutionAndPublication(LazyHelper,bool):this
48 (19.753% of base) : System.Private.CoreLib.dasm - AsyncValueTaskMethodBuilder`1:SetException(Exception):this (2 methods)
18 (19.149% of base) : System.Private.CoreLib.dasm - Collection`1:System.Collections.IList.Contains(Object):bool:this
18 (18.947% of base) : System.Private.CoreLib.dasm - Collection`1:System.Collections.IList.IndexOf(Object):int:this
62 (16.712% of base) : System.Private.CoreLib.dasm - ValueTuple`2:Equals(Object):bool:this (3 methods)
Top method improvements (percentages):
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector128`1:get_DisplayString():String:this
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector256`1:get_DisplayString():String:this
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector64`1:get_DisplayString():String:this
-94 (-14.506% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Implicit(ref):ReadOnlySpan`1 (18 methods)
-73 (-8.120% of base) : System.Private.CoreLib.dasm - Memory`1:Equals(Object):bool:this (4 methods)
-44 (-6.790% of base) : System.Private.CoreLib.dasm - Span`1:op_Implicit(ref):Span`1 (18 methods)
-26 (-3.641% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Inequality(ReadOnlySpan`1,ReadOnlySpan`1):bool (18 methods)
-26 (-3.641% of base) : System.Private.CoreLib.dasm - Span`1:op_Inequality(Span`1,Span`1):bool (18 methods)
-15 (-0.957% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Implicit(ArraySegment`1):ReadOnlySpan`1 (18 methods)
-2 (-0.930% of base) : System.Private.CoreLib.dasm - Memory`1:op_Implicit(ref):Memory`1 (4 methods)
-4 (-0.702% of base) : System.Private.CoreLib.dasm - AsyncValueTaskMethodBuilder`1:get_Task():ValueTask`1:this (2 methods)
-2 (-0.217% of base) : System.Private.CoreLib.dasm - ArraySegment`1:op_Inequality(ArraySegment`1,ArraySegment`1):bool (18 methods)
57 total methods with Code Size differences (12 improved, 45 regressed), 20772 unchanged.

@jkotas

jkotas commented Jun 25, 2020

Copy link
Copy Markdown
MemberAuthor

Example of code-size regression - inlining tends to increase code size on average:

List`1:Contains(KeyValuePair`2):bool:this

Baseline

G_M57469_IG01:
sub rsp, 40
;; bbWeight=1 PerfScore 0.25
G_M57469_IG02:
cmp dword ptr [rcx+16], 0
je SHORT G_M57469_IG05
;; bbWeight=1 PerfScore 3.00
G_M57469_IG03:
call [List`1:IndexOf(KeyValuePair`2):int:this]
cmp eax, -1
setne al
movzx rax, al
;; bbWeight=0.50 PerfScore 2.25
G_M57469_IG04:
add rsp, 40
ret ;; bbWeight=0.50 PerfScore 0.63
G_M57469_IG05:
xor eax, eax
;; bbWeight=0.50 PerfScore 0.13
G_M57469_IG06:
add rsp, 40
ret 

Diff

G_M57469_IG01:
push rdi
push rsi
push rbx
sub rsp, 64
xor rax, rax
mov qword ptr [rsp+28H], rax
mov qword ptr [rsp+38H], rcx
mov rsi, rcx
;; bbWeight=1 PerfScore 5.75
G_M57469_IG02:
cmp dword ptr [rsi+16], 0
je SHORT G_M57469_IG05
;; bbWeight=1 PerfScore 3.00
G_M57469_IG03:
mov rdi, gword ptr [rdx]
mov ebx, dword ptr [rdx+8]
mov rcx, qword ptr [rsi]
call [CORINFO_HELP_READYTORUN_GENERIC_HANDLE]
mov rcx, rax
mov rdx, gword ptr [rsi+8]
lea r8, bword ptr [rsp+28H]
mov gword ptr [r8], rdi
mov dword ptr [r8+8], ebx
mov r8d, dword ptr [rsi+16]
mov dword ptr [rsp+20H], r8d
lea r8, bword ptr [rsp+28H]
xor r9d, r9d
call [Array:IndexOf(ref,KeyValuePair`2,int,int):int]
cmp eax, -1
setne al
movzx rax, al
;; bbWeight=0.50 PerfScore 11.00
G_M57469_IG04:
add rsp, 64
pop rbx
pop rsi
pop rdi
ret ;; bbWeight=0.50 PerfScore 1.38
G_M57469_IG05:
xor eax, eax
;; bbWeight=0.50 PerfScore 0.13
G_M57469_IG06:
add rsp, 64
pop rbx
pop rsi
pop rdi
ret 

@jkotas

Copy link
Copy Markdown
MemberAuthor

Example of code-size improvement - inlining reduces code size when it enables additional constant-prop:

Vector128`1:get_DisplayString

Baseline:

G_M30183_IG01:
push rdi
push rsi
sub rsp, 40
mov qword ptr [rsp+20H], rdx
mov rdi, rcx
mov rsi, rdx
;; bbWeight=1 PerfScore 3.75
G_M30183_IG02:
mov rcx, rsi
call [CORINFO_HELP_READYTORUN_GENERIC_HANDLE]
mov rcx, rax
call [Vector128`1:get_IsSupported():bool]
test al, al
je SHORT G_M30183_IG05
;; bbWeight=1 PerfScore 7.75
G_M30183_IG03:
mov rcx, rsi
call [CORINFO_HELP_READYTORUN_GENERIC_HANDLE]
mov rcx, rdi
call rax
nop ;; bbWeight=0.50 PerfScore 3.38
G_M30183_IG04:
add rsp, 40
pop rsi
pop rdi
ret ;; bbWeight=0.50 PerfScore 1.13
G_M30183_IG05:
mov rcx, qword ptr [(reloc)]
mov rcx, gword ptr [rcx]
mov rdx, qword ptr [(reloc)]
mov rdx, gword ptr [rdx]
call [SR:GetResourceString(String,String):String]
nop ;; bbWeight=0.50 PerfScore 5.63
G_M30183_IG06:
add rsp, 40
pop rsi
pop rdi
ret ;; bbWeight=0.50 PerfScore 1.13

Diff:

G_M30183_IG01:
sub rsp, 40
;; bbWeight=1 PerfScore 0.25
G_M30183_IG02:
mov rcx, qword ptr [(reloc)]
mov rcx, gword ptr [rcx]
mov rdx, qword ptr [(reloc)]
mov rdx, gword ptr [rdx]
call [SR:GetResourceString(String,String):String]
nop ;; bbWeight=1 PerfScore 11.25
G_M30183_IG03:
add rsp, 40
ret 

@nietras

Copy link
Copy Markdown
Contributor

The example improvements are exactly the kind I am hoping for 👍

@jkotas
jkotas marked this pull request as ready for review June 26, 2020 05:52
@jkotas

Copy link
Copy Markdown
MemberAuthor

@dotnet/crossgen-contrib Any crossgen2 specific legs you would recommend to run on this?

@dotnet/jit-contrib Any JIT specific legs you would recommend to run on this?

Comment threadsrc/coreclr/src/tools/Common/Sorting/MergeSortCore.cs
Comment threadsrc/coreclr/src/vm/jitinterface.cpp
@jkotasjkotas changed the title WIP: Enable inlining of shared generics code within same typeEnable inlining of shared generics code within same typeJun 26, 2020

@MichalStrehovskyMichalStrehovsky left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Crossgen2 side looks good to me.

I echo Manish's question about the assert - I think that code might be reachable.

Not commenting on the parallel sort thing because I didn't look at that code before.

@mangod9

Copy link
Copy Markdown
Member

@dotnet/crossgen-contrib Any crossgen2 specific legs you would recommend to run on this?

@dotnet/jit-contrib Any JIT specific legs you would recommend to run on this?

for crossgen2 would be good to run the runtime-coreclr crossgen2 pipeline for validation.

@jkotas

Copy link
Copy Markdown
MemberAuthor

/azp run runtime-coreclr crossgen2

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

@jkotas

Copy link
Copy Markdown
MemberAuthor

for crossgen2 would be good to run the runtime-coreclr crossgen2 pipeline for validation.

@mangod9 What is the right way to trigger it? I have tried to trigger it above, but it does not seem to work.

@mangod9

Copy link
Copy Markdown
Member

for crossgen2 would be good to run the runtime-coreclr crossgen2 pipeline for validation.

@mangod9 What is the right way to trigger it? I have tried to trigger it above, but it does not seem to work.

Hmm.. not sure why that pipeline command fails (probably some config for the pipeline @trylek ?). For now I have manually triggered it via AzDo portal

@BruceForstall

Copy link
Copy Markdown
Contributor

I thought (almost) no triggers were enabled, due to issues with accidentally triggering too many builds. Also, #32777

@davidwrighton

Copy link
Copy Markdown
Member

@jkotas, you can no longer use any of the pipelines from the AzDo command line. You need to find the pipeline in AzDo and manually start a build.

Comment threadsrc/coreclr/src/tools/Common/Sorting/MergeSortCore.cs

@AndyAyersMSAndyAyersMS left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Were those diffs from crossgen or PMI?

Might want to run a wider set of diffs (pass -f to jit-diff) and/or look diffs from both prejitting and jitting.

Comment threadsrc/coreclr/src/jit/importer.cpp
Comment threadsrc/coreclr/src/jit/importer.cpp

@davidwrightondavidwrighton left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like you to split out the work to address the static field address issue into a separate PR, and to provide a better description of under what conditions an inlined shared generic lookup will work and where it won't work.

@jkotasjkotas mentioned this pull request Jun 26, 2020
@jkotas

Copy link
Copy Markdown
MemberAuthor

I'd like you to split out the work to address the static field address issue into a separate PR, and to provide a better description of under what conditions an inlined shared generic lookup will work and where it won't work.

@davidwrighton Done. The updated description is at the top.

@BruceForstall

Copy link
Copy Markdown
Contributor

@dotnet/jit-contrib Any JIT specific legs you would recommend to run on this?

Seems like it would be worthwhile to runtime-coreclr jitstress (https://dev.azure.com/dnceng/public/_build?definitionId=658) to run Pri-1 tests with JitStress=1/2, which would include inlining stress.

@jkotas

Copy link
Copy Markdown
MemberAuthor

Might want to run a wider set of diffs

-f crossgen diffs:

Found 25 files with textual diffs.
Summary of Code Size diffs:
(Lower is better)
Total bytes of diff: 28141 (0.086% of base)
diff is a regression.
Total byte diff includes -111 bytes from reconciling methods
Base had 2 unique methods, 111 unique bytes
Diff had 0 unique methods, 0 unique bytes
Top file regressions (bytes):
12800 : System.Collections.Immutable.dasm (5.998% of base)
5146 : System.Memory.dasm (6.358% of base)
2738 : System.Private.CoreLib.dasm (0.083% of base)
2735 : System.Collections.dasm (2.930% of base)
714 : System.Collections.Concurrent.dasm (1.559% of base)
587 : Microsoft.CodeAnalysis.dasm (0.078% of base)
473 : System.Reflection.Metadata.dasm (0.147% of base)
468 : System.Linq.dasm (0.353% of base)
298 : System.Threading.Tasks.Dataflow.dasm (0.217% of base)
282 : System.Linq.Expressions.dasm (0.010% of base)
266 : System.ObjectModel.dasm (1.353% of base)
202 : Microsoft.CSharp.dasm (0.062% of base)
179 : System.DirectoryServices.AccountManagement.dasm (0.072% of base)
171 : Microsoft.Diagnostics.Tracing.TraceEvent.dasm (0.005% of base)
153 : Newtonsoft.Json.dasm (0.025% of base)
138 : Microsoft.Extensions.Logging.Abstractions.dasm (0.565% of base)
135 : Microsoft.Diagnostics.FastSerialization.dasm (0.401% of base)
126 : System.Private.Xml.dasm (0.004% of base)
116 : System.Linq.Parallel.dasm (0.020% of base)
108 : Microsoft.CodeAnalysis.CSharp.dasm (0.005% of base)
25 total files with Code Size differences (0 improved, 25 regressed), 239 unchanged.
Top method regressions (bytes):
554 (45.975% of base) : System.Memory.dasm - ReadOnlySequence`1:TryGet(byref,byref,bool):bool:this (3 methods)
543 (35.560% of base) : System.Memory.dasm - SequenceReader`1:TryReadTo(byref,__Canon,__Canon,bool):bool:this (2 methods)
437 (108.168% of base) : System.Collections.Immutable.dasm - Builder:System.Collections.IEnumerable.GetEnumerator():IEnumerator:this (8 methods)
422 (28.964% of base) : System.Memory.dasm - SequenceReader`1:TryReadToSlow(byref,__Canon,__Canon,int,bool):bool:this (2 methods)
347 (10.534% of base) : System.Collections.Immutable.dasm - Enumerator:MoveNext():bool:this (15 methods)
344 (148.276% of base) : System.Collections.Immutable.dasm - Node:System.Collections.IEnumerable.GetEnumerator():IEnumerator:this (4 methods)
336 (13.925% of base) : System.Memory.dasm - ReadOnlySequence`1:Slice(long,long):ReadOnlySequence`1:this (3 methods)
330 (23.827% of base) : System.Collections.Immutable.dasm - Enumerator:Reset():this (11 methods)
323 (21.824% of base) : System.Collections.Immutable.dasm - Builder:AddRange(IEnumerable`1):this (6 methods)
291 (31.699% of base) : System.Memory.dasm - ReadOnlySequence`1:Slice(long):ReadOnlySequence`1:this (3 methods)
291 (41.044% of base) : System.Private.CoreLib.dasm - ValueTuple`3:System.IComparable.CompareTo(Object):int:this (3 methods)
287 (38.013% of base) : System.Reflection.Metadata.dasm - SignatureDecoder`2:DecodeType(byref,bool,int):__Canon:this
286 (54.476% of base) : System.Memory.dasm - ReadOnlySequence`1:get_FirstSpan():ReadOnlySpan`1:this (3 methods)
281 (19.957% of base) : System.Memory.dasm - SequenceReader`1:TryReadTo(byref,ReadOnlySpan`1,bool):bool:this (2 methods)
275 (158.046% of base) : System.Collections.Immutable.dasm - Node:System.Collections.Generic.IEnumerable<T>.GetEnumerator():IEnumerator`1:this (3 methods)
268 (52.652% of base) : System.Memory.dasm - ReadOnlySequence`1:get_First():ReadOnlyMemory`1:this (3 methods)
268 (41.358% of base) : System.Memory.dasm - ReadOnlySequence`1:GetPosition(long,SequencePosition):SequencePosition:this (3 methods)
267 (41.654% of base) : System.Memory.dasm - ReadOnlySequence`1:GetPosition(long):SequencePosition:this (3 methods)
266 (95.341% of base) : System.Collections.Immutable.dasm - Builder:System.Collections.Generic.IEnumerable<T>.GetEnumerator():IEnumerator`1:this (6 methods)
265 (24.953% of base) : System.Collections.Immutable.dasm - Node:CopyTo(ref,int):this (3 methods)
Top method improvements (bytes):
-169 (-26.365% of base) : System.Collections.Immutable.dasm - ImmutableArray`1:InsertRange(int,ImmutableArray`1):ImmutableArray`1:this (2 methods)
-145 (-27.672% of base) : System.Collections.Immutable.dasm - Node:GetEnumerator():Enumerator:this (4 base, 3 diff methods)
-94 (-14.506% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Implicit(ref):ReadOnlySpan`1 (18 methods)
-82 (-100.000% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:CreatePair(__Canon,Pair`2):Pair`2 (1 base, 0 diff methods)
-73 (-8.120% of base) : System.Private.CoreLib.dasm - Memory`1:Equals(Object):bool:this (4 methods)
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector128`1:get_DisplayString():String:this
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector256`1:get_DisplayString():String:this
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector64`1:get_DisplayString():String:this
-56 (-29.474% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:Add(__Canon,int):bool:this
-52 (-27.083% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:Add(__Canon,__Canon):bool:this
-44 (-6.790% of base) : System.Private.CoreLib.dasm - Span`1:op_Implicit(ref):Span`1 (18 methods)
-42 (-7.420% of base) : System.Collections.Immutable.dasm - ImmutableArray`1:RemoveAll(Predicate`1):ImmutableArray`1:this (2 methods)
-32 (-46.377% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:.ctor(__Canon,int):this
-29 (-100.000% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:CreatePair(__Canon,int):Pair`2 (1 base, 0 diff methods)
-26 (-3.641% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Inequality(ReadOnlySpan`1,ReadOnlySpan`1):bool (18 methods)
-26 (-3.641% of base) : System.Private.CoreLib.dasm - Span`1:op_Inequality(Span`1,Span`1):bool (18 methods)
-22 (-31.884% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:.ctor(__Canon,__Canon):this
-21 (-19.444% of base) : System.Linq.Parallel.dasm - OrderedGroupJoinHashLookup:CreateValuePair(GroupKeyData):Pair`2:this
-19 (-11.515% of base) : Microsoft.CodeAnalysis.CSharp.dasm - SyntaxList`1:Any(ushort):bool:this
-19 (-13.475% of base) : System.Linq.dasm - SparseArrayBuilder`1:Reserve(int):this
Top method regressions (percentages):
126 (787.500% of base) : System.DirectoryServices.AccountManagement.dasm - TrackedCollection`1:System.Collections.IEnumerable.GetEnumerator():IEnumerator:this
92 (575.000% of base) : System.Collections.Immutable.dasm - ImmutableList`1:System.Collections.Immutable.IImmutableList<T>.Remove(KeyValuePair`2,IEqualityComparer`1):IImmutableList`1:this
63 (393.750% of base) : System.Collections.dasm - SortedList`2:get_Keys():IList`1:this
63 (393.750% of base) : System.Collections.dasm - SortedList`2:System.Collections.Generic.IDictionary<TKey,TValue>.get_Keys():ICollection`1:this
63 (393.750% of base) : System.Collections.dasm - SortedList`2:System.Collections.IDictionary.get_Keys():ICollection:this
63 (393.750% of base) : System.Collections.dasm - SortedList`2:System.Collections.Generic.IReadOnlyDictionary<TKey,TValue>.get_Keys():IEnumerable`1:this
63 (393.750% of base) : System.Collections.dasm - SortedList`2:get_Values():IList`1:this
63 (393.750% of base) : System.Collections.dasm - SortedList`2:System.Collections.Generic.IDictionary<TKey,TValue>.get_Values():ICollection`1:this
63 (393.750% of base) : System.Collections.dasm - SortedList`2:System.Collections.IDictionary.get_Values():ICollection:this
63 (393.750% of base) : System.Collections.dasm - SortedList`2:System.Collections.Generic.IReadOnlyDictionary<TKey,TValue>.get_Values():IEnumerable`1:this
62 (387.500% of base) : System.Collections.dasm - SortedDictionary`2:System.Collections.Generic.IDictionary<TKey,TValue>.get_Keys():ICollection`1:this
62 (387.500% of base) : System.Collections.dasm - SortedDictionary`2:System.Collections.Generic.IReadOnlyDictionary<TKey,TValue>.get_Keys():IEnumerable`1:this
62 (387.500% of base) : System.Collections.dasm - SortedDictionary`2:System.Collections.Generic.IDictionary<TKey,TValue>.get_Values():ICollection`1:this
62 (387.500% of base) : System.Collections.dasm - SortedDictionary`2:System.Collections.Generic.IReadOnlyDictionary<TKey,TValue>.get_Values():IEnumerable`1:this
62 (387.500% of base) : System.Collections.dasm - SortedDictionary`2:System.Collections.IDictionary.get_Keys():ICollection:this
62 (387.500% of base) : System.Collections.dasm - SortedDictionary`2:System.Collections.IDictionary.get_Values():ICollection:this
61 (381.250% of base) : Newtonsoft.Json.dasm - EnumerableDictionaryWrapper`2:System.Collections.IEnumerable.GetEnumerator():IEnumerator:this
61 (381.250% of base) : System.Net.Http.dasm - HttpHeaderValueCollection`1:System.Collections.IEnumerable.GetEnumerator():IEnumerator:this
60 (375.000% of base) : System.Collections.Immutable.dasm - ImmutableList`1:System.Collections.Immutable.IImmutableList<T>.Remove(__Canon,IEqualityComparer`1):IImmutableList`1:this
60 (375.000% of base) : System.Collections.Immutable.dasm - ImmutableStack`1:System.Collections.Immutable.IImmutableStack<T>.Push(__Canon):IImmutableStack`1:this
Top method improvements (percentages):
-82 (-100.000% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:CreatePair(__Canon,Pair`2):Pair`2 (1 base, 0 diff methods)
-29 (-100.000% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:CreatePair(__Canon,int):Pair`2 (1 base, 0 diff methods)
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector128`1:get_DisplayString():String:this
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector256`1:get_DisplayString():String:this
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector64`1:get_DisplayString():String:this
-32 (-46.377% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:.ctor(__Canon,int):this
-22 (-31.884% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:.ctor(__Canon,__Canon):this
-56 (-29.474% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:Add(__Canon,int):bool:this
-145 (-27.672% of base) : System.Collections.Immutable.dasm - Node:GetEnumerator():Enumerator:this (4 base, 3 diff methods)
-52 (-27.083% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:Add(__Canon,__Canon):bool:this
-169 (-26.365% of base) : System.Collections.Immutable.dasm - ImmutableArray`1:InsertRange(int,ImmutableArray`1):ImmutableArray`1:this (2 methods)
-21 (-19.444% of base) : System.Linq.Parallel.dasm - OrderedGroupJoinHashLookup:CreateValuePair(GroupKeyData):Pair`2:this
-94 (-14.506% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Implicit(ref):ReadOnlySpan`1 (18 methods)
-19 (-13.475% of base) : System.Linq.dasm - SparseArrayBuilder`1:Reserve(int):this
-18 (-12.950% of base) : System.Collections.Immutable.dasm - ImmutableDictionary`2:TryGetValue(__Canon,byref):bool:this
-18 (-12.950% of base) : System.Collections.Immutable.dasm - ImmutableDictionary`2:TryGetKey(__Canon,byref):bool:this
-9 (-12.857% of base) : Microsoft.CodeAnalysis.VisualBasic.dasm - _Closure$__17`1:.cctor()
-13 (-12.745% of base) : System.Collections.Immutable.dasm - ImmutableDictionary`2:Contains(KeyValuePair`2):bool:this
-19 (-11.515% of base) : Microsoft.CodeAnalysis.CSharp.dasm - SyntaxList`1:Any(ushort):bool:this
-13 (-10.484% of base) : System.Collections.Immutable.dasm - ImmutableDictionary`2:ContainsKey(__Canon):bool:this
406 total methods with Code Size differences (43 improved, 363 regressed), 186975 unchanged.

@jkotas

Copy link
Copy Markdown
MemberAuthor
  • ReadOnlySequence size regressions is caused by AggresiveInlining on ~70 lines method
  • ImmutableCollections size regression is a lots of small methods that are getting inlined into each other now

@jkotas

Copy link
Copy Markdown
MemberAuthor

JIT stress failure is known issue #36681
Installer Build and Test coreclr failure is known issue #35068
oreCLR Pri0 Runtime Tests Run Windows_NT x86 checked is known issue #38284

@jkotas
jkotas merged commit 31699de into dotnet:masterJun 27, 2020
@jkotas
jkotas deleted the generics-inlining branch June 27, 2020 13:36
@nietras

Copy link
Copy Markdown
Contributor

Great to see this merged. 👍

when the caller and callee are the same exact type and instantiation.

This is quite clear but to be sure, I assume this means this won't work for base class methods for example? E. g. called in a derived class?

@jkotas

Copy link
Copy Markdown
MemberAuthor

I assume this means this won't work for base class methods for example? E. g. called in a derived class?

Correct.

@ghostghost locked as resolved and limited conversation to collaborators Dec 8, 2020
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

11 participants

@jkotas@nietras@mangod9@BruceForstall@davidwrighton@EgorBo@stephentoub@jakobbotsch@AndyAyersMS@MichalStrehovsky@Dotnet-GitSync-Bot
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

Enable inlining of shared generics code within same type - #38229

Merged
jkotas merged 11 commits into
dotnet:masterfrom
jkotas:generics-inlining
Jun 27, 2020
Merged

Enable inlining of shared generics code within same type#38229
jkotas merged 11 commits into
dotnet:masterfrom
jkotas:generics-inlining

Conversation

@jkotas

@jkotasjkotas commented Jun 22, 2020

Copy link
Copy Markdown
Member

This change allows inlining of generic dictionary lookups when the caller and callee are the same exact type and instantiation. It is a simpler version of #31833.

Motivation for this change is to unblock #37941.

Example:

class G<T>
{
static bool M1() => typeof(T);
object M2() => new T[1];
bool M3() => Unsafe.SizeOf<T>();
static string M()
{
// Assume that T is string
// All M1, M2, M3 are inlined after this change, but were not inlined before this change
M1(); M2(); M3();
// Still not inlined - different instantiation, not the same exact type
G<List<T>>.M1();
// Still not inlined - different type.
OtherG<T>.M();
}
}

@jkotas
jkotas marked this pull request as draft June 22, 2020 19:22
@jkotas
jkotasforce-pushed the generics-inlining branch from 9645520 to dd93ffaCompareJune 23, 2020 06:58
@jaredparjaredpar mentioned this pull request Jun 23, 2020
@jkotas
jkotasforce-pushed the generics-inlining branch 3 times, most recently from 8cbb058 to 3770ae3CompareJune 24, 2020 16:20
Comment threadsrc/coreclr/src/ToolBox/superpmi/superpmi-shared/methodcontext.cpp Outdated
@jkotas
jkotasforce-pushed the generics-inlining branch from 3819aa0 to d09090eCompareJune 24, 2020 23:22
@jkotas

Copy link
Copy Markdown
MemberAuthor

Asm diffs

Total bytes of diff: 2738 (0.083% of base)
diff is a regression.
Top file regressions (bytes):
2738 : System.Private.CoreLib.dasm (0.083% of base)
1 total files with Code Size differences (0 improved, 1 regressed), 0 unchanged.
Top method regressions (bytes):
291 (41.044% of base) : System.Private.CoreLib.dasm - ValueTuple`3:System.IComparable.CompareTo(Object):int:this (3 methods)
260 (67.010% of base) : System.Private.CoreLib.dasm - ValueTuple`3:Equals(Object):bool:this (3 methods)
184 (3.340% of base) : System.Private.CoreLib.dasm - GenericArraySortHelper`1:IntroSort(Span`1,int) (12 methods)
173 (28.978% of base) : System.Private.CoreLib.dasm - ValueTask`1:ToString():String:this (3 methods)
159 (15.071% of base) : System.Private.CoreLib.dasm - Memory`1:CopyTo(Memory`1):this (4 methods)
159 (14.171% of base) : System.Private.CoreLib.dasm - Memory`1:TryCopyTo(Memory`1):bool:this (4 methods)
157 (69.778% of base) : System.Private.CoreLib.dasm - ValueTuple`4:Equals(Object):bool:this (2 methods)
118 (6.833% of base) : System.Private.CoreLib.dasm - Vector128`1:Equals(Object):bool:this (11 methods)
118 (3.562% of base) : System.Private.CoreLib.dasm - GenericArraySortHelper`1:PickPivotAndPartition(Span`1):int (12 methods)
94 (8.818% of base) : System.Private.CoreLib.dasm - ReadOnlyMemory`1:CopyTo(Memory`1):this (4 methods)
94 (8.297% of base) : System.Private.CoreLib.dasm - ReadOnlyMemory`1:TryCopyTo(Memory`1):bool:this (4 methods)
87 (13.344% of base) : System.Private.CoreLib.dasm - Memory`1:ToArray():ref:this (4 methods)
87 (14.846% of base) : System.Private.CoreLib.dasm - ReadOnlyMemory`1:ToArray():ref:this (4 methods)
76 (8.736% of base) : System.Private.CoreLib.dasm - List`1:System.Collections.IList.IndexOf(Object):int:this (8 methods)
75 (202.703% of base) : System.Private.CoreLib.dasm - List`1:Contains(KeyValuePair`2):bool:this
72 (39.560% of base) : System.Private.CoreLib.dasm - Lazy`1:CreateValue():__Canon:this
66 (129.412% of base) : System.Private.CoreLib.dasm - List`1:Remove(KeyValuePair`2):bool:this
63 (75.000% of base) : System.Private.CoreLib.dasm - TaskCompletionSource`1:.ctor(Object):this (2 methods)
62 (16.712% of base) : System.Private.CoreLib.dasm - ValueTuple`2:Equals(Object):bool:this (3 methods)
60 (8.310% of base) : System.Private.CoreLib.dasm - ValueTuple`2:System.IComparable.CompareTo(Object):int:this (3 methods)
Top method improvements (bytes):
-94 (-14.506% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Implicit(ref):ReadOnlySpan`1 (18 methods)
-73 (-8.120% of base) : System.Private.CoreLib.dasm - Memory`1:Equals(Object):bool:this (4 methods)
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector128`1:get_DisplayString():String:this
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector256`1:get_DisplayString():String:this
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector64`1:get_DisplayString():String:this
-44 (-6.790% of base) : System.Private.CoreLib.dasm - Span`1:op_Implicit(ref):Span`1 (18 methods)
-26 (-3.641% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Inequality(ReadOnlySpan`1,ReadOnlySpan`1):bool (18 methods)
-26 (-3.641% of base) : System.Private.CoreLib.dasm - Span`1:op_Inequality(Span`1,Span`1):bool (18 methods)
-15 (-0.957% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Implicit(ArraySegment`1):ReadOnlySpan`1 (18 methods)
-4 (-0.702% of base) : System.Private.CoreLib.dasm - AsyncValueTaskMethodBuilder`1:get_Task():ValueTask`1:this (2 methods)
-2 (-0.217% of base) : System.Private.CoreLib.dasm - ArraySegment`1:op_Inequality(ArraySegment`1,ArraySegment`1):bool (18 methods)
-2 (-0.930% of base) : System.Private.CoreLib.dasm - Memory`1:op_Implicit(ref):Memory`1 (4 methods)
Top method regressions (percentages):
47 (293.750% of base) : System.Private.CoreLib.dasm - SingleProducerSingleConsumerQueue`1:System.Collections.IEnumerable.GetEnumerator():IEnumerator:this
75 (202.703% of base) : System.Private.CoreLib.dasm - List`1:Contains(KeyValuePair`2):bool:this
66 (129.412% of base) : System.Private.CoreLib.dasm - List`1:Remove(KeyValuePair`2):bool:this
46 (124.324% of base) : System.Private.CoreLib.dasm - List`1:Contains(__Canon):bool:this
63 (75.000% of base) : System.Private.CoreLib.dasm - TaskCompletionSource`1:.ctor(Object):this (2 methods)
37 (72.549% of base) : System.Private.CoreLib.dasm - List`1:Remove(__Canon):bool:this
36 (72.000% of base) : System.Private.CoreLib.dasm - AsyncTaskMethodBuilder`1:SetNotificationForWaitCompletion(bool):this
59 (70.238% of base) : System.Private.CoreLib.dasm - TaskCompletionSource`1:.ctor(int):this (2 methods)
157 (69.778% of base) : System.Private.CoreLib.dasm - ValueTuple`4:Equals(Object):bool:this (2 methods)
260 (67.010% of base) : System.Private.CoreLib.dasm - ValueTuple`3:Equals(Object):bool:this (3 methods)
25 (43.103% of base) : System.Private.CoreLib.dasm - HashSet`1:System.Collections.Generic.IEnumerable<T>.GetEnumerator():IEnumerator`1:this
25 (43.103% of base) : System.Private.CoreLib.dasm - HashSet`1:System.Collections.IEnumerable.GetEnumerator():IEnumerator:this
291 (41.044% of base) : System.Private.CoreLib.dasm - ValueTuple`3:System.IComparable.CompareTo(Object):int:this (3 methods)
72 (39.560% of base) : System.Private.CoreLib.dasm - Lazy`1:CreateValue():__Canon:this
173 (28.978% of base) : System.Private.CoreLib.dasm - ValueTask`1:ToString():String:this (3 methods)
47 (27.485% of base) : System.Private.CoreLib.dasm - Lazy`1:ExecutionAndPublication(LazyHelper,bool):this
48 (19.753% of base) : System.Private.CoreLib.dasm - AsyncValueTaskMethodBuilder`1:SetException(Exception):this (2 methods)
18 (19.149% of base) : System.Private.CoreLib.dasm - Collection`1:System.Collections.IList.Contains(Object):bool:this
18 (18.947% of base) : System.Private.CoreLib.dasm - Collection`1:System.Collections.IList.IndexOf(Object):int:this
62 (16.712% of base) : System.Private.CoreLib.dasm - ValueTuple`2:Equals(Object):bool:this (3 methods)
Top method improvements (percentages):
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector128`1:get_DisplayString():String:this
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector256`1:get_DisplayString():String:this
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector64`1:get_DisplayString():String:this
-94 (-14.506% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Implicit(ref):ReadOnlySpan`1 (18 methods)
-73 (-8.120% of base) : System.Private.CoreLib.dasm - Memory`1:Equals(Object):bool:this (4 methods)
-44 (-6.790% of base) : System.Private.CoreLib.dasm - Span`1:op_Implicit(ref):Span`1 (18 methods)
-26 (-3.641% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Inequality(ReadOnlySpan`1,ReadOnlySpan`1):bool (18 methods)
-26 (-3.641% of base) : System.Private.CoreLib.dasm - Span`1:op_Inequality(Span`1,Span`1):bool (18 methods)
-15 (-0.957% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Implicit(ArraySegment`1):ReadOnlySpan`1 (18 methods)
-2 (-0.930% of base) : System.Private.CoreLib.dasm - Memory`1:op_Implicit(ref):Memory`1 (4 methods)
-4 (-0.702% of base) : System.Private.CoreLib.dasm - AsyncValueTaskMethodBuilder`1:get_Task():ValueTask`1:this (2 methods)
-2 (-0.217% of base) : System.Private.CoreLib.dasm - ArraySegment`1:op_Inequality(ArraySegment`1,ArraySegment`1):bool (18 methods)
57 total methods with Code Size differences (12 improved, 45 regressed), 20772 unchanged.

@jkotas

jkotas commented Jun 25, 2020

Copy link
Copy Markdown
MemberAuthor

Example of code-size regression - inlining tends to increase code size on average:

List`1:Contains(KeyValuePair`2):bool:this

Baseline

G_M57469_IG01:
sub rsp, 40
;; bbWeight=1 PerfScore 0.25
G_M57469_IG02:
cmp dword ptr [rcx+16], 0
je SHORT G_M57469_IG05
;; bbWeight=1 PerfScore 3.00
G_M57469_IG03:
call [List`1:IndexOf(KeyValuePair`2):int:this]
cmp eax, -1
setne al
movzx rax, al
;; bbWeight=0.50 PerfScore 2.25
G_M57469_IG04:
add rsp, 40
ret ;; bbWeight=0.50 PerfScore 0.63
G_M57469_IG05:
xor eax, eax
;; bbWeight=0.50 PerfScore 0.13
G_M57469_IG06:
add rsp, 40
ret 

Diff

G_M57469_IG01:
push rdi
push rsi
push rbx
sub rsp, 64
xor rax, rax
mov qword ptr [rsp+28H], rax
mov qword ptr [rsp+38H], rcx
mov rsi, rcx
;; bbWeight=1 PerfScore 5.75
G_M57469_IG02:
cmp dword ptr [rsi+16], 0
je SHORT G_M57469_IG05
;; bbWeight=1 PerfScore 3.00
G_M57469_IG03:
mov rdi, gword ptr [rdx]
mov ebx, dword ptr [rdx+8]
mov rcx, qword ptr [rsi]
call [CORINFO_HELP_READYTORUN_GENERIC_HANDLE]
mov rcx, rax
mov rdx, gword ptr [rsi+8]
lea r8, bword ptr [rsp+28H]
mov gword ptr [r8], rdi
mov dword ptr [r8+8], ebx
mov r8d, dword ptr [rsi+16]
mov dword ptr [rsp+20H], r8d
lea r8, bword ptr [rsp+28H]
xor r9d, r9d
call [Array:IndexOf(ref,KeyValuePair`2,int,int):int]
cmp eax, -1
setne al
movzx rax, al
;; bbWeight=0.50 PerfScore 11.00
G_M57469_IG04:
add rsp, 64
pop rbx
pop rsi
pop rdi
ret ;; bbWeight=0.50 PerfScore 1.38
G_M57469_IG05:
xor eax, eax
;; bbWeight=0.50 PerfScore 0.13
G_M57469_IG06:
add rsp, 64
pop rbx
pop rsi
pop rdi
ret 

@jkotas

Copy link
Copy Markdown
MemberAuthor

Example of code-size improvement - inlining reduces code size when it enables additional constant-prop:

Vector128`1:get_DisplayString

Baseline:

G_M30183_IG01:
push rdi
push rsi
sub rsp, 40
mov qword ptr [rsp+20H], rdx
mov rdi, rcx
mov rsi, rdx
;; bbWeight=1 PerfScore 3.75
G_M30183_IG02:
mov rcx, rsi
call [CORINFO_HELP_READYTORUN_GENERIC_HANDLE]
mov rcx, rax
call [Vector128`1:get_IsSupported():bool]
test al, al
je SHORT G_M30183_IG05
;; bbWeight=1 PerfScore 7.75
G_M30183_IG03:
mov rcx, rsi
call [CORINFO_HELP_READYTORUN_GENERIC_HANDLE]
mov rcx, rdi
call rax
nop ;; bbWeight=0.50 PerfScore 3.38
G_M30183_IG04:
add rsp, 40
pop rsi
pop rdi
ret ;; bbWeight=0.50 PerfScore 1.13
G_M30183_IG05:
mov rcx, qword ptr [(reloc)]
mov rcx, gword ptr [rcx]
mov rdx, qword ptr [(reloc)]
mov rdx, gword ptr [rdx]
call [SR:GetResourceString(String,String):String]
nop ;; bbWeight=0.50 PerfScore 5.63
G_M30183_IG06:
add rsp, 40
pop rsi
pop rdi
ret ;; bbWeight=0.50 PerfScore 1.13

Diff:

G_M30183_IG01:
sub rsp, 40
;; bbWeight=1 PerfScore 0.25
G_M30183_IG02:
mov rcx, qword ptr [(reloc)]
mov rcx, gword ptr [rcx]
mov rdx, qword ptr [(reloc)]
mov rdx, gword ptr [rdx]
call [SR:GetResourceString(String,String):String]
nop ;; bbWeight=1 PerfScore 11.25
G_M30183_IG03:
add rsp, 40
ret 

@nietras

Copy link
Copy Markdown
Contributor

The example improvements are exactly the kind I am hoping for 👍

@jkotas
jkotas marked this pull request as ready for review June 26, 2020 05:52
@jkotas

Copy link
Copy Markdown
MemberAuthor

@dotnet/crossgen-contrib Any crossgen2 specific legs you would recommend to run on this?

@dotnet/jit-contrib Any JIT specific legs you would recommend to run on this?

Comment threadsrc/coreclr/src/tools/Common/Sorting/MergeSortCore.cs
Comment threadsrc/coreclr/src/vm/jitinterface.cpp
@jkotasjkotas changed the title WIP: Enable inlining of shared generics code within same typeEnable inlining of shared generics code within same typeJun 26, 2020

@MichalStrehovskyMichalStrehovsky left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Crossgen2 side looks good to me.

I echo Manish's question about the assert - I think that code might be reachable.

Not commenting on the parallel sort thing because I didn't look at that code before.

@mangod9

Copy link
Copy Markdown
Member

@dotnet/crossgen-contrib Any crossgen2 specific legs you would recommend to run on this?

@dotnet/jit-contrib Any JIT specific legs you would recommend to run on this?

for crossgen2 would be good to run the runtime-coreclr crossgen2 pipeline for validation.

@jkotas

Copy link
Copy Markdown
MemberAuthor

/azp run runtime-coreclr crossgen2

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

@jkotas

Copy link
Copy Markdown
MemberAuthor

for crossgen2 would be good to run the runtime-coreclr crossgen2 pipeline for validation.

@mangod9 What is the right way to trigger it? I have tried to trigger it above, but it does not seem to work.

@mangod9

Copy link
Copy Markdown
Member

for crossgen2 would be good to run the runtime-coreclr crossgen2 pipeline for validation.

@mangod9 What is the right way to trigger it? I have tried to trigger it above, but it does not seem to work.

Hmm.. not sure why that pipeline command fails (probably some config for the pipeline @trylek ?). For now I have manually triggered it via AzDo portal

@BruceForstall

Copy link
Copy Markdown
Contributor

I thought (almost) no triggers were enabled, due to issues with accidentally triggering too many builds. Also, #32777

@davidwrighton

Copy link
Copy Markdown
Member

@jkotas, you can no longer use any of the pipelines from the AzDo command line. You need to find the pipeline in AzDo and manually start a build.

Comment threadsrc/coreclr/src/tools/Common/Sorting/MergeSortCore.cs

@AndyAyersMSAndyAyersMS left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Were those diffs from crossgen or PMI?

Might want to run a wider set of diffs (pass -f to jit-diff) and/or look diffs from both prejitting and jitting.

Comment threadsrc/coreclr/src/jit/importer.cpp
Comment threadsrc/coreclr/src/jit/importer.cpp

@davidwrightondavidwrighton left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like you to split out the work to address the static field address issue into a separate PR, and to provide a better description of under what conditions an inlined shared generic lookup will work and where it won't work.

@jkotasjkotas mentioned this pull request Jun 26, 2020
@jkotas

Copy link
Copy Markdown
MemberAuthor

I'd like you to split out the work to address the static field address issue into a separate PR, and to provide a better description of under what conditions an inlined shared generic lookup will work and where it won't work.

@davidwrighton Done. The updated description is at the top.

@BruceForstall

Copy link
Copy Markdown
Contributor

@dotnet/jit-contrib Any JIT specific legs you would recommend to run on this?

Seems like it would be worthwhile to runtime-coreclr jitstress (https://dev.azure.com/dnceng/public/_build?definitionId=658) to run Pri-1 tests with JitStress=1/2, which would include inlining stress.

@jkotas

Copy link
Copy Markdown
MemberAuthor

Might want to run a wider set of diffs

-f crossgen diffs:

Found 25 files with textual diffs.
Summary of Code Size diffs:
(Lower is better)
Total bytes of diff: 28141 (0.086% of base)
diff is a regression.
Total byte diff includes -111 bytes from reconciling methods
Base had 2 unique methods, 111 unique bytes
Diff had 0 unique methods, 0 unique bytes
Top file regressions (bytes):
12800 : System.Collections.Immutable.dasm (5.998% of base)
5146 : System.Memory.dasm (6.358% of base)
2738 : System.Private.CoreLib.dasm (0.083% of base)
2735 : System.Collections.dasm (2.930% of base)
714 : System.Collections.Concurrent.dasm (1.559% of base)
587 : Microsoft.CodeAnalysis.dasm (0.078% of base)
473 : System.Reflection.Metadata.dasm (0.147% of base)
468 : System.Linq.dasm (0.353% of base)
298 : System.Threading.Tasks.Dataflow.dasm (0.217% of base)
282 : System.Linq.Expressions.dasm (0.010% of base)
266 : System.ObjectModel.dasm (1.353% of base)
202 : Microsoft.CSharp.dasm (0.062% of base)
179 : System.DirectoryServices.AccountManagement.dasm (0.072% of base)
171 : Microsoft.Diagnostics.Tracing.TraceEvent.dasm (0.005% of base)
153 : Newtonsoft.Json.dasm (0.025% of base)
138 : Microsoft.Extensions.Logging.Abstractions.dasm (0.565% of base)
135 : Microsoft.Diagnostics.FastSerialization.dasm (0.401% of base)
126 : System.Private.Xml.dasm (0.004% of base)
116 : System.Linq.Parallel.dasm (0.020% of base)
108 : Microsoft.CodeAnalysis.CSharp.dasm (0.005% of base)
25 total files with Code Size differences (0 improved, 25 regressed), 239 unchanged.
Top method regressions (bytes):
554 (45.975% of base) : System.Memory.dasm - ReadOnlySequence`1:TryGet(byref,byref,bool):bool:this (3 methods)
543 (35.560% of base) : System.Memory.dasm - SequenceReader`1:TryReadTo(byref,__Canon,__Canon,bool):bool:this (2 methods)
437 (108.168% of base) : System.Collections.Immutable.dasm - Builder:System.Collections.IEnumerable.GetEnumerator():IEnumerator:this (8 methods)
422 (28.964% of base) : System.Memory.dasm - SequenceReader`1:TryReadToSlow(byref,__Canon,__Canon,int,bool):bool:this (2 methods)
347 (10.534% of base) : System.Collections.Immutable.dasm - Enumerator:MoveNext():bool:this (15 methods)
344 (148.276% of base) : System.Collections.Immutable.dasm - Node:System.Collections.IEnumerable.GetEnumerator():IEnumerator:this (4 methods)
336 (13.925% of base) : System.Memory.dasm - ReadOnlySequence`1:Slice(long,long):ReadOnlySequence`1:this (3 methods)
330 (23.827% of base) : System.Collections.Immutable.dasm - Enumerator:Reset():this (11 methods)
323 (21.824% of base) : System.Collections.Immutable.dasm - Builder:AddRange(IEnumerable`1):this (6 methods)
291 (31.699% of base) : System.Memory.dasm - ReadOnlySequence`1:Slice(long):ReadOnlySequence`1:this (3 methods)
291 (41.044% of base) : System.Private.CoreLib.dasm - ValueTuple`3:System.IComparable.CompareTo(Object):int:this (3 methods)
287 (38.013% of base) : System.Reflection.Metadata.dasm - SignatureDecoder`2:DecodeType(byref,bool,int):__Canon:this
286 (54.476% of base) : System.Memory.dasm - ReadOnlySequence`1:get_FirstSpan():ReadOnlySpan`1:this (3 methods)
281 (19.957% of base) : System.Memory.dasm - SequenceReader`1:TryReadTo(byref,ReadOnlySpan`1,bool):bool:this (2 methods)
275 (158.046% of base) : System.Collections.Immutable.dasm - Node:System.Collections.Generic.IEnumerable<T>.GetEnumerator():IEnumerator`1:this (3 methods)
268 (52.652% of base) : System.Memory.dasm - ReadOnlySequence`1:get_First():ReadOnlyMemory`1:this (3 methods)
268 (41.358% of base) : System.Memory.dasm - ReadOnlySequence`1:GetPosition(long,SequencePosition):SequencePosition:this (3 methods)
267 (41.654% of base) : System.Memory.dasm - ReadOnlySequence`1:GetPosition(long):SequencePosition:this (3 methods)
266 (95.341% of base) : System.Collections.Immutable.dasm - Builder:System.Collections.Generic.IEnumerable<T>.GetEnumerator():IEnumerator`1:this (6 methods)
265 (24.953% of base) : System.Collections.Immutable.dasm - Node:CopyTo(ref,int):this (3 methods)
Top method improvements (bytes):
-169 (-26.365% of base) : System.Collections.Immutable.dasm - ImmutableArray`1:InsertRange(int,ImmutableArray`1):ImmutableArray`1:this (2 methods)
-145 (-27.672% of base) : System.Collections.Immutable.dasm - Node:GetEnumerator():Enumerator:this (4 base, 3 diff methods)
-94 (-14.506% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Implicit(ref):ReadOnlySpan`1 (18 methods)
-82 (-100.000% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:CreatePair(__Canon,Pair`2):Pair`2 (1 base, 0 diff methods)
-73 (-8.120% of base) : System.Private.CoreLib.dasm - Memory`1:Equals(Object):bool:this (4 methods)
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector128`1:get_DisplayString():String:this
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector256`1:get_DisplayString():String:this
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector64`1:get_DisplayString():String:this
-56 (-29.474% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:Add(__Canon,int):bool:this
-52 (-27.083% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:Add(__Canon,__Canon):bool:this
-44 (-6.790% of base) : System.Private.CoreLib.dasm - Span`1:op_Implicit(ref):Span`1 (18 methods)
-42 (-7.420% of base) : System.Collections.Immutable.dasm - ImmutableArray`1:RemoveAll(Predicate`1):ImmutableArray`1:this (2 methods)
-32 (-46.377% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:.ctor(__Canon,int):this
-29 (-100.000% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:CreatePair(__Canon,int):Pair`2 (1 base, 0 diff methods)
-26 (-3.641% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Inequality(ReadOnlySpan`1,ReadOnlySpan`1):bool (18 methods)
-26 (-3.641% of base) : System.Private.CoreLib.dasm - Span`1:op_Inequality(Span`1,Span`1):bool (18 methods)
-22 (-31.884% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:.ctor(__Canon,__Canon):this
-21 (-19.444% of base) : System.Linq.Parallel.dasm - OrderedGroupJoinHashLookup:CreateValuePair(GroupKeyData):Pair`2:this
-19 (-11.515% of base) : Microsoft.CodeAnalysis.CSharp.dasm - SyntaxList`1:Any(ushort):bool:this
-19 (-13.475% of base) : System.Linq.dasm - SparseArrayBuilder`1:Reserve(int):this
Top method regressions (percentages):
126 (787.500% of base) : System.DirectoryServices.AccountManagement.dasm - TrackedCollection`1:System.Collections.IEnumerable.GetEnumerator():IEnumerator:this
92 (575.000% of base) : System.Collections.Immutable.dasm - ImmutableList`1:System.Collections.Immutable.IImmutableList<T>.Remove(KeyValuePair`2,IEqualityComparer`1):IImmutableList`1:this
63 (393.750% of base) : System.Collections.dasm - SortedList`2:get_Keys():IList`1:this
63 (393.750% of base) : System.Collections.dasm - SortedList`2:System.Collections.Generic.IDictionary<TKey,TValue>.get_Keys():ICollection`1:this
63 (393.750% of base) : System.Collections.dasm - SortedList`2:System.Collections.IDictionary.get_Keys():ICollection:this
63 (393.750% of base) : System.Collections.dasm - SortedList`2:System.Collections.Generic.IReadOnlyDictionary<TKey,TValue>.get_Keys():IEnumerable`1:this
63 (393.750% of base) : System.Collections.dasm - SortedList`2:get_Values():IList`1:this
63 (393.750% of base) : System.Collections.dasm - SortedList`2:System.Collections.Generic.IDictionary<TKey,TValue>.get_Values():ICollection`1:this
63 (393.750% of base) : System.Collections.dasm - SortedList`2:System.Collections.IDictionary.get_Values():ICollection:this
63 (393.750% of base) : System.Collections.dasm - SortedList`2:System.Collections.Generic.IReadOnlyDictionary<TKey,TValue>.get_Values():IEnumerable`1:this
62 (387.500% of base) : System.Collections.dasm - SortedDictionary`2:System.Collections.Generic.IDictionary<TKey,TValue>.get_Keys():ICollection`1:this
62 (387.500% of base) : System.Collections.dasm - SortedDictionary`2:System.Collections.Generic.IReadOnlyDictionary<TKey,TValue>.get_Keys():IEnumerable`1:this
62 (387.500% of base) : System.Collections.dasm - SortedDictionary`2:System.Collections.Generic.IDictionary<TKey,TValue>.get_Values():ICollection`1:this
62 (387.500% of base) : System.Collections.dasm - SortedDictionary`2:System.Collections.Generic.IReadOnlyDictionary<TKey,TValue>.get_Values():IEnumerable`1:this
62 (387.500% of base) : System.Collections.dasm - SortedDictionary`2:System.Collections.IDictionary.get_Keys():ICollection:this
62 (387.500% of base) : System.Collections.dasm - SortedDictionary`2:System.Collections.IDictionary.get_Values():ICollection:this
61 (381.250% of base) : Newtonsoft.Json.dasm - EnumerableDictionaryWrapper`2:System.Collections.IEnumerable.GetEnumerator():IEnumerator:this
61 (381.250% of base) : System.Net.Http.dasm - HttpHeaderValueCollection`1:System.Collections.IEnumerable.GetEnumerator():IEnumerator:this
60 (375.000% of base) : System.Collections.Immutable.dasm - ImmutableList`1:System.Collections.Immutable.IImmutableList<T>.Remove(__Canon,IEqualityComparer`1):IImmutableList`1:this
60 (375.000% of base) : System.Collections.Immutable.dasm - ImmutableStack`1:System.Collections.Immutable.IImmutableStack<T>.Push(__Canon):IImmutableStack`1:this
Top method improvements (percentages):
-82 (-100.000% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:CreatePair(__Canon,Pair`2):Pair`2 (1 base, 0 diff methods)
-29 (-100.000% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:CreatePair(__Canon,int):Pair`2 (1 base, 0 diff methods)
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector128`1:get_DisplayString():String:this
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector256`1:get_DisplayString():String:this
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector64`1:get_DisplayString():String:this
-32 (-46.377% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:.ctor(__Canon,int):this
-22 (-31.884% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:.ctor(__Canon,__Canon):this
-56 (-29.474% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:Add(__Canon,int):bool:this
-145 (-27.672% of base) : System.Collections.Immutable.dasm - Node:GetEnumerator():Enumerator:this (4 base, 3 diff methods)
-52 (-27.083% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:Add(__Canon,__Canon):bool:this
-169 (-26.365% of base) : System.Collections.Immutable.dasm - ImmutableArray`1:InsertRange(int,ImmutableArray`1):ImmutableArray`1:this (2 methods)
-21 (-19.444% of base) : System.Linq.Parallel.dasm - OrderedGroupJoinHashLookup:CreateValuePair(GroupKeyData):Pair`2:this
-94 (-14.506% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Implicit(ref):ReadOnlySpan`1 (18 methods)
-19 (-13.475% of base) : System.Linq.dasm - SparseArrayBuilder`1:Reserve(int):this
-18 (-12.950% of base) : System.Collections.Immutable.dasm - ImmutableDictionary`2:TryGetValue(__Canon,byref):bool:this
-18 (-12.950% of base) : System.Collections.Immutable.dasm - ImmutableDictionary`2:TryGetKey(__Canon,byref):bool:this
-9 (-12.857% of base) : Microsoft.CodeAnalysis.VisualBasic.dasm - _Closure$__17`1:.cctor()
-13 (-12.745% of base) : System.Collections.Immutable.dasm - ImmutableDictionary`2:Contains(KeyValuePair`2):bool:this
-19 (-11.515% of base) : Microsoft.CodeAnalysis.CSharp.dasm - SyntaxList`1:Any(ushort):bool:this
-13 (-10.484% of base) : System.Collections.Immutable.dasm - ImmutableDictionary`2:ContainsKey(__Canon):bool:this
406 total methods with Code Size differences (43 improved, 363 regressed), 186975 unchanged.

@jkotas

Copy link
Copy Markdown
MemberAuthor
  • ReadOnlySequence size regressions is caused by AggresiveInlining on ~70 lines method
  • ImmutableCollections size regression is a lots of small methods that are getting inlined into each other now

@jkotas

Copy link
Copy Markdown
MemberAuthor

JIT stress failure is known issue #36681
Installer Build and Test coreclr failure is known issue #35068
oreCLR Pri0 Runtime Tests Run Windows_NT x86 checked is known issue #38284

@jkotas
jkotas merged commit 31699de into dotnet:masterJun 27, 2020
@jkotas
jkotas deleted the generics-inlining branch June 27, 2020 13:36
@nietras

Copy link
Copy Markdown
Contributor

Great to see this merged. 👍

when the caller and callee are the same exact type and instantiation.

This is quite clear but to be sure, I assume this means this won't work for base class methods for example? E. g. called in a derived class?

@jkotas

Copy link
Copy Markdown
MemberAuthor

I assume this means this won't work for base class methods for example? E. g. called in a derived class?

Correct.

@ghostghost locked as resolved and limited conversation to collaborators Dec 8, 2020
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

11 participants

@jkotas@nietras@mangod9@BruceForstall@davidwrighton@EgorBo@stephentoub@jakobbotsch@AndyAyersMS@MichalStrehovsky@Dotnet-GitSync-Bot
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Enable inlining of shared generics code within same type - #38229

Merged
jkotas merged 11 commits into
dotnet:masterfrom
jkotas:generics-inlining
Jun 27, 2020
Merged

Enable inlining of shared generics code within same type#38229
jkotas merged 11 commits into
dotnet:masterfrom
jkotas:generics-inlining

Conversation

@jkotas

@jkotasjkotas commented Jun 22, 2020

Copy link
Copy Markdown
Member

This change allows inlining of generic dictionary lookups when the caller and callee are the same exact type and instantiation. It is a simpler version of #31833.

Motivation for this change is to unblock #37941.

Example:

class G<T>
{
static bool M1() => typeof(T);
object M2() => new T[1];
bool M3() => Unsafe.SizeOf<T>();
static string M()
{
// Assume that T is string
// All M1, M2, M3 are inlined after this change, but were not inlined before this change
M1(); M2(); M3();
// Still not inlined - different instantiation, not the same exact type
G<List<T>>.M1();
// Still not inlined - different type.
OtherG<T>.M();
}
}

@jkotas
jkotas marked this pull request as draft June 22, 2020 19:22
@jkotas
jkotasforce-pushed the generics-inlining branch from 9645520 to dd93ffaCompareJune 23, 2020 06:58
@jaredparjaredpar mentioned this pull request Jun 23, 2020
@jkotas
jkotasforce-pushed the generics-inlining branch 3 times, most recently from 8cbb058 to 3770ae3CompareJune 24, 2020 16:20
Comment threadsrc/coreclr/src/ToolBox/superpmi/superpmi-shared/methodcontext.cpp Outdated
@jkotas
jkotasforce-pushed the generics-inlining branch from 3819aa0 to d09090eCompareJune 24, 2020 23:22
@jkotas

Copy link
Copy Markdown
MemberAuthor

Asm diffs

Total bytes of diff: 2738 (0.083% of base)
diff is a regression.
Top file regressions (bytes):
2738 : System.Private.CoreLib.dasm (0.083% of base)
1 total files with Code Size differences (0 improved, 1 regressed), 0 unchanged.
Top method regressions (bytes):
291 (41.044% of base) : System.Private.CoreLib.dasm - ValueTuple`3:System.IComparable.CompareTo(Object):int:this (3 methods)
260 (67.010% of base) : System.Private.CoreLib.dasm - ValueTuple`3:Equals(Object):bool:this (3 methods)
184 (3.340% of base) : System.Private.CoreLib.dasm - GenericArraySortHelper`1:IntroSort(Span`1,int) (12 methods)
173 (28.978% of base) : System.Private.CoreLib.dasm - ValueTask`1:ToString():String:this (3 methods)
159 (15.071% of base) : System.Private.CoreLib.dasm - Memory`1:CopyTo(Memory`1):this (4 methods)
159 (14.171% of base) : System.Private.CoreLib.dasm - Memory`1:TryCopyTo(Memory`1):bool:this (4 methods)
157 (69.778% of base) : System.Private.CoreLib.dasm - ValueTuple`4:Equals(Object):bool:this (2 methods)
118 (6.833% of base) : System.Private.CoreLib.dasm - Vector128`1:Equals(Object):bool:this (11 methods)
118 (3.562% of base) : System.Private.CoreLib.dasm - GenericArraySortHelper`1:PickPivotAndPartition(Span`1):int (12 methods)
94 (8.818% of base) : System.Private.CoreLib.dasm - ReadOnlyMemory`1:CopyTo(Memory`1):this (4 methods)
94 (8.297% of base) : System.Private.CoreLib.dasm - ReadOnlyMemory`1:TryCopyTo(Memory`1):bool:this (4 methods)
87 (13.344% of base) : System.Private.CoreLib.dasm - Memory`1:ToArray():ref:this (4 methods)
87 (14.846% of base) : System.Private.CoreLib.dasm - ReadOnlyMemory`1:ToArray():ref:this (4 methods)
76 (8.736% of base) : System.Private.CoreLib.dasm - List`1:System.Collections.IList.IndexOf(Object):int:this (8 methods)
75 (202.703% of base) : System.Private.CoreLib.dasm - List`1:Contains(KeyValuePair`2):bool:this
72 (39.560% of base) : System.Private.CoreLib.dasm - Lazy`1:CreateValue():__Canon:this
66 (129.412% of base) : System.Private.CoreLib.dasm - List`1:Remove(KeyValuePair`2):bool:this
63 (75.000% of base) : System.Private.CoreLib.dasm - TaskCompletionSource`1:.ctor(Object):this (2 methods)
62 (16.712% of base) : System.Private.CoreLib.dasm - ValueTuple`2:Equals(Object):bool:this (3 methods)
60 (8.310% of base) : System.Private.CoreLib.dasm - ValueTuple`2:System.IComparable.CompareTo(Object):int:this (3 methods)
Top method improvements (bytes):
-94 (-14.506% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Implicit(ref):ReadOnlySpan`1 (18 methods)
-73 (-8.120% of base) : System.Private.CoreLib.dasm - Memory`1:Equals(Object):bool:this (4 methods)
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector128`1:get_DisplayString():String:this
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector256`1:get_DisplayString():String:this
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector64`1:get_DisplayString():String:this
-44 (-6.790% of base) : System.Private.CoreLib.dasm - Span`1:op_Implicit(ref):Span`1 (18 methods)
-26 (-3.641% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Inequality(ReadOnlySpan`1,ReadOnlySpan`1):bool (18 methods)
-26 (-3.641% of base) : System.Private.CoreLib.dasm - Span`1:op_Inequality(Span`1,Span`1):bool (18 methods)
-15 (-0.957% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Implicit(ArraySegment`1):ReadOnlySpan`1 (18 methods)
-4 (-0.702% of base) : System.Private.CoreLib.dasm - AsyncValueTaskMethodBuilder`1:get_Task():ValueTask`1:this (2 methods)
-2 (-0.217% of base) : System.Private.CoreLib.dasm - ArraySegment`1:op_Inequality(ArraySegment`1,ArraySegment`1):bool (18 methods)
-2 (-0.930% of base) : System.Private.CoreLib.dasm - Memory`1:op_Implicit(ref):Memory`1 (4 methods)
Top method regressions (percentages):
47 (293.750% of base) : System.Private.CoreLib.dasm - SingleProducerSingleConsumerQueue`1:System.Collections.IEnumerable.GetEnumerator():IEnumerator:this
75 (202.703% of base) : System.Private.CoreLib.dasm - List`1:Contains(KeyValuePair`2):bool:this
66 (129.412% of base) : System.Private.CoreLib.dasm - List`1:Remove(KeyValuePair`2):bool:this
46 (124.324% of base) : System.Private.CoreLib.dasm - List`1:Contains(__Canon):bool:this
63 (75.000% of base) : System.Private.CoreLib.dasm - TaskCompletionSource`1:.ctor(Object):this (2 methods)
37 (72.549% of base) : System.Private.CoreLib.dasm - List`1:Remove(__Canon):bool:this
36 (72.000% of base) : System.Private.CoreLib.dasm - AsyncTaskMethodBuilder`1:SetNotificationForWaitCompletion(bool):this
59 (70.238% of base) : System.Private.CoreLib.dasm - TaskCompletionSource`1:.ctor(int):this (2 methods)
157 (69.778% of base) : System.Private.CoreLib.dasm - ValueTuple`4:Equals(Object):bool:this (2 methods)
260 (67.010% of base) : System.Private.CoreLib.dasm - ValueTuple`3:Equals(Object):bool:this (3 methods)
25 (43.103% of base) : System.Private.CoreLib.dasm - HashSet`1:System.Collections.Generic.IEnumerable<T>.GetEnumerator():IEnumerator`1:this
25 (43.103% of base) : System.Private.CoreLib.dasm - HashSet`1:System.Collections.IEnumerable.GetEnumerator():IEnumerator:this
291 (41.044% of base) : System.Private.CoreLib.dasm - ValueTuple`3:System.IComparable.CompareTo(Object):int:this (3 methods)
72 (39.560% of base) : System.Private.CoreLib.dasm - Lazy`1:CreateValue():__Canon:this
173 (28.978% of base) : System.Private.CoreLib.dasm - ValueTask`1:ToString():String:this (3 methods)
47 (27.485% of base) : System.Private.CoreLib.dasm - Lazy`1:ExecutionAndPublication(LazyHelper,bool):this
48 (19.753% of base) : System.Private.CoreLib.dasm - AsyncValueTaskMethodBuilder`1:SetException(Exception):this (2 methods)
18 (19.149% of base) : System.Private.CoreLib.dasm - Collection`1:System.Collections.IList.Contains(Object):bool:this
18 (18.947% of base) : System.Private.CoreLib.dasm - Collection`1:System.Collections.IList.IndexOf(Object):int:this
62 (16.712% of base) : System.Private.CoreLib.dasm - ValueTuple`2:Equals(Object):bool:this (3 methods)
Top method improvements (percentages):
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector128`1:get_DisplayString():String:this
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector256`1:get_DisplayString():String:this
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector64`1:get_DisplayString():String:this
-94 (-14.506% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Implicit(ref):ReadOnlySpan`1 (18 methods)
-73 (-8.120% of base) : System.Private.CoreLib.dasm - Memory`1:Equals(Object):bool:this (4 methods)
-44 (-6.790% of base) : System.Private.CoreLib.dasm - Span`1:op_Implicit(ref):Span`1 (18 methods)
-26 (-3.641% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Inequality(ReadOnlySpan`1,ReadOnlySpan`1):bool (18 methods)
-26 (-3.641% of base) : System.Private.CoreLib.dasm - Span`1:op_Inequality(Span`1,Span`1):bool (18 methods)
-15 (-0.957% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Implicit(ArraySegment`1):ReadOnlySpan`1 (18 methods)
-2 (-0.930% of base) : System.Private.CoreLib.dasm - Memory`1:op_Implicit(ref):Memory`1 (4 methods)
-4 (-0.702% of base) : System.Private.CoreLib.dasm - AsyncValueTaskMethodBuilder`1:get_Task():ValueTask`1:this (2 methods)
-2 (-0.217% of base) : System.Private.CoreLib.dasm - ArraySegment`1:op_Inequality(ArraySegment`1,ArraySegment`1):bool (18 methods)
57 total methods with Code Size differences (12 improved, 45 regressed), 20772 unchanged.

@jkotas

jkotas commented Jun 25, 2020

Copy link
Copy Markdown
MemberAuthor

Example of code-size regression - inlining tends to increase code size on average:

List`1:Contains(KeyValuePair`2):bool:this

Baseline

G_M57469_IG01:
sub rsp, 40
;; bbWeight=1 PerfScore 0.25
G_M57469_IG02:
cmp dword ptr [rcx+16], 0
je SHORT G_M57469_IG05
;; bbWeight=1 PerfScore 3.00
G_M57469_IG03:
call [List`1:IndexOf(KeyValuePair`2):int:this]
cmp eax, -1
setne al
movzx rax, al
;; bbWeight=0.50 PerfScore 2.25
G_M57469_IG04:
add rsp, 40
ret ;; bbWeight=0.50 PerfScore 0.63
G_M57469_IG05:
xor eax, eax
;; bbWeight=0.50 PerfScore 0.13
G_M57469_IG06:
add rsp, 40
ret 

Diff

G_M57469_IG01:
push rdi
push rsi
push rbx
sub rsp, 64
xor rax, rax
mov qword ptr [rsp+28H], rax
mov qword ptr [rsp+38H], rcx
mov rsi, rcx
;; bbWeight=1 PerfScore 5.75
G_M57469_IG02:
cmp dword ptr [rsi+16], 0
je SHORT G_M57469_IG05
;; bbWeight=1 PerfScore 3.00
G_M57469_IG03:
mov rdi, gword ptr [rdx]
mov ebx, dword ptr [rdx+8]
mov rcx, qword ptr [rsi]
call [CORINFO_HELP_READYTORUN_GENERIC_HANDLE]
mov rcx, rax
mov rdx, gword ptr [rsi+8]
lea r8, bword ptr [rsp+28H]
mov gword ptr [r8], rdi
mov dword ptr [r8+8], ebx
mov r8d, dword ptr [rsi+16]
mov dword ptr [rsp+20H], r8d
lea r8, bword ptr [rsp+28H]
xor r9d, r9d
call [Array:IndexOf(ref,KeyValuePair`2,int,int):int]
cmp eax, -1
setne al
movzx rax, al
;; bbWeight=0.50 PerfScore 11.00
G_M57469_IG04:
add rsp, 64
pop rbx
pop rsi
pop rdi
ret ;; bbWeight=0.50 PerfScore 1.38
G_M57469_IG05:
xor eax, eax
;; bbWeight=0.50 PerfScore 0.13
G_M57469_IG06:
add rsp, 64
pop rbx
pop rsi
pop rdi
ret 

@jkotas

Copy link
Copy Markdown
MemberAuthor

Example of code-size improvement - inlining reduces code size when it enables additional constant-prop:

Vector128`1:get_DisplayString

Baseline:

G_M30183_IG01:
push rdi
push rsi
sub rsp, 40
mov qword ptr [rsp+20H], rdx
mov rdi, rcx
mov rsi, rdx
;; bbWeight=1 PerfScore 3.75
G_M30183_IG02:
mov rcx, rsi
call [CORINFO_HELP_READYTORUN_GENERIC_HANDLE]
mov rcx, rax
call [Vector128`1:get_IsSupported():bool]
test al, al
je SHORT G_M30183_IG05
;; bbWeight=1 PerfScore 7.75
G_M30183_IG03:
mov rcx, rsi
call [CORINFO_HELP_READYTORUN_GENERIC_HANDLE]
mov rcx, rdi
call rax
nop ;; bbWeight=0.50 PerfScore 3.38
G_M30183_IG04:
add rsp, 40
pop rsi
pop rdi
ret ;; bbWeight=0.50 PerfScore 1.13
G_M30183_IG05:
mov rcx, qword ptr [(reloc)]
mov rcx, gword ptr [rcx]
mov rdx, qword ptr [(reloc)]
mov rdx, gword ptr [rdx]
call [SR:GetResourceString(String,String):String]
nop ;; bbWeight=0.50 PerfScore 5.63
G_M30183_IG06:
add rsp, 40
pop rsi
pop rdi
ret ;; bbWeight=0.50 PerfScore 1.13

Diff:

G_M30183_IG01:
sub rsp, 40
;; bbWeight=1 PerfScore 0.25
G_M30183_IG02:
mov rcx, qword ptr [(reloc)]
mov rcx, gword ptr [rcx]
mov rdx, qword ptr [(reloc)]
mov rdx, gword ptr [rdx]
call [SR:GetResourceString(String,String):String]
nop ;; bbWeight=1 PerfScore 11.25
G_M30183_IG03:
add rsp, 40
ret 

@nietras

Copy link
Copy Markdown
Contributor

The example improvements are exactly the kind I am hoping for 👍

@jkotas
jkotas marked this pull request as ready for review June 26, 2020 05:52
@jkotas

Copy link
Copy Markdown
MemberAuthor

@dotnet/crossgen-contrib Any crossgen2 specific legs you would recommend to run on this?

@dotnet/jit-contrib Any JIT specific legs you would recommend to run on this?

Comment threadsrc/coreclr/src/tools/Common/Sorting/MergeSortCore.cs
Comment threadsrc/coreclr/src/vm/jitinterface.cpp
@jkotasjkotas changed the title WIP: Enable inlining of shared generics code within same typeEnable inlining of shared generics code within same typeJun 26, 2020

@MichalStrehovskyMichalStrehovsky left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Crossgen2 side looks good to me.

I echo Manish's question about the assert - I think that code might be reachable.

Not commenting on the parallel sort thing because I didn't look at that code before.

@mangod9

Copy link
Copy Markdown
Member

@dotnet/crossgen-contrib Any crossgen2 specific legs you would recommend to run on this?

@dotnet/jit-contrib Any JIT specific legs you would recommend to run on this?

for crossgen2 would be good to run the runtime-coreclr crossgen2 pipeline for validation.

@jkotas

Copy link
Copy Markdown
MemberAuthor

/azp run runtime-coreclr crossgen2

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

@jkotas

Copy link
Copy Markdown
MemberAuthor

for crossgen2 would be good to run the runtime-coreclr crossgen2 pipeline for validation.

@mangod9 What is the right way to trigger it? I have tried to trigger it above, but it does not seem to work.

@mangod9

Copy link
Copy Markdown
Member

for crossgen2 would be good to run the runtime-coreclr crossgen2 pipeline for validation.

@mangod9 What is the right way to trigger it? I have tried to trigger it above, but it does not seem to work.

Hmm.. not sure why that pipeline command fails (probably some config for the pipeline @trylek ?). For now I have manually triggered it via AzDo portal

@BruceForstall

Copy link
Copy Markdown
Contributor

I thought (almost) no triggers were enabled, due to issues with accidentally triggering too many builds. Also, #32777

@davidwrighton

Copy link
Copy Markdown
Member

@jkotas, you can no longer use any of the pipelines from the AzDo command line. You need to find the pipeline in AzDo and manually start a build.

Comment threadsrc/coreclr/src/tools/Common/Sorting/MergeSortCore.cs

@AndyAyersMSAndyAyersMS left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Were those diffs from crossgen or PMI?

Might want to run a wider set of diffs (pass -f to jit-diff) and/or look diffs from both prejitting and jitting.

Comment threadsrc/coreclr/src/jit/importer.cpp
Comment threadsrc/coreclr/src/jit/importer.cpp

@davidwrightondavidwrighton left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like you to split out the work to address the static field address issue into a separate PR, and to provide a better description of under what conditions an inlined shared generic lookup will work and where it won't work.

@jkotasjkotas mentioned this pull request Jun 26, 2020
@jkotas

Copy link
Copy Markdown
MemberAuthor

I'd like you to split out the work to address the static field address issue into a separate PR, and to provide a better description of under what conditions an inlined shared generic lookup will work and where it won't work.

@davidwrighton Done. The updated description is at the top.

@BruceForstall

Copy link
Copy Markdown
Contributor

@dotnet/jit-contrib Any JIT specific legs you would recommend to run on this?

Seems like it would be worthwhile to runtime-coreclr jitstress (https://dev.azure.com/dnceng/public/_build?definitionId=658) to run Pri-1 tests with JitStress=1/2, which would include inlining stress.

@jkotas

Copy link
Copy Markdown
MemberAuthor

Might want to run a wider set of diffs

-f crossgen diffs:

Found 25 files with textual diffs.
Summary of Code Size diffs:
(Lower is better)
Total bytes of diff: 28141 (0.086% of base)
diff is a regression.
Total byte diff includes -111 bytes from reconciling methods
Base had 2 unique methods, 111 unique bytes
Diff had 0 unique methods, 0 unique bytes
Top file regressions (bytes):
12800 : System.Collections.Immutable.dasm (5.998% of base)
5146 : System.Memory.dasm (6.358% of base)
2738 : System.Private.CoreLib.dasm (0.083% of base)
2735 : System.Collections.dasm (2.930% of base)
714 : System.Collections.Concurrent.dasm (1.559% of base)
587 : Microsoft.CodeAnalysis.dasm (0.078% of base)
473 : System.Reflection.Metadata.dasm (0.147% of base)
468 : System.Linq.dasm (0.353% of base)
298 : System.Threading.Tasks.Dataflow.dasm (0.217% of base)
282 : System.Linq.Expressions.dasm (0.010% of base)
266 : System.ObjectModel.dasm (1.353% of base)
202 : Microsoft.CSharp.dasm (0.062% of base)
179 : System.DirectoryServices.AccountManagement.dasm (0.072% of base)
171 : Microsoft.Diagnostics.Tracing.TraceEvent.dasm (0.005% of base)
153 : Newtonsoft.Json.dasm (0.025% of base)
138 : Microsoft.Extensions.Logging.Abstractions.dasm (0.565% of base)
135 : Microsoft.Diagnostics.FastSerialization.dasm (0.401% of base)
126 : System.Private.Xml.dasm (0.004% of base)
116 : System.Linq.Parallel.dasm (0.020% of base)
108 : Microsoft.CodeAnalysis.CSharp.dasm (0.005% of base)
25 total files with Code Size differences (0 improved, 25 regressed), 239 unchanged.
Top method regressions (bytes):
554 (45.975% of base) : System.Memory.dasm - ReadOnlySequence`1:TryGet(byref,byref,bool):bool:this (3 methods)
543 (35.560% of base) : System.Memory.dasm - SequenceReader`1:TryReadTo(byref,__Canon,__Canon,bool):bool:this (2 methods)
437 (108.168% of base) : System.Collections.Immutable.dasm - Builder:System.Collections.IEnumerable.GetEnumerator():IEnumerator:this (8 methods)
422 (28.964% of base) : System.Memory.dasm - SequenceReader`1:TryReadToSlow(byref,__Canon,__Canon,int,bool):bool:this (2 methods)
347 (10.534% of base) : System.Collections.Immutable.dasm - Enumerator:MoveNext():bool:this (15 methods)
344 (148.276% of base) : System.Collections.Immutable.dasm - Node:System.Collections.IEnumerable.GetEnumerator():IEnumerator:this (4 methods)
336 (13.925% of base) : System.Memory.dasm - ReadOnlySequence`1:Slice(long,long):ReadOnlySequence`1:this (3 methods)
330 (23.827% of base) : System.Collections.Immutable.dasm - Enumerator:Reset():this (11 methods)
323 (21.824% of base) : System.Collections.Immutable.dasm - Builder:AddRange(IEnumerable`1):this (6 methods)
291 (31.699% of base) : System.Memory.dasm - ReadOnlySequence`1:Slice(long):ReadOnlySequence`1:this (3 methods)
291 (41.044% of base) : System.Private.CoreLib.dasm - ValueTuple`3:System.IComparable.CompareTo(Object):int:this (3 methods)
287 (38.013% of base) : System.Reflection.Metadata.dasm - SignatureDecoder`2:DecodeType(byref,bool,int):__Canon:this
286 (54.476% of base) : System.Memory.dasm - ReadOnlySequence`1:get_FirstSpan():ReadOnlySpan`1:this (3 methods)
281 (19.957% of base) : System.Memory.dasm - SequenceReader`1:TryReadTo(byref,ReadOnlySpan`1,bool):bool:this (2 methods)
275 (158.046% of base) : System.Collections.Immutable.dasm - Node:System.Collections.Generic.IEnumerable<T>.GetEnumerator():IEnumerator`1:this (3 methods)
268 (52.652% of base) : System.Memory.dasm - ReadOnlySequence`1:get_First():ReadOnlyMemory`1:this (3 methods)
268 (41.358% of base) : System.Memory.dasm - ReadOnlySequence`1:GetPosition(long,SequencePosition):SequencePosition:this (3 methods)
267 (41.654% of base) : System.Memory.dasm - ReadOnlySequence`1:GetPosition(long):SequencePosition:this (3 methods)
266 (95.341% of base) : System.Collections.Immutable.dasm - Builder:System.Collections.Generic.IEnumerable<T>.GetEnumerator():IEnumerator`1:this (6 methods)
265 (24.953% of base) : System.Collections.Immutable.dasm - Node:CopyTo(ref,int):this (3 methods)
Top method improvements (bytes):
-169 (-26.365% of base) : System.Collections.Immutable.dasm - ImmutableArray`1:InsertRange(int,ImmutableArray`1):ImmutableArray`1:this (2 methods)
-145 (-27.672% of base) : System.Collections.Immutable.dasm - Node:GetEnumerator():Enumerator:this (4 base, 3 diff methods)
-94 (-14.506% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Implicit(ref):ReadOnlySpan`1 (18 methods)
-82 (-100.000% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:CreatePair(__Canon,Pair`2):Pair`2 (1 base, 0 diff methods)
-73 (-8.120% of base) : System.Private.CoreLib.dasm - Memory`1:Equals(Object):bool:this (4 methods)
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector128`1:get_DisplayString():String:this
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector256`1:get_DisplayString():String:this
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector64`1:get_DisplayString():String:this
-56 (-29.474% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:Add(__Canon,int):bool:this
-52 (-27.083% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:Add(__Canon,__Canon):bool:this
-44 (-6.790% of base) : System.Private.CoreLib.dasm - Span`1:op_Implicit(ref):Span`1 (18 methods)
-42 (-7.420% of base) : System.Collections.Immutable.dasm - ImmutableArray`1:RemoveAll(Predicate`1):ImmutableArray`1:this (2 methods)
-32 (-46.377% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:.ctor(__Canon,int):this
-29 (-100.000% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:CreatePair(__Canon,int):Pair`2 (1 base, 0 diff methods)
-26 (-3.641% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Inequality(ReadOnlySpan`1,ReadOnlySpan`1):bool (18 methods)
-26 (-3.641% of base) : System.Private.CoreLib.dasm - Span`1:op_Inequality(Span`1,Span`1):bool (18 methods)
-22 (-31.884% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:.ctor(__Canon,__Canon):this
-21 (-19.444% of base) : System.Linq.Parallel.dasm - OrderedGroupJoinHashLookup:CreateValuePair(GroupKeyData):Pair`2:this
-19 (-11.515% of base) : Microsoft.CodeAnalysis.CSharp.dasm - SyntaxList`1:Any(ushort):bool:this
-19 (-13.475% of base) : System.Linq.dasm - SparseArrayBuilder`1:Reserve(int):this
Top method regressions (percentages):
126 (787.500% of base) : System.DirectoryServices.AccountManagement.dasm - TrackedCollection`1:System.Collections.IEnumerable.GetEnumerator():IEnumerator:this
92 (575.000% of base) : System.Collections.Immutable.dasm - ImmutableList`1:System.Collections.Immutable.IImmutableList<T>.Remove(KeyValuePair`2,IEqualityComparer`1):IImmutableList`1:this
63 (393.750% of base) : System.Collections.dasm - SortedList`2:get_Keys():IList`1:this
63 (393.750% of base) : System.Collections.dasm - SortedList`2:System.Collections.Generic.IDictionary<TKey,TValue>.get_Keys():ICollection`1:this
63 (393.750% of base) : System.Collections.dasm - SortedList`2:System.Collections.IDictionary.get_Keys():ICollection:this
63 (393.750% of base) : System.Collections.dasm - SortedList`2:System.Collections.Generic.IReadOnlyDictionary<TKey,TValue>.get_Keys():IEnumerable`1:this
63 (393.750% of base) : System.Collections.dasm - SortedList`2:get_Values():IList`1:this
63 (393.750% of base) : System.Collections.dasm - SortedList`2:System.Collections.Generic.IDictionary<TKey,TValue>.get_Values():ICollection`1:this
63 (393.750% of base) : System.Collections.dasm - SortedList`2:System.Collections.IDictionary.get_Values():ICollection:this
63 (393.750% of base) : System.Collections.dasm - SortedList`2:System.Collections.Generic.IReadOnlyDictionary<TKey,TValue>.get_Values():IEnumerable`1:this
62 (387.500% of base) : System.Collections.dasm - SortedDictionary`2:System.Collections.Generic.IDictionary<TKey,TValue>.get_Keys():ICollection`1:this
62 (387.500% of base) : System.Collections.dasm - SortedDictionary`2:System.Collections.Generic.IReadOnlyDictionary<TKey,TValue>.get_Keys():IEnumerable`1:this
62 (387.500% of base) : System.Collections.dasm - SortedDictionary`2:System.Collections.Generic.IDictionary<TKey,TValue>.get_Values():ICollection`1:this
62 (387.500% of base) : System.Collections.dasm - SortedDictionary`2:System.Collections.Generic.IReadOnlyDictionary<TKey,TValue>.get_Values():IEnumerable`1:this
62 (387.500% of base) : System.Collections.dasm - SortedDictionary`2:System.Collections.IDictionary.get_Keys():ICollection:this
62 (387.500% of base) : System.Collections.dasm - SortedDictionary`2:System.Collections.IDictionary.get_Values():ICollection:this
61 (381.250% of base) : Newtonsoft.Json.dasm - EnumerableDictionaryWrapper`2:System.Collections.IEnumerable.GetEnumerator():IEnumerator:this
61 (381.250% of base) : System.Net.Http.dasm - HttpHeaderValueCollection`1:System.Collections.IEnumerable.GetEnumerator():IEnumerator:this
60 (375.000% of base) : System.Collections.Immutable.dasm - ImmutableList`1:System.Collections.Immutable.IImmutableList<T>.Remove(__Canon,IEqualityComparer`1):IImmutableList`1:this
60 (375.000% of base) : System.Collections.Immutable.dasm - ImmutableStack`1:System.Collections.Immutable.IImmutableStack<T>.Push(__Canon):IImmutableStack`1:this
Top method improvements (percentages):
-82 (-100.000% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:CreatePair(__Canon,Pair`2):Pair`2 (1 base, 0 diff methods)
-29 (-100.000% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:CreatePair(__Canon,int):Pair`2 (1 base, 0 diff methods)
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector128`1:get_DisplayString():String:this
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector256`1:get_DisplayString():String:this
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector64`1:get_DisplayString():String:this
-32 (-46.377% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:.ctor(__Canon,int):this
-22 (-31.884% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:.ctor(__Canon,__Canon):this
-56 (-29.474% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:Add(__Canon,int):bool:this
-145 (-27.672% of base) : System.Collections.Immutable.dasm - Node:GetEnumerator():Enumerator:this (4 base, 3 diff methods)
-52 (-27.083% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:Add(__Canon,__Canon):bool:this
-169 (-26.365% of base) : System.Collections.Immutable.dasm - ImmutableArray`1:InsertRange(int,ImmutableArray`1):ImmutableArray`1:this (2 methods)
-21 (-19.444% of base) : System.Linq.Parallel.dasm - OrderedGroupJoinHashLookup:CreateValuePair(GroupKeyData):Pair`2:this
-94 (-14.506% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Implicit(ref):ReadOnlySpan`1 (18 methods)
-19 (-13.475% of base) : System.Linq.dasm - SparseArrayBuilder`1:Reserve(int):this
-18 (-12.950% of base) : System.Collections.Immutable.dasm - ImmutableDictionary`2:TryGetValue(__Canon,byref):bool:this
-18 (-12.950% of base) : System.Collections.Immutable.dasm - ImmutableDictionary`2:TryGetKey(__Canon,byref):bool:this
-9 (-12.857% of base) : Microsoft.CodeAnalysis.VisualBasic.dasm - _Closure$__17`1:.cctor()
-13 (-12.745% of base) : System.Collections.Immutable.dasm - ImmutableDictionary`2:Contains(KeyValuePair`2):bool:this
-19 (-11.515% of base) : Microsoft.CodeAnalysis.CSharp.dasm - SyntaxList`1:Any(ushort):bool:this
-13 (-10.484% of base) : System.Collections.Immutable.dasm - ImmutableDictionary`2:ContainsKey(__Canon):bool:this
406 total methods with Code Size differences (43 improved, 363 regressed), 186975 unchanged.

@jkotas

Copy link
Copy Markdown
MemberAuthor
  • ReadOnlySequence size regressions is caused by AggresiveInlining on ~70 lines method
  • ImmutableCollections size regression is a lots of small methods that are getting inlined into each other now

@jkotas

Copy link
Copy Markdown
MemberAuthor

JIT stress failure is known issue #36681
Installer Build and Test coreclr failure is known issue #35068
oreCLR Pri0 Runtime Tests Run Windows_NT x86 checked is known issue #38284

@jkotas
jkotas merged commit 31699de into dotnet:masterJun 27, 2020
@jkotas
jkotas deleted the generics-inlining branch June 27, 2020 13:36
@nietras

Copy link
Copy Markdown
Contributor

Great to see this merged. 👍

when the caller and callee are the same exact type and instantiation.

This is quite clear but to be sure, I assume this means this won't work for base class methods for example? E. g. called in a derived class?

@jkotas

Copy link
Copy Markdown
MemberAuthor

I assume this means this won't work for base class methods for example? E. g. called in a derived class?

Correct.

@ghostghost locked as resolved and limited conversation to collaborators Dec 8, 2020
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

11 participants

@jkotas@nietras@mangod9@BruceForstall@davidwrighton@EgorBo@stephentoub@jakobbotsch@AndyAyersMS@MichalStrehovsky@Dotnet-GitSync-Bot
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Enable inlining of shared generics code within same type - #38229

Merged
jkotas merged 11 commits into
dotnet:masterfrom
jkotas:generics-inlining
Jun 27, 2020
Merged

Enable inlining of shared generics code within same type#38229
jkotas merged 11 commits into
dotnet:masterfrom
jkotas:generics-inlining

Conversation

@jkotas

@jkotasjkotas commented Jun 22, 2020

Copy link
Copy Markdown
Member

This change allows inlining of generic dictionary lookups when the caller and callee are the same exact type and instantiation. It is a simpler version of #31833.

Motivation for this change is to unblock #37941.

Example:

class G<T>
{
static bool M1() => typeof(T);
object M2() => new T[1];
bool M3() => Unsafe.SizeOf<T>();
static string M()
{
// Assume that T is string
// All M1, M2, M3 are inlined after this change, but were not inlined before this change
M1(); M2(); M3();
// Still not inlined - different instantiation, not the same exact type
G<List<T>>.M1();
// Still not inlined - different type.
OtherG<T>.M();
}
}

@jkotas
jkotas marked this pull request as draft June 22, 2020 19:22
@jkotas
jkotasforce-pushed the generics-inlining branch from 9645520 to dd93ffaCompareJune 23, 2020 06:58
@jaredparjaredpar mentioned this pull request Jun 23, 2020
@jkotas
jkotasforce-pushed the generics-inlining branch 3 times, most recently from 8cbb058 to 3770ae3CompareJune 24, 2020 16:20
Comment threadsrc/coreclr/src/ToolBox/superpmi/superpmi-shared/methodcontext.cpp Outdated
@jkotas
jkotasforce-pushed the generics-inlining branch from 3819aa0 to d09090eCompareJune 24, 2020 23:22
@jkotas

Copy link
Copy Markdown
MemberAuthor

Asm diffs

Total bytes of diff: 2738 (0.083% of base)
diff is a regression.
Top file regressions (bytes):
2738 : System.Private.CoreLib.dasm (0.083% of base)
1 total files with Code Size differences (0 improved, 1 regressed), 0 unchanged.
Top method regressions (bytes):
291 (41.044% of base) : System.Private.CoreLib.dasm - ValueTuple`3:System.IComparable.CompareTo(Object):int:this (3 methods)
260 (67.010% of base) : System.Private.CoreLib.dasm - ValueTuple`3:Equals(Object):bool:this (3 methods)
184 (3.340% of base) : System.Private.CoreLib.dasm - GenericArraySortHelper`1:IntroSort(Span`1,int) (12 methods)
173 (28.978% of base) : System.Private.CoreLib.dasm - ValueTask`1:ToString():String:this (3 methods)
159 (15.071% of base) : System.Private.CoreLib.dasm - Memory`1:CopyTo(Memory`1):this (4 methods)
159 (14.171% of base) : System.Private.CoreLib.dasm - Memory`1:TryCopyTo(Memory`1):bool:this (4 methods)
157 (69.778% of base) : System.Private.CoreLib.dasm - ValueTuple`4:Equals(Object):bool:this (2 methods)
118 (6.833% of base) : System.Private.CoreLib.dasm - Vector128`1:Equals(Object):bool:this (11 methods)
118 (3.562% of base) : System.Private.CoreLib.dasm - GenericArraySortHelper`1:PickPivotAndPartition(Span`1):int (12 methods)
94 (8.818% of base) : System.Private.CoreLib.dasm - ReadOnlyMemory`1:CopyTo(Memory`1):this (4 methods)
94 (8.297% of base) : System.Private.CoreLib.dasm - ReadOnlyMemory`1:TryCopyTo(Memory`1):bool:this (4 methods)
87 (13.344% of base) : System.Private.CoreLib.dasm - Memory`1:ToArray():ref:this (4 methods)
87 (14.846% of base) : System.Private.CoreLib.dasm - ReadOnlyMemory`1:ToArray():ref:this (4 methods)
76 (8.736% of base) : System.Private.CoreLib.dasm - List`1:System.Collections.IList.IndexOf(Object):int:this (8 methods)
75 (202.703% of base) : System.Private.CoreLib.dasm - List`1:Contains(KeyValuePair`2):bool:this
72 (39.560% of base) : System.Private.CoreLib.dasm - Lazy`1:CreateValue():__Canon:this
66 (129.412% of base) : System.Private.CoreLib.dasm - List`1:Remove(KeyValuePair`2):bool:this
63 (75.000% of base) : System.Private.CoreLib.dasm - TaskCompletionSource`1:.ctor(Object):this (2 methods)
62 (16.712% of base) : System.Private.CoreLib.dasm - ValueTuple`2:Equals(Object):bool:this (3 methods)
60 (8.310% of base) : System.Private.CoreLib.dasm - ValueTuple`2:System.IComparable.CompareTo(Object):int:this (3 methods)
Top method improvements (bytes):
-94 (-14.506% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Implicit(ref):ReadOnlySpan`1 (18 methods)
-73 (-8.120% of base) : System.Private.CoreLib.dasm - Memory`1:Equals(Object):bool:this (4 methods)
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector128`1:get_DisplayString():String:this
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector256`1:get_DisplayString():String:this
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector64`1:get_DisplayString():String:this
-44 (-6.790% of base) : System.Private.CoreLib.dasm - Span`1:op_Implicit(ref):Span`1 (18 methods)
-26 (-3.641% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Inequality(ReadOnlySpan`1,ReadOnlySpan`1):bool (18 methods)
-26 (-3.641% of base) : System.Private.CoreLib.dasm - Span`1:op_Inequality(Span`1,Span`1):bool (18 methods)
-15 (-0.957% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Implicit(ArraySegment`1):ReadOnlySpan`1 (18 methods)
-4 (-0.702% of base) : System.Private.CoreLib.dasm - AsyncValueTaskMethodBuilder`1:get_Task():ValueTask`1:this (2 methods)
-2 (-0.217% of base) : System.Private.CoreLib.dasm - ArraySegment`1:op_Inequality(ArraySegment`1,ArraySegment`1):bool (18 methods)
-2 (-0.930% of base) : System.Private.CoreLib.dasm - Memory`1:op_Implicit(ref):Memory`1 (4 methods)
Top method regressions (percentages):
47 (293.750% of base) : System.Private.CoreLib.dasm - SingleProducerSingleConsumerQueue`1:System.Collections.IEnumerable.GetEnumerator():IEnumerator:this
75 (202.703% of base) : System.Private.CoreLib.dasm - List`1:Contains(KeyValuePair`2):bool:this
66 (129.412% of base) : System.Private.CoreLib.dasm - List`1:Remove(KeyValuePair`2):bool:this
46 (124.324% of base) : System.Private.CoreLib.dasm - List`1:Contains(__Canon):bool:this
63 (75.000% of base) : System.Private.CoreLib.dasm - TaskCompletionSource`1:.ctor(Object):this (2 methods)
37 (72.549% of base) : System.Private.CoreLib.dasm - List`1:Remove(__Canon):bool:this
36 (72.000% of base) : System.Private.CoreLib.dasm - AsyncTaskMethodBuilder`1:SetNotificationForWaitCompletion(bool):this
59 (70.238% of base) : System.Private.CoreLib.dasm - TaskCompletionSource`1:.ctor(int):this (2 methods)
157 (69.778% of base) : System.Private.CoreLib.dasm - ValueTuple`4:Equals(Object):bool:this (2 methods)
260 (67.010% of base) : System.Private.CoreLib.dasm - ValueTuple`3:Equals(Object):bool:this (3 methods)
25 (43.103% of base) : System.Private.CoreLib.dasm - HashSet`1:System.Collections.Generic.IEnumerable<T>.GetEnumerator():IEnumerator`1:this
25 (43.103% of base) : System.Private.CoreLib.dasm - HashSet`1:System.Collections.IEnumerable.GetEnumerator():IEnumerator:this
291 (41.044% of base) : System.Private.CoreLib.dasm - ValueTuple`3:System.IComparable.CompareTo(Object):int:this (3 methods)
72 (39.560% of base) : System.Private.CoreLib.dasm - Lazy`1:CreateValue():__Canon:this
173 (28.978% of base) : System.Private.CoreLib.dasm - ValueTask`1:ToString():String:this (3 methods)
47 (27.485% of base) : System.Private.CoreLib.dasm - Lazy`1:ExecutionAndPublication(LazyHelper,bool):this
48 (19.753% of base) : System.Private.CoreLib.dasm - AsyncValueTaskMethodBuilder`1:SetException(Exception):this (2 methods)
18 (19.149% of base) : System.Private.CoreLib.dasm - Collection`1:System.Collections.IList.Contains(Object):bool:this
18 (18.947% of base) : System.Private.CoreLib.dasm - Collection`1:System.Collections.IList.IndexOf(Object):int:this
62 (16.712% of base) : System.Private.CoreLib.dasm - ValueTuple`2:Equals(Object):bool:this (3 methods)
Top method improvements (percentages):
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector128`1:get_DisplayString():String:this
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector256`1:get_DisplayString():String:this
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector64`1:get_DisplayString():String:this
-94 (-14.506% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Implicit(ref):ReadOnlySpan`1 (18 methods)
-73 (-8.120% of base) : System.Private.CoreLib.dasm - Memory`1:Equals(Object):bool:this (4 methods)
-44 (-6.790% of base) : System.Private.CoreLib.dasm - Span`1:op_Implicit(ref):Span`1 (18 methods)
-26 (-3.641% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Inequality(ReadOnlySpan`1,ReadOnlySpan`1):bool (18 methods)
-26 (-3.641% of base) : System.Private.CoreLib.dasm - Span`1:op_Inequality(Span`1,Span`1):bool (18 methods)
-15 (-0.957% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Implicit(ArraySegment`1):ReadOnlySpan`1 (18 methods)
-2 (-0.930% of base) : System.Private.CoreLib.dasm - Memory`1:op_Implicit(ref):Memory`1 (4 methods)
-4 (-0.702% of base) : System.Private.CoreLib.dasm - AsyncValueTaskMethodBuilder`1:get_Task():ValueTask`1:this (2 methods)
-2 (-0.217% of base) : System.Private.CoreLib.dasm - ArraySegment`1:op_Inequality(ArraySegment`1,ArraySegment`1):bool (18 methods)
57 total methods with Code Size differences (12 improved, 45 regressed), 20772 unchanged.

@jkotas

jkotas commented Jun 25, 2020

Copy link
Copy Markdown
MemberAuthor

Example of code-size regression - inlining tends to increase code size on average:

List`1:Contains(KeyValuePair`2):bool:this

Baseline

G_M57469_IG01:
sub rsp, 40
;; bbWeight=1 PerfScore 0.25
G_M57469_IG02:
cmp dword ptr [rcx+16], 0
je SHORT G_M57469_IG05
;; bbWeight=1 PerfScore 3.00
G_M57469_IG03:
call [List`1:IndexOf(KeyValuePair`2):int:this]
cmp eax, -1
setne al
movzx rax, al
;; bbWeight=0.50 PerfScore 2.25
G_M57469_IG04:
add rsp, 40
ret ;; bbWeight=0.50 PerfScore 0.63
G_M57469_IG05:
xor eax, eax
;; bbWeight=0.50 PerfScore 0.13
G_M57469_IG06:
add rsp, 40
ret 

Diff

G_M57469_IG01:
push rdi
push rsi
push rbx
sub rsp, 64
xor rax, rax
mov qword ptr [rsp+28H], rax
mov qword ptr [rsp+38H], rcx
mov rsi, rcx
;; bbWeight=1 PerfScore 5.75
G_M57469_IG02:
cmp dword ptr [rsi+16], 0
je SHORT G_M57469_IG05
;; bbWeight=1 PerfScore 3.00
G_M57469_IG03:
mov rdi, gword ptr [rdx]
mov ebx, dword ptr [rdx+8]
mov rcx, qword ptr [rsi]
call [CORINFO_HELP_READYTORUN_GENERIC_HANDLE]
mov rcx, rax
mov rdx, gword ptr [rsi+8]
lea r8, bword ptr [rsp+28H]
mov gword ptr [r8], rdi
mov dword ptr [r8+8], ebx
mov r8d, dword ptr [rsi+16]
mov dword ptr [rsp+20H], r8d
lea r8, bword ptr [rsp+28H]
xor r9d, r9d
call [Array:IndexOf(ref,KeyValuePair`2,int,int):int]
cmp eax, -1
setne al
movzx rax, al
;; bbWeight=0.50 PerfScore 11.00
G_M57469_IG04:
add rsp, 64
pop rbx
pop rsi
pop rdi
ret ;; bbWeight=0.50 PerfScore 1.38
G_M57469_IG05:
xor eax, eax
;; bbWeight=0.50 PerfScore 0.13
G_M57469_IG06:
add rsp, 64
pop rbx
pop rsi
pop rdi
ret 

@jkotas

Copy link
Copy Markdown
MemberAuthor

Example of code-size improvement - inlining reduces code size when it enables additional constant-prop:

Vector128`1:get_DisplayString

Baseline:

G_M30183_IG01:
push rdi
push rsi
sub rsp, 40
mov qword ptr [rsp+20H], rdx
mov rdi, rcx
mov rsi, rdx
;; bbWeight=1 PerfScore 3.75
G_M30183_IG02:
mov rcx, rsi
call [CORINFO_HELP_READYTORUN_GENERIC_HANDLE]
mov rcx, rax
call [Vector128`1:get_IsSupported():bool]
test al, al
je SHORT G_M30183_IG05
;; bbWeight=1 PerfScore 7.75
G_M30183_IG03:
mov rcx, rsi
call [CORINFO_HELP_READYTORUN_GENERIC_HANDLE]
mov rcx, rdi
call rax
nop ;; bbWeight=0.50 PerfScore 3.38
G_M30183_IG04:
add rsp, 40
pop rsi
pop rdi
ret ;; bbWeight=0.50 PerfScore 1.13
G_M30183_IG05:
mov rcx, qword ptr [(reloc)]
mov rcx, gword ptr [rcx]
mov rdx, qword ptr [(reloc)]
mov rdx, gword ptr [rdx]
call [SR:GetResourceString(String,String):String]
nop ;; bbWeight=0.50 PerfScore 5.63
G_M30183_IG06:
add rsp, 40
pop rsi
pop rdi
ret ;; bbWeight=0.50 PerfScore 1.13

Diff:

G_M30183_IG01:
sub rsp, 40
;; bbWeight=1 PerfScore 0.25
G_M30183_IG02:
mov rcx, qword ptr [(reloc)]
mov rcx, gword ptr [rcx]
mov rdx, qword ptr [(reloc)]
mov rdx, gword ptr [rdx]
call [SR:GetResourceString(String,String):String]
nop ;; bbWeight=1 PerfScore 11.25
G_M30183_IG03:
add rsp, 40
ret 

@nietras

Copy link
Copy Markdown
Contributor

The example improvements are exactly the kind I am hoping for 👍

@jkotas
jkotas marked this pull request as ready for review June 26, 2020 05:52
@jkotas

Copy link
Copy Markdown
MemberAuthor

@dotnet/crossgen-contrib Any crossgen2 specific legs you would recommend to run on this?

@dotnet/jit-contrib Any JIT specific legs you would recommend to run on this?

Comment threadsrc/coreclr/src/tools/Common/Sorting/MergeSortCore.cs
Comment threadsrc/coreclr/src/vm/jitinterface.cpp
@jkotasjkotas changed the title WIP: Enable inlining of shared generics code within same typeEnable inlining of shared generics code within same typeJun 26, 2020

@MichalStrehovskyMichalStrehovsky left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Crossgen2 side looks good to me.

I echo Manish's question about the assert - I think that code might be reachable.

Not commenting on the parallel sort thing because I didn't look at that code before.

@mangod9

Copy link
Copy Markdown
Member

@dotnet/crossgen-contrib Any crossgen2 specific legs you would recommend to run on this?

@dotnet/jit-contrib Any JIT specific legs you would recommend to run on this?

for crossgen2 would be good to run the runtime-coreclr crossgen2 pipeline for validation.

@jkotas

Copy link
Copy Markdown
MemberAuthor

/azp run runtime-coreclr crossgen2

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

@jkotas

Copy link
Copy Markdown
MemberAuthor

for crossgen2 would be good to run the runtime-coreclr crossgen2 pipeline for validation.

@mangod9 What is the right way to trigger it? I have tried to trigger it above, but it does not seem to work.

@mangod9

Copy link
Copy Markdown
Member

for crossgen2 would be good to run the runtime-coreclr crossgen2 pipeline for validation.

@mangod9 What is the right way to trigger it? I have tried to trigger it above, but it does not seem to work.

Hmm.. not sure why that pipeline command fails (probably some config for the pipeline @trylek ?). For now I have manually triggered it via AzDo portal

@BruceForstall

Copy link
Copy Markdown
Contributor

I thought (almost) no triggers were enabled, due to issues with accidentally triggering too many builds. Also, #32777

@davidwrighton

Copy link
Copy Markdown
Member

@jkotas, you can no longer use any of the pipelines from the AzDo command line. You need to find the pipeline in AzDo and manually start a build.

Comment threadsrc/coreclr/src/tools/Common/Sorting/MergeSortCore.cs

@AndyAyersMSAndyAyersMS left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Were those diffs from crossgen or PMI?

Might want to run a wider set of diffs (pass -f to jit-diff) and/or look diffs from both prejitting and jitting.

Comment threadsrc/coreclr/src/jit/importer.cpp
Comment threadsrc/coreclr/src/jit/importer.cpp

@davidwrightondavidwrighton left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like you to split out the work to address the static field address issue into a separate PR, and to provide a better description of under what conditions an inlined shared generic lookup will work and where it won't work.

@jkotasjkotas mentioned this pull request Jun 26, 2020
@jkotas

Copy link
Copy Markdown
MemberAuthor

I'd like you to split out the work to address the static field address issue into a separate PR, and to provide a better description of under what conditions an inlined shared generic lookup will work and where it won't work.

@davidwrighton Done. The updated description is at the top.

@BruceForstall

Copy link
Copy Markdown
Contributor

@dotnet/jit-contrib Any JIT specific legs you would recommend to run on this?

Seems like it would be worthwhile to runtime-coreclr jitstress (https://dev.azure.com/dnceng/public/_build?definitionId=658) to run Pri-1 tests with JitStress=1/2, which would include inlining stress.

@jkotas

Copy link
Copy Markdown
MemberAuthor

Might want to run a wider set of diffs

-f crossgen diffs:

Found 25 files with textual diffs.
Summary of Code Size diffs:
(Lower is better)
Total bytes of diff: 28141 (0.086% of base)
diff is a regression.
Total byte diff includes -111 bytes from reconciling methods
Base had 2 unique methods, 111 unique bytes
Diff had 0 unique methods, 0 unique bytes
Top file regressions (bytes):
12800 : System.Collections.Immutable.dasm (5.998% of base)
5146 : System.Memory.dasm (6.358% of base)
2738 : System.Private.CoreLib.dasm (0.083% of base)
2735 : System.Collections.dasm (2.930% of base)
714 : System.Collections.Concurrent.dasm (1.559% of base)
587 : Microsoft.CodeAnalysis.dasm (0.078% of base)
473 : System.Reflection.Metadata.dasm (0.147% of base)
468 : System.Linq.dasm (0.353% of base)
298 : System.Threading.Tasks.Dataflow.dasm (0.217% of base)
282 : System.Linq.Expressions.dasm (0.010% of base)
266 : System.ObjectModel.dasm (1.353% of base)
202 : Microsoft.CSharp.dasm (0.062% of base)
179 : System.DirectoryServices.AccountManagement.dasm (0.072% of base)
171 : Microsoft.Diagnostics.Tracing.TraceEvent.dasm (0.005% of base)
153 : Newtonsoft.Json.dasm (0.025% of base)
138 : Microsoft.Extensions.Logging.Abstractions.dasm (0.565% of base)
135 : Microsoft.Diagnostics.FastSerialization.dasm (0.401% of base)
126 : System.Private.Xml.dasm (0.004% of base)
116 : System.Linq.Parallel.dasm (0.020% of base)
108 : Microsoft.CodeAnalysis.CSharp.dasm (0.005% of base)
25 total files with Code Size differences (0 improved, 25 regressed), 239 unchanged.
Top method regressions (bytes):
554 (45.975% of base) : System.Memory.dasm - ReadOnlySequence`1:TryGet(byref,byref,bool):bool:this (3 methods)
543 (35.560% of base) : System.Memory.dasm - SequenceReader`1:TryReadTo(byref,__Canon,__Canon,bool):bool:this (2 methods)
437 (108.168% of base) : System.Collections.Immutable.dasm - Builder:System.Collections.IEnumerable.GetEnumerator():IEnumerator:this (8 methods)
422 (28.964% of base) : System.Memory.dasm - SequenceReader`1:TryReadToSlow(byref,__Canon,__Canon,int,bool):bool:this (2 methods)
347 (10.534% of base) : System.Collections.Immutable.dasm - Enumerator:MoveNext():bool:this (15 methods)
344 (148.276% of base) : System.Collections.Immutable.dasm - Node:System.Collections.IEnumerable.GetEnumerator():IEnumerator:this (4 methods)
336 (13.925% of base) : System.Memory.dasm - ReadOnlySequence`1:Slice(long,long):ReadOnlySequence`1:this (3 methods)
330 (23.827% of base) : System.Collections.Immutable.dasm - Enumerator:Reset():this (11 methods)
323 (21.824% of base) : System.Collections.Immutable.dasm - Builder:AddRange(IEnumerable`1):this (6 methods)
291 (31.699% of base) : System.Memory.dasm - ReadOnlySequence`1:Slice(long):ReadOnlySequence`1:this (3 methods)
291 (41.044% of base) : System.Private.CoreLib.dasm - ValueTuple`3:System.IComparable.CompareTo(Object):int:this (3 methods)
287 (38.013% of base) : System.Reflection.Metadata.dasm - SignatureDecoder`2:DecodeType(byref,bool,int):__Canon:this
286 (54.476% of base) : System.Memory.dasm - ReadOnlySequence`1:get_FirstSpan():ReadOnlySpan`1:this (3 methods)
281 (19.957% of base) : System.Memory.dasm - SequenceReader`1:TryReadTo(byref,ReadOnlySpan`1,bool):bool:this (2 methods)
275 (158.046% of base) : System.Collections.Immutable.dasm - Node:System.Collections.Generic.IEnumerable<T>.GetEnumerator():IEnumerator`1:this (3 methods)
268 (52.652% of base) : System.Memory.dasm - ReadOnlySequence`1:get_First():ReadOnlyMemory`1:this (3 methods)
268 (41.358% of base) : System.Memory.dasm - ReadOnlySequence`1:GetPosition(long,SequencePosition):SequencePosition:this (3 methods)
267 (41.654% of base) : System.Memory.dasm - ReadOnlySequence`1:GetPosition(long):SequencePosition:this (3 methods)
266 (95.341% of base) : System.Collections.Immutable.dasm - Builder:System.Collections.Generic.IEnumerable<T>.GetEnumerator():IEnumerator`1:this (6 methods)
265 (24.953% of base) : System.Collections.Immutable.dasm - Node:CopyTo(ref,int):this (3 methods)
Top method improvements (bytes):
-169 (-26.365% of base) : System.Collections.Immutable.dasm - ImmutableArray`1:InsertRange(int,ImmutableArray`1):ImmutableArray`1:this (2 methods)
-145 (-27.672% of base) : System.Collections.Immutable.dasm - Node:GetEnumerator():Enumerator:this (4 base, 3 diff methods)
-94 (-14.506% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Implicit(ref):ReadOnlySpan`1 (18 methods)
-82 (-100.000% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:CreatePair(__Canon,Pair`2):Pair`2 (1 base, 0 diff methods)
-73 (-8.120% of base) : System.Private.CoreLib.dasm - Memory`1:Equals(Object):bool:this (4 methods)
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector128`1:get_DisplayString():String:this
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector256`1:get_DisplayString():String:this
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector64`1:get_DisplayString():String:this
-56 (-29.474% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:Add(__Canon,int):bool:this
-52 (-27.083% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:Add(__Canon,__Canon):bool:this
-44 (-6.790% of base) : System.Private.CoreLib.dasm - Span`1:op_Implicit(ref):Span`1 (18 methods)
-42 (-7.420% of base) : System.Collections.Immutable.dasm - ImmutableArray`1:RemoveAll(Predicate`1):ImmutableArray`1:this (2 methods)
-32 (-46.377% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:.ctor(__Canon,int):this
-29 (-100.000% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:CreatePair(__Canon,int):Pair`2 (1 base, 0 diff methods)
-26 (-3.641% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Inequality(ReadOnlySpan`1,ReadOnlySpan`1):bool (18 methods)
-26 (-3.641% of base) : System.Private.CoreLib.dasm - Span`1:op_Inequality(Span`1,Span`1):bool (18 methods)
-22 (-31.884% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:.ctor(__Canon,__Canon):this
-21 (-19.444% of base) : System.Linq.Parallel.dasm - OrderedGroupJoinHashLookup:CreateValuePair(GroupKeyData):Pair`2:this
-19 (-11.515% of base) : Microsoft.CodeAnalysis.CSharp.dasm - SyntaxList`1:Any(ushort):bool:this
-19 (-13.475% of base) : System.Linq.dasm - SparseArrayBuilder`1:Reserve(int):this
Top method regressions (percentages):
126 (787.500% of base) : System.DirectoryServices.AccountManagement.dasm - TrackedCollection`1:System.Collections.IEnumerable.GetEnumerator():IEnumerator:this
92 (575.000% of base) : System.Collections.Immutable.dasm - ImmutableList`1:System.Collections.Immutable.IImmutableList<T>.Remove(KeyValuePair`2,IEqualityComparer`1):IImmutableList`1:this
63 (393.750% of base) : System.Collections.dasm - SortedList`2:get_Keys():IList`1:this
63 (393.750% of base) : System.Collections.dasm - SortedList`2:System.Collections.Generic.IDictionary<TKey,TValue>.get_Keys():ICollection`1:this
63 (393.750% of base) : System.Collections.dasm - SortedList`2:System.Collections.IDictionary.get_Keys():ICollection:this
63 (393.750% of base) : System.Collections.dasm - SortedList`2:System.Collections.Generic.IReadOnlyDictionary<TKey,TValue>.get_Keys():IEnumerable`1:this
63 (393.750% of base) : System.Collections.dasm - SortedList`2:get_Values():IList`1:this
63 (393.750% of base) : System.Collections.dasm - SortedList`2:System.Collections.Generic.IDictionary<TKey,TValue>.get_Values():ICollection`1:this
63 (393.750% of base) : System.Collections.dasm - SortedList`2:System.Collections.IDictionary.get_Values():ICollection:this
63 (393.750% of base) : System.Collections.dasm - SortedList`2:System.Collections.Generic.IReadOnlyDictionary<TKey,TValue>.get_Values():IEnumerable`1:this
62 (387.500% of base) : System.Collections.dasm - SortedDictionary`2:System.Collections.Generic.IDictionary<TKey,TValue>.get_Keys():ICollection`1:this
62 (387.500% of base) : System.Collections.dasm - SortedDictionary`2:System.Collections.Generic.IReadOnlyDictionary<TKey,TValue>.get_Keys():IEnumerable`1:this
62 (387.500% of base) : System.Collections.dasm - SortedDictionary`2:System.Collections.Generic.IDictionary<TKey,TValue>.get_Values():ICollection`1:this
62 (387.500% of base) : System.Collections.dasm - SortedDictionary`2:System.Collections.Generic.IReadOnlyDictionary<TKey,TValue>.get_Values():IEnumerable`1:this
62 (387.500% of base) : System.Collections.dasm - SortedDictionary`2:System.Collections.IDictionary.get_Keys():ICollection:this
62 (387.500% of base) : System.Collections.dasm - SortedDictionary`2:System.Collections.IDictionary.get_Values():ICollection:this
61 (381.250% of base) : Newtonsoft.Json.dasm - EnumerableDictionaryWrapper`2:System.Collections.IEnumerable.GetEnumerator():IEnumerator:this
61 (381.250% of base) : System.Net.Http.dasm - HttpHeaderValueCollection`1:System.Collections.IEnumerable.GetEnumerator():IEnumerator:this
60 (375.000% of base) : System.Collections.Immutable.dasm - ImmutableList`1:System.Collections.Immutable.IImmutableList<T>.Remove(__Canon,IEqualityComparer`1):IImmutableList`1:this
60 (375.000% of base) : System.Collections.Immutable.dasm - ImmutableStack`1:System.Collections.Immutable.IImmutableStack<T>.Push(__Canon):IImmutableStack`1:this
Top method improvements (percentages):
-82 (-100.000% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:CreatePair(__Canon,Pair`2):Pair`2 (1 base, 0 diff methods)
-29 (-100.000% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:CreatePair(__Canon,int):Pair`2 (1 base, 0 diff methods)
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector128`1:get_DisplayString():String:this
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector256`1:get_DisplayString():String:this
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector64`1:get_DisplayString():String:this
-32 (-46.377% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:.ctor(__Canon,int):this
-22 (-31.884% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:.ctor(__Canon,__Canon):this
-56 (-29.474% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:Add(__Canon,int):bool:this
-145 (-27.672% of base) : System.Collections.Immutable.dasm - Node:GetEnumerator():Enumerator:this (4 base, 3 diff methods)
-52 (-27.083% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:Add(__Canon,__Canon):bool:this
-169 (-26.365% of base) : System.Collections.Immutable.dasm - ImmutableArray`1:InsertRange(int,ImmutableArray`1):ImmutableArray`1:this (2 methods)
-21 (-19.444% of base) : System.Linq.Parallel.dasm - OrderedGroupJoinHashLookup:CreateValuePair(GroupKeyData):Pair`2:this
-94 (-14.506% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Implicit(ref):ReadOnlySpan`1 (18 methods)
-19 (-13.475% of base) : System.Linq.dasm - SparseArrayBuilder`1:Reserve(int):this
-18 (-12.950% of base) : System.Collections.Immutable.dasm - ImmutableDictionary`2:TryGetValue(__Canon,byref):bool:this
-18 (-12.950% of base) : System.Collections.Immutable.dasm - ImmutableDictionary`2:TryGetKey(__Canon,byref):bool:this
-9 (-12.857% of base) : Microsoft.CodeAnalysis.VisualBasic.dasm - _Closure$__17`1:.cctor()
-13 (-12.745% of base) : System.Collections.Immutable.dasm - ImmutableDictionary`2:Contains(KeyValuePair`2):bool:this
-19 (-11.515% of base) : Microsoft.CodeAnalysis.CSharp.dasm - SyntaxList`1:Any(ushort):bool:this
-13 (-10.484% of base) : System.Collections.Immutable.dasm - ImmutableDictionary`2:ContainsKey(__Canon):bool:this
406 total methods with Code Size differences (43 improved, 363 regressed), 186975 unchanged.

@jkotas

Copy link
Copy Markdown
MemberAuthor
  • ReadOnlySequence size regressions is caused by AggresiveInlining on ~70 lines method
  • ImmutableCollections size regression is a lots of small methods that are getting inlined into each other now

@jkotas

Copy link
Copy Markdown
MemberAuthor

JIT stress failure is known issue #36681
Installer Build and Test coreclr failure is known issue #35068
oreCLR Pri0 Runtime Tests Run Windows_NT x86 checked is known issue #38284

@jkotas
jkotas merged commit 31699de into dotnet:masterJun 27, 2020
@jkotas
jkotas deleted the generics-inlining branch June 27, 2020 13:36
@nietras

Copy link
Copy Markdown
Contributor

Great to see this merged. 👍

when the caller and callee are the same exact type and instantiation.

This is quite clear but to be sure, I assume this means this won't work for base class methods for example? E. g. called in a derived class?

@jkotas

Copy link
Copy Markdown
MemberAuthor

I assume this means this won't work for base class methods for example? E. g. called in a derived class?

Correct.

@ghostghost locked as resolved and limited conversation to collaborators Dec 8, 2020
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

11 participants

@jkotas@nietras@mangod9@BruceForstall@davidwrighton@EgorBo@stephentoub@jakobbotsch@AndyAyersMS@MichalStrehovsky@Dotnet-GitSync-Bot
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

Enable inlining of shared generics code within same type - #38229

Merged
jkotas merged 11 commits into
dotnet:masterfrom
jkotas:generics-inlining
Jun 27, 2020
Merged

Enable inlining of shared generics code within same type#38229
jkotas merged 11 commits into
dotnet:masterfrom
jkotas:generics-inlining

Conversation

@jkotas

@jkotasjkotas commented Jun 22, 2020

Copy link
Copy Markdown
Member

This change allows inlining of generic dictionary lookups when the caller and callee are the same exact type and instantiation. It is a simpler version of #31833.

Motivation for this change is to unblock #37941.

Example:

class G<T>
{
static bool M1() => typeof(T);
object M2() => new T[1];
bool M3() => Unsafe.SizeOf<T>();
static string M()
{
// Assume that T is string
// All M1, M2, M3 are inlined after this change, but were not inlined before this change
M1(); M2(); M3();
// Still not inlined - different instantiation, not the same exact type
G<List<T>>.M1();
// Still not inlined - different type.
OtherG<T>.M();
}
}

@jkotas
jkotas marked this pull request as draft June 22, 2020 19:22
@jkotas
jkotasforce-pushed the generics-inlining branch from 9645520 to dd93ffaCompareJune 23, 2020 06:58
@jaredparjaredpar mentioned this pull request Jun 23, 2020
@jkotas
jkotasforce-pushed the generics-inlining branch 3 times, most recently from 8cbb058 to 3770ae3CompareJune 24, 2020 16:20
Comment threadsrc/coreclr/src/ToolBox/superpmi/superpmi-shared/methodcontext.cpp Outdated
@jkotas
jkotasforce-pushed the generics-inlining branch from 3819aa0 to d09090eCompareJune 24, 2020 23:22
@jkotas

Copy link
Copy Markdown
MemberAuthor

Asm diffs

Total bytes of diff: 2738 (0.083% of base)
diff is a regression.
Top file regressions (bytes):
2738 : System.Private.CoreLib.dasm (0.083% of base)
1 total files with Code Size differences (0 improved, 1 regressed), 0 unchanged.
Top method regressions (bytes):
291 (41.044% of base) : System.Private.CoreLib.dasm - ValueTuple`3:System.IComparable.CompareTo(Object):int:this (3 methods)
260 (67.010% of base) : System.Private.CoreLib.dasm - ValueTuple`3:Equals(Object):bool:this (3 methods)
184 (3.340% of base) : System.Private.CoreLib.dasm - GenericArraySortHelper`1:IntroSort(Span`1,int) (12 methods)
173 (28.978% of base) : System.Private.CoreLib.dasm - ValueTask`1:ToString():String:this (3 methods)
159 (15.071% of base) : System.Private.CoreLib.dasm - Memory`1:CopyTo(Memory`1):this (4 methods)
159 (14.171% of base) : System.Private.CoreLib.dasm - Memory`1:TryCopyTo(Memory`1):bool:this (4 methods)
157 (69.778% of base) : System.Private.CoreLib.dasm - ValueTuple`4:Equals(Object):bool:this (2 methods)
118 (6.833% of base) : System.Private.CoreLib.dasm - Vector128`1:Equals(Object):bool:this (11 methods)
118 (3.562% of base) : System.Private.CoreLib.dasm - GenericArraySortHelper`1:PickPivotAndPartition(Span`1):int (12 methods)
94 (8.818% of base) : System.Private.CoreLib.dasm - ReadOnlyMemory`1:CopyTo(Memory`1):this (4 methods)
94 (8.297% of base) : System.Private.CoreLib.dasm - ReadOnlyMemory`1:TryCopyTo(Memory`1):bool:this (4 methods)
87 (13.344% of base) : System.Private.CoreLib.dasm - Memory`1:ToArray():ref:this (4 methods)
87 (14.846% of base) : System.Private.CoreLib.dasm - ReadOnlyMemory`1:ToArray():ref:this (4 methods)
76 (8.736% of base) : System.Private.CoreLib.dasm - List`1:System.Collections.IList.IndexOf(Object):int:this (8 methods)
75 (202.703% of base) : System.Private.CoreLib.dasm - List`1:Contains(KeyValuePair`2):bool:this
72 (39.560% of base) : System.Private.CoreLib.dasm - Lazy`1:CreateValue():__Canon:this
66 (129.412% of base) : System.Private.CoreLib.dasm - List`1:Remove(KeyValuePair`2):bool:this
63 (75.000% of base) : System.Private.CoreLib.dasm - TaskCompletionSource`1:.ctor(Object):this (2 methods)
62 (16.712% of base) : System.Private.CoreLib.dasm - ValueTuple`2:Equals(Object):bool:this (3 methods)
60 (8.310% of base) : System.Private.CoreLib.dasm - ValueTuple`2:System.IComparable.CompareTo(Object):int:this (3 methods)
Top method improvements (bytes):
-94 (-14.506% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Implicit(ref):ReadOnlySpan`1 (18 methods)
-73 (-8.120% of base) : System.Private.CoreLib.dasm - Memory`1:Equals(Object):bool:this (4 methods)
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector128`1:get_DisplayString():String:this
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector256`1:get_DisplayString():String:this
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector64`1:get_DisplayString():String:this
-44 (-6.790% of base) : System.Private.CoreLib.dasm - Span`1:op_Implicit(ref):Span`1 (18 methods)
-26 (-3.641% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Inequality(ReadOnlySpan`1,ReadOnlySpan`1):bool (18 methods)
-26 (-3.641% of base) : System.Private.CoreLib.dasm - Span`1:op_Inequality(Span`1,Span`1):bool (18 methods)
-15 (-0.957% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Implicit(ArraySegment`1):ReadOnlySpan`1 (18 methods)
-4 (-0.702% of base) : System.Private.CoreLib.dasm - AsyncValueTaskMethodBuilder`1:get_Task():ValueTask`1:this (2 methods)
-2 (-0.217% of base) : System.Private.CoreLib.dasm - ArraySegment`1:op_Inequality(ArraySegment`1,ArraySegment`1):bool (18 methods)
-2 (-0.930% of base) : System.Private.CoreLib.dasm - Memory`1:op_Implicit(ref):Memory`1 (4 methods)
Top method regressions (percentages):
47 (293.750% of base) : System.Private.CoreLib.dasm - SingleProducerSingleConsumerQueue`1:System.Collections.IEnumerable.GetEnumerator():IEnumerator:this
75 (202.703% of base) : System.Private.CoreLib.dasm - List`1:Contains(KeyValuePair`2):bool:this
66 (129.412% of base) : System.Private.CoreLib.dasm - List`1:Remove(KeyValuePair`2):bool:this
46 (124.324% of base) : System.Private.CoreLib.dasm - List`1:Contains(__Canon):bool:this
63 (75.000% of base) : System.Private.CoreLib.dasm - TaskCompletionSource`1:.ctor(Object):this (2 methods)
37 (72.549% of base) : System.Private.CoreLib.dasm - List`1:Remove(__Canon):bool:this
36 (72.000% of base) : System.Private.CoreLib.dasm - AsyncTaskMethodBuilder`1:SetNotificationForWaitCompletion(bool):this
59 (70.238% of base) : System.Private.CoreLib.dasm - TaskCompletionSource`1:.ctor(int):this (2 methods)
157 (69.778% of base) : System.Private.CoreLib.dasm - ValueTuple`4:Equals(Object):bool:this (2 methods)
260 (67.010% of base) : System.Private.CoreLib.dasm - ValueTuple`3:Equals(Object):bool:this (3 methods)
25 (43.103% of base) : System.Private.CoreLib.dasm - HashSet`1:System.Collections.Generic.IEnumerable<T>.GetEnumerator():IEnumerator`1:this
25 (43.103% of base) : System.Private.CoreLib.dasm - HashSet`1:System.Collections.IEnumerable.GetEnumerator():IEnumerator:this
291 (41.044% of base) : System.Private.CoreLib.dasm - ValueTuple`3:System.IComparable.CompareTo(Object):int:this (3 methods)
72 (39.560% of base) : System.Private.CoreLib.dasm - Lazy`1:CreateValue():__Canon:this
173 (28.978% of base) : System.Private.CoreLib.dasm - ValueTask`1:ToString():String:this (3 methods)
47 (27.485% of base) : System.Private.CoreLib.dasm - Lazy`1:ExecutionAndPublication(LazyHelper,bool):this
48 (19.753% of base) : System.Private.CoreLib.dasm - AsyncValueTaskMethodBuilder`1:SetException(Exception):this (2 methods)
18 (19.149% of base) : System.Private.CoreLib.dasm - Collection`1:System.Collections.IList.Contains(Object):bool:this
18 (18.947% of base) : System.Private.CoreLib.dasm - Collection`1:System.Collections.IList.IndexOf(Object):int:this
62 (16.712% of base) : System.Private.CoreLib.dasm - ValueTuple`2:Equals(Object):bool:this (3 methods)
Top method improvements (percentages):
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector128`1:get_DisplayString():String:this
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector256`1:get_DisplayString():String:this
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector64`1:get_DisplayString():String:this
-94 (-14.506% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Implicit(ref):ReadOnlySpan`1 (18 methods)
-73 (-8.120% of base) : System.Private.CoreLib.dasm - Memory`1:Equals(Object):bool:this (4 methods)
-44 (-6.790% of base) : System.Private.CoreLib.dasm - Span`1:op_Implicit(ref):Span`1 (18 methods)
-26 (-3.641% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Inequality(ReadOnlySpan`1,ReadOnlySpan`1):bool (18 methods)
-26 (-3.641% of base) : System.Private.CoreLib.dasm - Span`1:op_Inequality(Span`1,Span`1):bool (18 methods)
-15 (-0.957% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Implicit(ArraySegment`1):ReadOnlySpan`1 (18 methods)
-2 (-0.930% of base) : System.Private.CoreLib.dasm - Memory`1:op_Implicit(ref):Memory`1 (4 methods)
-4 (-0.702% of base) : System.Private.CoreLib.dasm - AsyncValueTaskMethodBuilder`1:get_Task():ValueTask`1:this (2 methods)
-2 (-0.217% of base) : System.Private.CoreLib.dasm - ArraySegment`1:op_Inequality(ArraySegment`1,ArraySegment`1):bool (18 methods)
57 total methods with Code Size differences (12 improved, 45 regressed), 20772 unchanged.

@jkotas

jkotas commented Jun 25, 2020

Copy link
Copy Markdown
MemberAuthor

Example of code-size regression - inlining tends to increase code size on average:

List`1:Contains(KeyValuePair`2):bool:this

Baseline

G_M57469_IG01:
sub rsp, 40
;; bbWeight=1 PerfScore 0.25
G_M57469_IG02:
cmp dword ptr [rcx+16], 0
je SHORT G_M57469_IG05
;; bbWeight=1 PerfScore 3.00
G_M57469_IG03:
call [List`1:IndexOf(KeyValuePair`2):int:this]
cmp eax, -1
setne al
movzx rax, al
;; bbWeight=0.50 PerfScore 2.25
G_M57469_IG04:
add rsp, 40
ret ;; bbWeight=0.50 PerfScore 0.63
G_M57469_IG05:
xor eax, eax
;; bbWeight=0.50 PerfScore 0.13
G_M57469_IG06:
add rsp, 40
ret 

Diff

G_M57469_IG01:
push rdi
push rsi
push rbx
sub rsp, 64
xor rax, rax
mov qword ptr [rsp+28H], rax
mov qword ptr [rsp+38H], rcx
mov rsi, rcx
;; bbWeight=1 PerfScore 5.75
G_M57469_IG02:
cmp dword ptr [rsi+16], 0
je SHORT G_M57469_IG05
;; bbWeight=1 PerfScore 3.00
G_M57469_IG03:
mov rdi, gword ptr [rdx]
mov ebx, dword ptr [rdx+8]
mov rcx, qword ptr [rsi]
call [CORINFO_HELP_READYTORUN_GENERIC_HANDLE]
mov rcx, rax
mov rdx, gword ptr [rsi+8]
lea r8, bword ptr [rsp+28H]
mov gword ptr [r8], rdi
mov dword ptr [r8+8], ebx
mov r8d, dword ptr [rsi+16]
mov dword ptr [rsp+20H], r8d
lea r8, bword ptr [rsp+28H]
xor r9d, r9d
call [Array:IndexOf(ref,KeyValuePair`2,int,int):int]
cmp eax, -1
setne al
movzx rax, al
;; bbWeight=0.50 PerfScore 11.00
G_M57469_IG04:
add rsp, 64
pop rbx
pop rsi
pop rdi
ret ;; bbWeight=0.50 PerfScore 1.38
G_M57469_IG05:
xor eax, eax
;; bbWeight=0.50 PerfScore 0.13
G_M57469_IG06:
add rsp, 64
pop rbx
pop rsi
pop rdi
ret 

@jkotas

Copy link
Copy Markdown
MemberAuthor

Example of code-size improvement - inlining reduces code size when it enables additional constant-prop:

Vector128`1:get_DisplayString

Baseline:

G_M30183_IG01:
push rdi
push rsi
sub rsp, 40
mov qword ptr [rsp+20H], rdx
mov rdi, rcx
mov rsi, rdx
;; bbWeight=1 PerfScore 3.75
G_M30183_IG02:
mov rcx, rsi
call [CORINFO_HELP_READYTORUN_GENERIC_HANDLE]
mov rcx, rax
call [Vector128`1:get_IsSupported():bool]
test al, al
je SHORT G_M30183_IG05
;; bbWeight=1 PerfScore 7.75
G_M30183_IG03:
mov rcx, rsi
call [CORINFO_HELP_READYTORUN_GENERIC_HANDLE]
mov rcx, rdi
call rax
nop ;; bbWeight=0.50 PerfScore 3.38
G_M30183_IG04:
add rsp, 40
pop rsi
pop rdi
ret ;; bbWeight=0.50 PerfScore 1.13
G_M30183_IG05:
mov rcx, qword ptr [(reloc)]
mov rcx, gword ptr [rcx]
mov rdx, qword ptr [(reloc)]
mov rdx, gword ptr [rdx]
call [SR:GetResourceString(String,String):String]
nop ;; bbWeight=0.50 PerfScore 5.63
G_M30183_IG06:
add rsp, 40
pop rsi
pop rdi
ret ;; bbWeight=0.50 PerfScore 1.13

Diff:

G_M30183_IG01:
sub rsp, 40
;; bbWeight=1 PerfScore 0.25
G_M30183_IG02:
mov rcx, qword ptr [(reloc)]
mov rcx, gword ptr [rcx]
mov rdx, qword ptr [(reloc)]
mov rdx, gword ptr [rdx]
call [SR:GetResourceString(String,String):String]
nop ;; bbWeight=1 PerfScore 11.25
G_M30183_IG03:
add rsp, 40
ret 

@nietras

Copy link
Copy Markdown
Contributor

The example improvements are exactly the kind I am hoping for 👍

@jkotas
jkotas marked this pull request as ready for review June 26, 2020 05:52
@jkotas

Copy link
Copy Markdown
MemberAuthor

@dotnet/crossgen-contrib Any crossgen2 specific legs you would recommend to run on this?

@dotnet/jit-contrib Any JIT specific legs you would recommend to run on this?

Comment threadsrc/coreclr/src/tools/Common/Sorting/MergeSortCore.cs
Comment threadsrc/coreclr/src/vm/jitinterface.cpp
@jkotasjkotas changed the title WIP: Enable inlining of shared generics code within same typeEnable inlining of shared generics code within same typeJun 26, 2020

@MichalStrehovskyMichalStrehovsky left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Crossgen2 side looks good to me.

I echo Manish's question about the assert - I think that code might be reachable.

Not commenting on the parallel sort thing because I didn't look at that code before.

@mangod9

Copy link
Copy Markdown
Member

@dotnet/crossgen-contrib Any crossgen2 specific legs you would recommend to run on this?

@dotnet/jit-contrib Any JIT specific legs you would recommend to run on this?

for crossgen2 would be good to run the runtime-coreclr crossgen2 pipeline for validation.

@jkotas

Copy link
Copy Markdown
MemberAuthor

/azp run runtime-coreclr crossgen2

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

@jkotas

Copy link
Copy Markdown
MemberAuthor

for crossgen2 would be good to run the runtime-coreclr crossgen2 pipeline for validation.

@mangod9 What is the right way to trigger it? I have tried to trigger it above, but it does not seem to work.

@mangod9

Copy link
Copy Markdown
Member

for crossgen2 would be good to run the runtime-coreclr crossgen2 pipeline for validation.

@mangod9 What is the right way to trigger it? I have tried to trigger it above, but it does not seem to work.

Hmm.. not sure why that pipeline command fails (probably some config for the pipeline @trylek ?). For now I have manually triggered it via AzDo portal

@BruceForstall

Copy link
Copy Markdown
Contributor

I thought (almost) no triggers were enabled, due to issues with accidentally triggering too many builds. Also, #32777

@davidwrighton

Copy link
Copy Markdown
Member

@jkotas, you can no longer use any of the pipelines from the AzDo command line. You need to find the pipeline in AzDo and manually start a build.

Comment threadsrc/coreclr/src/tools/Common/Sorting/MergeSortCore.cs

@AndyAyersMSAndyAyersMS left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Were those diffs from crossgen or PMI?

Might want to run a wider set of diffs (pass -f to jit-diff) and/or look diffs from both prejitting and jitting.

Comment threadsrc/coreclr/src/jit/importer.cpp
Comment threadsrc/coreclr/src/jit/importer.cpp

@davidwrightondavidwrighton left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like you to split out the work to address the static field address issue into a separate PR, and to provide a better description of under what conditions an inlined shared generic lookup will work and where it won't work.

@jkotasjkotas mentioned this pull request Jun 26, 2020
@jkotas

Copy link
Copy Markdown
MemberAuthor

I'd like you to split out the work to address the static field address issue into a separate PR, and to provide a better description of under what conditions an inlined shared generic lookup will work and where it won't work.

@davidwrighton Done. The updated description is at the top.

@BruceForstall

Copy link
Copy Markdown
Contributor

@dotnet/jit-contrib Any JIT specific legs you would recommend to run on this?

Seems like it would be worthwhile to runtime-coreclr jitstress (https://dev.azure.com/dnceng/public/_build?definitionId=658) to run Pri-1 tests with JitStress=1/2, which would include inlining stress.

@jkotas

Copy link
Copy Markdown
MemberAuthor

Might want to run a wider set of diffs

-f crossgen diffs:

Found 25 files with textual diffs.
Summary of Code Size diffs:
(Lower is better)
Total bytes of diff: 28141 (0.086% of base)
diff is a regression.
Total byte diff includes -111 bytes from reconciling methods
Base had 2 unique methods, 111 unique bytes
Diff had 0 unique methods, 0 unique bytes
Top file regressions (bytes):
12800 : System.Collections.Immutable.dasm (5.998% of base)
5146 : System.Memory.dasm (6.358% of base)
2738 : System.Private.CoreLib.dasm (0.083% of base)
2735 : System.Collections.dasm (2.930% of base)
714 : System.Collections.Concurrent.dasm (1.559% of base)
587 : Microsoft.CodeAnalysis.dasm (0.078% of base)
473 : System.Reflection.Metadata.dasm (0.147% of base)
468 : System.Linq.dasm (0.353% of base)
298 : System.Threading.Tasks.Dataflow.dasm (0.217% of base)
282 : System.Linq.Expressions.dasm (0.010% of base)
266 : System.ObjectModel.dasm (1.353% of base)
202 : Microsoft.CSharp.dasm (0.062% of base)
179 : System.DirectoryServices.AccountManagement.dasm (0.072% of base)
171 : Microsoft.Diagnostics.Tracing.TraceEvent.dasm (0.005% of base)
153 : Newtonsoft.Json.dasm (0.025% of base)
138 : Microsoft.Extensions.Logging.Abstractions.dasm (0.565% of base)
135 : Microsoft.Diagnostics.FastSerialization.dasm (0.401% of base)
126 : System.Private.Xml.dasm (0.004% of base)
116 : System.Linq.Parallel.dasm (0.020% of base)
108 : Microsoft.CodeAnalysis.CSharp.dasm (0.005% of base)
25 total files with Code Size differences (0 improved, 25 regressed), 239 unchanged.
Top method regressions (bytes):
554 (45.975% of base) : System.Memory.dasm - ReadOnlySequence`1:TryGet(byref,byref,bool):bool:this (3 methods)
543 (35.560% of base) : System.Memory.dasm - SequenceReader`1:TryReadTo(byref,__Canon,__Canon,bool):bool:this (2 methods)
437 (108.168% of base) : System.Collections.Immutable.dasm - Builder:System.Collections.IEnumerable.GetEnumerator():IEnumerator:this (8 methods)
422 (28.964% of base) : System.Memory.dasm - SequenceReader`1:TryReadToSlow(byref,__Canon,__Canon,int,bool):bool:this (2 methods)
347 (10.534% of base) : System.Collections.Immutable.dasm - Enumerator:MoveNext():bool:this (15 methods)
344 (148.276% of base) : System.Collections.Immutable.dasm - Node:System.Collections.IEnumerable.GetEnumerator():IEnumerator:this (4 methods)
336 (13.925% of base) : System.Memory.dasm - ReadOnlySequence`1:Slice(long,long):ReadOnlySequence`1:this (3 methods)
330 (23.827% of base) : System.Collections.Immutable.dasm - Enumerator:Reset():this (11 methods)
323 (21.824% of base) : System.Collections.Immutable.dasm - Builder:AddRange(IEnumerable`1):this (6 methods)
291 (31.699% of base) : System.Memory.dasm - ReadOnlySequence`1:Slice(long):ReadOnlySequence`1:this (3 methods)
291 (41.044% of base) : System.Private.CoreLib.dasm - ValueTuple`3:System.IComparable.CompareTo(Object):int:this (3 methods)
287 (38.013% of base) : System.Reflection.Metadata.dasm - SignatureDecoder`2:DecodeType(byref,bool,int):__Canon:this
286 (54.476% of base) : System.Memory.dasm - ReadOnlySequence`1:get_FirstSpan():ReadOnlySpan`1:this (3 methods)
281 (19.957% of base) : System.Memory.dasm - SequenceReader`1:TryReadTo(byref,ReadOnlySpan`1,bool):bool:this (2 methods)
275 (158.046% of base) : System.Collections.Immutable.dasm - Node:System.Collections.Generic.IEnumerable<T>.GetEnumerator():IEnumerator`1:this (3 methods)
268 (52.652% of base) : System.Memory.dasm - ReadOnlySequence`1:get_First():ReadOnlyMemory`1:this (3 methods)
268 (41.358% of base) : System.Memory.dasm - ReadOnlySequence`1:GetPosition(long,SequencePosition):SequencePosition:this (3 methods)
267 (41.654% of base) : System.Memory.dasm - ReadOnlySequence`1:GetPosition(long):SequencePosition:this (3 methods)
266 (95.341% of base) : System.Collections.Immutable.dasm - Builder:System.Collections.Generic.IEnumerable<T>.GetEnumerator():IEnumerator`1:this (6 methods)
265 (24.953% of base) : System.Collections.Immutable.dasm - Node:CopyTo(ref,int):this (3 methods)
Top method improvements (bytes):
-169 (-26.365% of base) : System.Collections.Immutable.dasm - ImmutableArray`1:InsertRange(int,ImmutableArray`1):ImmutableArray`1:this (2 methods)
-145 (-27.672% of base) : System.Collections.Immutable.dasm - Node:GetEnumerator():Enumerator:this (4 base, 3 diff methods)
-94 (-14.506% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Implicit(ref):ReadOnlySpan`1 (18 methods)
-82 (-100.000% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:CreatePair(__Canon,Pair`2):Pair`2 (1 base, 0 diff methods)
-73 (-8.120% of base) : System.Private.CoreLib.dasm - Memory`1:Equals(Object):bool:this (4 methods)
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector128`1:get_DisplayString():String:this
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector256`1:get_DisplayString():String:this
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector64`1:get_DisplayString():String:this
-56 (-29.474% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:Add(__Canon,int):bool:this
-52 (-27.083% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:Add(__Canon,__Canon):bool:this
-44 (-6.790% of base) : System.Private.CoreLib.dasm - Span`1:op_Implicit(ref):Span`1 (18 methods)
-42 (-7.420% of base) : System.Collections.Immutable.dasm - ImmutableArray`1:RemoveAll(Predicate`1):ImmutableArray`1:this (2 methods)
-32 (-46.377% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:.ctor(__Canon,int):this
-29 (-100.000% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:CreatePair(__Canon,int):Pair`2 (1 base, 0 diff methods)
-26 (-3.641% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Inequality(ReadOnlySpan`1,ReadOnlySpan`1):bool (18 methods)
-26 (-3.641% of base) : System.Private.CoreLib.dasm - Span`1:op_Inequality(Span`1,Span`1):bool (18 methods)
-22 (-31.884% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:.ctor(__Canon,__Canon):this
-21 (-19.444% of base) : System.Linq.Parallel.dasm - OrderedGroupJoinHashLookup:CreateValuePair(GroupKeyData):Pair`2:this
-19 (-11.515% of base) : Microsoft.CodeAnalysis.CSharp.dasm - SyntaxList`1:Any(ushort):bool:this
-19 (-13.475% of base) : System.Linq.dasm - SparseArrayBuilder`1:Reserve(int):this
Top method regressions (percentages):
126 (787.500% of base) : System.DirectoryServices.AccountManagement.dasm - TrackedCollection`1:System.Collections.IEnumerable.GetEnumerator():IEnumerator:this
92 (575.000% of base) : System.Collections.Immutable.dasm - ImmutableList`1:System.Collections.Immutable.IImmutableList<T>.Remove(KeyValuePair`2,IEqualityComparer`1):IImmutableList`1:this
63 (393.750% of base) : System.Collections.dasm - SortedList`2:get_Keys():IList`1:this
63 (393.750% of base) : System.Collections.dasm - SortedList`2:System.Collections.Generic.IDictionary<TKey,TValue>.get_Keys():ICollection`1:this
63 (393.750% of base) : System.Collections.dasm - SortedList`2:System.Collections.IDictionary.get_Keys():ICollection:this
63 (393.750% of base) : System.Collections.dasm - SortedList`2:System.Collections.Generic.IReadOnlyDictionary<TKey,TValue>.get_Keys():IEnumerable`1:this
63 (393.750% of base) : System.Collections.dasm - SortedList`2:get_Values():IList`1:this
63 (393.750% of base) : System.Collections.dasm - SortedList`2:System.Collections.Generic.IDictionary<TKey,TValue>.get_Values():ICollection`1:this
63 (393.750% of base) : System.Collections.dasm - SortedList`2:System.Collections.IDictionary.get_Values():ICollection:this
63 (393.750% of base) : System.Collections.dasm - SortedList`2:System.Collections.Generic.IReadOnlyDictionary<TKey,TValue>.get_Values():IEnumerable`1:this
62 (387.500% of base) : System.Collections.dasm - SortedDictionary`2:System.Collections.Generic.IDictionary<TKey,TValue>.get_Keys():ICollection`1:this
62 (387.500% of base) : System.Collections.dasm - SortedDictionary`2:System.Collections.Generic.IReadOnlyDictionary<TKey,TValue>.get_Keys():IEnumerable`1:this
62 (387.500% of base) : System.Collections.dasm - SortedDictionary`2:System.Collections.Generic.IDictionary<TKey,TValue>.get_Values():ICollection`1:this
62 (387.500% of base) : System.Collections.dasm - SortedDictionary`2:System.Collections.Generic.IReadOnlyDictionary<TKey,TValue>.get_Values():IEnumerable`1:this
62 (387.500% of base) : System.Collections.dasm - SortedDictionary`2:System.Collections.IDictionary.get_Keys():ICollection:this
62 (387.500% of base) : System.Collections.dasm - SortedDictionary`2:System.Collections.IDictionary.get_Values():ICollection:this
61 (381.250% of base) : Newtonsoft.Json.dasm - EnumerableDictionaryWrapper`2:System.Collections.IEnumerable.GetEnumerator():IEnumerator:this
61 (381.250% of base) : System.Net.Http.dasm - HttpHeaderValueCollection`1:System.Collections.IEnumerable.GetEnumerator():IEnumerator:this
60 (375.000% of base) : System.Collections.Immutable.dasm - ImmutableList`1:System.Collections.Immutable.IImmutableList<T>.Remove(__Canon,IEqualityComparer`1):IImmutableList`1:this
60 (375.000% of base) : System.Collections.Immutable.dasm - ImmutableStack`1:System.Collections.Immutable.IImmutableStack<T>.Push(__Canon):IImmutableStack`1:this
Top method improvements (percentages):
-82 (-100.000% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:CreatePair(__Canon,Pair`2):Pair`2 (1 base, 0 diff methods)
-29 (-100.000% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:CreatePair(__Canon,int):Pair`2 (1 base, 0 diff methods)
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector128`1:get_DisplayString():String:this
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector256`1:get_DisplayString():String:this
-59 (-62.105% of base) : System.Private.CoreLib.dasm - Vector64`1:get_DisplayString():String:this
-32 (-46.377% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:.ctor(__Canon,int):this
-22 (-31.884% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:.ctor(__Canon,__Canon):this
-56 (-29.474% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:Add(__Canon,int):bool:this
-145 (-27.672% of base) : System.Collections.Immutable.dasm - Node:GetEnumerator():Enumerator:this (4 base, 3 diff methods)
-52 (-27.083% of base) : System.Linq.Parallel.dasm - HashLookupValueList`2:Add(__Canon,__Canon):bool:this
-169 (-26.365% of base) : System.Collections.Immutable.dasm - ImmutableArray`1:InsertRange(int,ImmutableArray`1):ImmutableArray`1:this (2 methods)
-21 (-19.444% of base) : System.Linq.Parallel.dasm - OrderedGroupJoinHashLookup:CreateValuePair(GroupKeyData):Pair`2:this
-94 (-14.506% of base) : System.Private.CoreLib.dasm - ReadOnlySpan`1:op_Implicit(ref):ReadOnlySpan`1 (18 methods)
-19 (-13.475% of base) : System.Linq.dasm - SparseArrayBuilder`1:Reserve(int):this
-18 (-12.950% of base) : System.Collections.Immutable.dasm - ImmutableDictionary`2:TryGetValue(__Canon,byref):bool:this
-18 (-12.950% of base) : System.Collections.Immutable.dasm - ImmutableDictionary`2:TryGetKey(__Canon,byref):bool:this
-9 (-12.857% of base) : Microsoft.CodeAnalysis.VisualBasic.dasm - _Closure$__17`1:.cctor()
-13 (-12.745% of base) : System.Collections.Immutable.dasm - ImmutableDictionary`2:Contains(KeyValuePair`2):bool:this
-19 (-11.515% of base) : Microsoft.CodeAnalysis.CSharp.dasm - SyntaxList`1:Any(ushort):bool:this
-13 (-10.484% of base) : System.Collections.Immutable.dasm - ImmutableDictionary`2:ContainsKey(__Canon):bool:this
406 total methods with Code Size differences (43 improved, 363 regressed), 186975 unchanged.

@jkotas

Copy link
Copy Markdown
MemberAuthor
  • ReadOnlySequence size regressions is caused by AggresiveInlining on ~70 lines method
  • ImmutableCollections size regression is a lots of small methods that are getting inlined into each other now

@jkotas

Copy link
Copy Markdown
MemberAuthor

JIT stress failure is known issue #36681
Installer Build and Test coreclr failure is known issue #35068
oreCLR Pri0 Runtime Tests Run Windows_NT x86 checked is known issue #38284

@jkotas
jkotas merged commit 31699de into dotnet:masterJun 27, 2020
@jkotas
jkotas deleted the generics-inlining branch June 27, 2020 13:36
@nietras

Copy link
Copy Markdown
Contributor

Great to see this merged. 👍

when the caller and callee are the same exact type and instantiation.

This is quite clear but to be sure, I assume this means this won't work for base class methods for example? E. g. called in a derived class?

@jkotas

Copy link
Copy Markdown
MemberAuthor

I assume this means this won't work for base class methods for example? E. g. called in a derived class?

Correct.

@ghostghost locked as resolved and limited conversation to collaborators Dec 8, 2020
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

11 participants

@jkotas@nietras@mangod9@BruceForstall@davidwrighton@EgorBo@stephentoub@jakobbotsch@AndyAyersMS@MichalStrehovsky@Dotnet-GitSync-Bot