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

Reduce Allocations in ValidateSignature #2586

Merged
merged 1 commit into from
May 9, 2024

Conversation

eerhardt
Copy link
Contributor

@eerhardt eerhardt commented May 6, 2024

When we need to create an enumerable with a single element in it, we are allocating a new List<SecurityKey> { key }. This allocates both the list, and then allocates a 4 element array backing the list (because this syntax is C# sugar for just calling Add on the collection).

Instead, use a collection expression which optimizes the single element case.

There was one method in Saml that had this pattern in it, but was unused. I opted to remove it. Please let me know if it should be preserved for some reason.

Using an internal benchmark with 1,000 iterations, these allocations will be cut in half:

image

When we need to create an enumerable with a single element in it, we are allocating a `new List<SecurityKey> { key }`. This allocates both the list, and then allocates a 4 element array backing the list (because this syntax is C# sugar for just calling Add on the collection).

Instead, use a collection expression which optimizes the single element case.

There was one method in Saml that had this pattern in it, but was unused. I opted to remove it. Please let me know if it should be preserved for some reason.
@eerhardt eerhardt requested a review from a team as a code owner May 6, 2024 21:05
@eerhardt
Copy link
Contributor Author

eerhardt commented May 6, 2024

See dotnet/roslyn#71147 for what Roslyn does in these cases. It generates a <>z__ReadOnlySingleElementList class that is used for the single element case.

@eerhardt eerhardt merged commit 4228935 into dev May 9, 2024
5 checks passed
@eerhardt eerhardt deleted the eerhardt/ReduceListAllocations branch May 9, 2024 18:37
@kellyyangsong kellyyangsong added this to the 7.5.2 milestone Jun 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants