-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Reduce some allocations in Razor #35208
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
Conversation
* Avoids boxing in HashCodeCombiner * Avoids dictionary enumerator boxing in BoundAttributeDescriptorComparer.GetHashCode Based on https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1366879?src=WorkItemMention&src-action=artifact_link
src/Razor/Microsoft.AspNetCore.Razor.Language/src/BoundAttributeDescriptorComparer.cs
Outdated
Show resolved
Hide resolved
src/Razor/Microsoft.AspNetCore.Razor.Language/src/BoundAttributeDescriptorComparer.cs
Outdated
Show resolved
Hide resolved
…teDescriptorComparer.cs
| } | ||
|
|
||
| [MethodImpl(MethodImplOptions.AggressiveInlining)] | ||
| public void Add(IEnumerable e) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't care about this? Presumably it's just calling the generic version now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't see any uses for this and it seemed messier to add a generic version. Regardless, most of our codebase is using System.HashCode and Razor is an outlier due to it targeting ns2.0.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you see any uses of this before the introduction of the generic form? Derived types would prefer to call the generic form than cast to IEnumerable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just checked and I don't see any references prior to this PR.
Based on https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1366879?src=WorkItemMention&src-action=artifact_link