Shrink Dictionary asm by moving non-generic enumerator out

[benaadams]

I think my version of Jitdiff is reporting summary regression/improvement backwards in error?

Total bytes of diff: 624 (0.02% of base)
    diff is a regression.

Total byte diff includes -2303 bytes from reconciling methods
        Base had    4 unique methods,     2840 unique bytes
        Diff had    3 unique methods,      537 unique bytes

Top file regressions (bytes):
         624 : System.Private.CoreLib.dasm (0.02% of base)

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

Top method regressions (bytes):
         475 (55.88% of base) : System.Private.CoreLib.dasm - Enumerator:.ctor(Dictionary`2):this (22 base, 33 diff methods)
         250 (     ∞ of base) : System.Private.CoreLib.dasm - KeyValuePair`2:System.Collections.Generic.IKeyValue.get_Value():Object:this (0 base, 13 diff methods)
         236 (     ∞ of base) : System.Private.CoreLib.dasm - KeyValuePair`2:System.Collections.Generic.IKeyValue.get_Key():Object:this (0 base, 13 diff methods)
         111 (81.02% of base) : System.Private.CoreLib.dasm - DictionaryEnumerator:get_Entry():DictionaryEntry:this (1 base, 2 diff methods)
          82 (264.52% of base) : System.Private.CoreLib.dasm - DictionaryEnumerator:MoveNext():bool:this (1 base, 2 diff methods)
          51 (     ∞ of base) : System.Private.CoreLib.dasm - DictionaryEnumerator:.ctor(IEnumerable):this (0 base, 1 diff methods)
          41 (67.21% of base) : System.Private.CoreLib.dasm - DictionaryEnumerator:get_Key():Object:this (1 base, 2 diff methods)
          41 (67.21% of base) : System.Private.CoreLib.dasm - DictionaryEnumerator:get_Value():Object:this (1 base, 2 diff methods)
          29 (100.00% of base) : System.Private.CoreLib.dasm - DictionaryEnumerator:Reset():this (1 base, 2 diff methods)
           5 ( 7.94% of base) : System.Private.CoreLib.dasm - DictionaryEnumerator:get_Current():Object:this (1 base, 2 diff methods)

Top method improvements (bytes):
       -1495 (-29.41% of base) : System.Private.CoreLib.dasm - Enumerator:System.Collections.IEnumerator.get_Current():Object:this (65 methods)
       -1138 (-100.00% of base) : System.Private.CoreLib.dasm - Enumerator:System.Collections.IDictionaryEnumerator.get_Entry():DictionaryEntry:this (11 base, 0 diff methods)
        -752 (-51.26% of base) : System.Private.CoreLib.dasm - Dictionary`2:System.Collections.IDictionary.GetEnumerator():IDictionaryEnumerator:this (11 methods)
        -583 (-100.00% of base) : System.Private.CoreLib.dasm - Enumerator:System.Collections.IDictionaryEnumerator.get_Value():Object:this (11 base, 0 diff methods)
        -563 (-100.00% of base) : System.Private.CoreLib.dasm - Enumerator:.ctor(Dictionary`2,int):this (11 base, 0 diff methods)
        -556 (-100.00% of base) : System.Private.CoreLib.dasm - Enumerator:System.Collections.IDictionaryEnumerator.get_Key():Object:this (11 base, 0 diff methods)
         -51 (-6.22% of base) : System.Private.CoreLib.dasm - Dictionary`2:GetEnumerator():Enumerator:this (8 methods)
         -49 (-3.34% of base) : System.Private.CoreLib.dasm - Dictionary`2:System.Collections.Generic.IEnumerable<System.Collections.Generic.KeyValuePair<TKey,TValue>>.GetEnumerator():IEnumerator`1:this (11 methods)
         -49 (-3.34% of base) : System.Private.CoreLib.dasm - Dictionary`2:System.Collections.IEnumerable.GetEnumerator():IEnumerator:this (11 methods)
         -36 (-4.04% of base) : System.Private.CoreLib.dasm - ManyElementAsyncLocalValueMap:Set(IAsyncLocal,Object,bool):IAsyncLocalValueMap:this
         -15 (-4.31% of base) : System.Private.CoreLib.dasm - ManifestBuilder:GetChannelData():ref:this
          -6 (-0.12% of base) : System.Private.CoreLib.dasm - ManifestBuilder:CreateManifestString():String:this
          -5 (-1.47% of base) : System.Private.CoreLib.dasm - ResourceManager:ReleaseAllResources():this
          -3 (-0.73% of base) : System.Private.CoreLib.dasm - DiagnosticCounter:GetMetadataString():String:this
          -2 (-0.65% of base) : System.Private.CoreLib.dasm - AssemblyLoadContext:OnProcessExit()

25 total methods with Code Size differences (15 improved, 10 regressed), 20354 unchanged.

/cc @jkotas not sure if this is an interesting change?

[AndyAyersMS]

This was with --pmi I take it?

PMI uses a simple-minded strategy to attempt instantiation of generic classes and methods. If you shift around generic code the strategy may succeed or fail more often, so total sizes can be misleading.

Crossgen diffs should offer a more stable picture but will be fairly restrictive about what gets instantiated, so may miss things that PMI can spot.

[benaadams]

This was with --pmi I take it?

No, just regular crossgen diff of System.Private.CoreLib

Will try pmi

[benaadams]

@AndyAyersMS what I was confused about is the header says

Total bytes of diff: 624 (0.02% of base)
    diff is a regression.

and

Top file regressions (bytes):
         624 : System.Private.CoreLib.dasm (0.02% of base)

However the differences all look like improvements? e.g.

Total byte diff includes -2303 bytes from reconciling methods
        Base had    4 unique methods,     2840 unique bytes
        Diff had    3 unique methods,      537 unique bytes

[benaadams]

Looks like it would be a break; though hopefully not one depend on?

Type 'System.Collections.Generic.Dictionary<TKey, TValue>.Enumerator' does not implement 
interface 'System.Collections.IDictionaryEnumerator' in the implementation but it does in the 
contract.

[jkotas]

Looks like it would be a break; though hopefully not one depend on?

There are going to be apps that depend on it for sure. We have tried to messing with the iterators like this in .NET Core 1.0 days and it did not work well. I do not think we would want to make this breaking change.

Clusters of non-generic code in generic types show up quite a bit. The AOT compiler (where this duplication hurts the most) should be smart enough to see this duplication and generate one piece of shared code for it. .NET Native for UWP has this optimization.

[benaadams]

Fair enough

[benaadams]

XML uses it :) #32296

[AndyAyersMS]

@benaadams -- looks like the logic in Reconcile has it backwards:

https://github.com/dotnet/jitutils/blob/ea23722bd743a4700f7f38fac872ec896f2711a5/src/jit-analyze/jit-analyze.cs#L443-L445

It should add the diff and subtract the base.