OR: performance tune Match function [PLAT-23274] #2
Conversation
|
Some Perf stats on this change: Gen0 collections collections completely eliminated |
src/OpenRasta/UriTemplate.cs
Outdated
|
|
||
| var cullCount = 0; | ||
| foreach (var baseUriSegment in baseSegments) | ||
| if (RemoveTrailingSlash(baseUriSegment) == candidateSegments[cullCount]) |
There was a problem hiding this comment.
Could the RemoveTrailingSlash call be moved into the UriTemplateTable.Match() method here so that we don't need to trim each segment for each possible UriTemplate instance?
There was a problem hiding this comment.
Additionally, could the RemoveTrailingSlash call be simplified to just baseUriSegment.TrimEnd('/')?
| public Collection<UriTemplateMatch> Match(Uri uri) | ||
| { | ||
| var lastMaxLiteralSegmentCount = 0; | ||
| var matches = new Collection<UriTemplateMatch>(); | ||
| foreach (var template in KeyValuePairs) | ||
| if (BaseAddress != null) |
There was a problem hiding this comment.
If I'm reading this right, if BaseAddress is null then no URL will ever find a match. Should BaseAddress be validated to not be null when initializing the UriTemplateTable instance?
| { | ||
| case SegmentType.Literal when | ||
| string.CompareOrdinal(candidateSegment.ProposedSegment.Text, candidateSegment.UnescapedText) != 0: | ||
| string.CompareOrdinal(_segments[i].Text, unescapedText) != 0: |
There was a problem hiding this comment.
Not your code but this could be simplified to !string.Equals(_segments[i].Text, unescapedText, StringComparison.OrdinalIgnoreCase).
Hi there!
Thank you so much for contributing this pull request. We love when the community
steps up and share, and we're enormously thankful.
You don't have to do all of those following things, they are what we try and get
done for code coming in:
CHANGELOG.md
project, or scenarios in theTestRig.masterIf those are needed, and you haven't and can't, we thank you enormously for your
contribution, and we'll add those before merging the pull request.
Thanks!
The OpenRasta community