Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Special case List<ClaimsIdentity> in SelectPrimaryIdentity #111799

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

vcsjones
Copy link
Member

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
Copy link
Member

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

@AndyAyersMS
Copy link
Member

AndyAyersMS commented Jan 24, 2025

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
Copy link
Member Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ClaimsPrincipal.SelectPrimaryIdentity allocates unnecessary enumerator
4 participants