Special case List<ClaimsIdentity> in SelectPrimaryIdentity

[vcsjones]

This special-cases SelectPrimaryIdentity to avoid allocating an enumerator instance if the underlying list of claims is a List<ClaimsIdentity>. We only do this if the type is exactly the List, no derived types and no IList<T>.

Fixes #107861

[stephentoub]

Would @AndyAyersMS's work with PGO and stack allocation of enumerators handle this automatically rather than needing to manually handle the special-casing?

[AndyAyersMS]

Would @AndyAyersMS's work with PGO and stack allocation of enumerators handle this automatically rather than needing to manually handle the special-casing?

Seems like it should. If you to do type analysis on a collection to try and leverage a struct enumerator that some specific collection type offers you are more or less duplicating what these (not yet merged) changes do.

There is PR up for the changes (#111473). They are in fairly decent shape, and they optimize the allocation and much of the overhead for abstract enumeration (provided the actual enumerator methods aren't too complicated -- works nicely for Lists, Arrays, etc).

Writeup on the changes in case you want to learn more.

[vcsjones]

Seems like it should

I lack the expertise to see if that PR would eliminate the enumerator allocation, but I would be in favor of closing this pull request and the associated issue if the JIT is able to handle it.

[stephentoub]

Seems like it should

I lack the expertise to see if that PR would eliminate the enumerator allocation, but I would be in favor of closing this pull request and the associated issue if the JIT is able to handle it.

Let's hold off on this until Andy's change merges and then validate this isn't actually needed.

Thanks.

[bartonjs]

It certainly sounds like Andy's change should address this, since List<T> is a specific target in the writeup (https://github.com/dotnet/runtime/blob/41f8397b864dd7280c75d0068b3cb5bfeb55a8a4/docs/design/coreclr/jit/DeabstractionAndConditionalEscapeAnalysis.md#listt).

I love when we are able to make changes to the BCL/JIT/Runtime that means that the simplest code matches (or overtakes) what complex code is striving to do.

[AndyAyersMS]

There's no particular restriction on which collection types can participate -- the optimization currently keys off of the GetEnumerator call and uses PGO/GDV to specialize for the most likely types seen at runtime. Whether the optimization succeeds on eliminating the enumerator allocation and how well the enumerator gets optimized subsequently depends on the complexity of the enumerator logic, though I will probably be chipping away at removing some of those limitations over time (eg, if the MoveNext has a try we won't optimize).

Behind this there is also a limitation from PGO. It will currently only specialize for the most frequently seen type so if a given use site sees a variety of collections and no one type appears more than about 30% of the time, we won't optimize at all. We have ambitions of broadening this too (multi-guess PGO), but that and conditional escape interact with each other and I haven't yet tried to make the combination work well.