Avoid dependency on LINQ orderby iterators when building exception message #53123
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Tagging subscribers to this area: @eerhardt, @maryamariyan Issue DetailsSaves about 2k compressed for Blazor default template
|
nil4
reviewed
May 22, 2021
src/libraries/Microsoft.Extensions.DependencyInjection/src/ServiceLookup/CallSiteChain.cs
Outdated
Show resolved
Hide resolved
eerhardt
approved these changes
May 22, 2021
Member
|
This saves 2K because it's the only remaining LINQ in the default code path? |
Contributor
Author
Yep, the only usage of OrderBy + dependencies |
src/libraries/Microsoft.Extensions.DependencyInjection/src/ServiceLookup/CallSiteChain.cs
Outdated
Show resolved
Hide resolved
marek-safar
force-pushed
the
di2
branch
2 times, most recently
from
8993385 to
a9e7356
Compare
…sssage Saves about 2k compressed for Blazor default template
stephentoub
reviewed
May 25, 2021
Comment on lines
+52
to
+53
| var ordered = new List<KeyValuePair<Type, ChainItemInfo>>(_callSiteChain); | ||
| ordered.Sort((a, b) => a.Value.Order.CompareTo(b.Value.Order)); |
Member
There was a problem hiding this comment.
FWIW, this could avoid the List and just use an array, e.g.
var ordered = new KeyValuePair<Type, ChainItemInfo>[_callSiteChain.Length];
((ICollection<KeyValuePair<TKey, TValue>>)_callSiteChain).CopyTo(ordered, 0);
Array.Sort(ordered, (a, b) => a.Value.Order.CompareTo(b.Value.Order));That'll also let the compiler change the subsequent foreach into a for loop.
Regardless, nice to see the unnecessary OrderBy removal.
stephentoub
reviewed
May 25, 2021
| var ordered = new List<KeyValuePair<Type, ChainItemInfo>>(_callSiteChain); | ||
| ordered.Sort((a, b) => a.Value.Order.CompareTo(b.Value.Order)); | ||
|
|
||
| foreach (KeyValuePair<Type, ChainItemInfo> pair in ordered) |
Member
There was a problem hiding this comment.
The below if/else could also be simplified:
builder.Append(TypeNameHelper.GetTypeDisplayName(serviceType));
if (implementationType != null && serviceType != implementationType)
{
builder.Append('(')
.Append(TypeNameHelper.GetTypeDisplayName(implementationType))
.Append(')');
}
ghost
locked as resolved and limited conversation to collaborators
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
6 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Saves about 2k compressed for Blazor default template