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.

Sign up for GitHub

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

OR: performance tune Match function [PLAT-23274] #2

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

OR: performance tune Match function [PLAT-23274] #2

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
76652db
OR: performance tune Match function [PLAT-23274]
jackmott Feb 14, 2020
5187750
OR: revert broken candidateSegment changes [PLAT-23274]
jackmott Feb 14, 2020
b8b5286
OR: remove unused using [PLAT-23274]
jackmott Feb 14, 2020
620e311
OR: eliminate anonymous class to reduce allocations [PLAT-23274]
jackmott Feb 14, 2020
5b98222
OR: pull the removetrailingslash outside the loop [PLAT-23274]
jackmott Feb 14, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 39 additions & 36 deletions src/OpenRasta/UriTemplate.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,15 +119,21 @@ static Dictionary<string, UrlSegment> ParsePathSegments(List<UrlSegment> segment

static Dictionary<string, QuerySegment> ParseQueryStringSegments(IEnumerable<QuerySegment> queryString)
{
return queryString
.GroupBy(qs => qs.Key, StringComparer.OrdinalIgnoreCase)
.ToDictionary(qs => qs.Key, qs => qs.First(), StringComparer.OrdinalIgnoreCase);
var result = new Dictionary<string, QuerySegment>(StringComparer.OrdinalIgnoreCase);
foreach(var qs in queryString)
{
if (!result.ContainsKey(qs.Key))
{
result.Add(qs.Key, qs);
}
}
return result;
}

public static IEnumerable<QuerySegment> ParseQueryStringSegments(string query)
public static List<QuerySegment> ParseQueryStringSegments(string query)
{
var kvPairs = query.Split('&');

var kvPairs = query.Split('&');
List<QuerySegment> result = new List<QuerySegment>(kvPairs.Length);
foreach (var kvPair in kvPairs)
{
var unescapedString = Uri.UnescapeDataString(kvPair.Replace('+', ' '));
Expand All @@ -150,15 +156,15 @@ public static IEnumerable<QuerySegment> ParseQueryStringSegments(string query)
RawValue = val,
Type = valAsVariable == null ? SegmentType.Literal : SegmentType.Variable
};
yield return (segment);
result.Add(segment);
}
else
{
yield return new QuerySegment {Key = unescapedString, Value = null, Type = SegmentType.Literal};
result.Add(new QuerySegment { Key = unescapedString, Value = null, Type = SegmentType.Literal });
}
}
return result;
}

static List<UrlSegment> ParsePathSegments(Uri templateUri)
{
var passedSegments = new List<UrlSegment>();
Expand Down Expand Up @@ -228,8 +234,8 @@ public Uri BindByName(Uri baseAddress, NameValueCollection parameters)
else if (segment.Type == SegmentType.Variable)
{
var value = parameters[segment.Text.ToUpperInvariant()];


path.Append(value.Replace("/", "%2F")
.Replace("?", "%3F")
.Replace("#", "%23"));
Expand All @@ -247,7 +253,7 @@ public Uri BindByName(Uri baseAddress, NameValueCollection parameters)
var qsValue = parameters[querySegment.Value.Value]
.Replace("&", "%25")
.Replace("#", "%23");

path.Append(querySegment.Value.Key).Append("=")
.Append(qsValue).Append("&");
}
Expand Down Expand Up @@ -333,18 +339,26 @@ public bool IsEquivalentTo(UriTemplate other)

return true;
}

string RemoveTrailingSlash(string str)
{
return str.LastIndexOf('/') == str.Length - 1 ? str.Substring(0, str.Length - 1) : str;
}
public UriTemplateMatch Match(Uri baseAddress, Uri uri)
{
if (baseAddress == null || uri == null)
var baseLeft = baseAddress.GetLeftPart(UriPartial.Authority);
var baseSegments = baseAddress.Segments.Select(RemoveTrailingSlash).ToArray();
return Match(baseAddress, baseLeft, baseSegments, uri);
}
public UriTemplateMatch Match(Uri baseAddress, string baseLeft, string[] baseSegments, Uri uri)
{
if (uri == null)
return null;
if (baseAddress.GetLeftPart(UriPartial.Authority) != uri.GetLeftPart(UriPartial.Authority))
if (baseLeft != uri.GetLeftPart(UriPartial.Authority))
return null;

var baseUriSegments = baseAddress.Segments.Select(RemoveTrailingSlash);
var candidateSegments = new List<string>(uri.Segments.Select(RemoveTrailingSlash));

foreach (var baseUriSegment in baseUriSegments)
var segments = uri.Segments;
var candidateSegments = baseSegments.ToList();
foreach (var baseUriSegment in baseSegments)
if (baseUriSegment == candidateSegments[0])
candidateSegments.RemoveAt(0);

Expand All @@ -358,31 +372,25 @@ public UriTemplateMatch Match(Uri baseAddress, Uri uri)
for (var i = 0; i < _segments.Count; i++)
{
var segment = candidateSegments[i];
var unescapedText = Uri.UnescapeDataString(segment);

var candidateSegment = new
{
Text = segment,
UnescapedText = Uri.UnescapeDataString(segment),
ProposedSegment = _segments[i]
};

candidateSegments[i] = candidateSegment.Text;
candidateSegments[i] = segment;

switch (candidateSegment.ProposedSegment.Type)
switch (_segments[i].Type)
{
case SegmentType.Literal when
string.CompareOrdinal(candidateSegment.ProposedSegment.Text, candidateSegment.UnescapedText) != 0:
string.CompareOrdinal(_segments[i].Text, unescapedText) != 0:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not your code but this could be simplified to !string.Equals(_segments[i].Text, unescapedText, StringComparison.OrdinalIgnoreCase).

return null;
case SegmentType.Wildcard:
throw new NotImplementedException("Not finished wildcards implementation yet");
case SegmentType.Variable:
boundVariables.Add(candidateSegment.ProposedSegment.Text, Uri.UnescapeDataString(candidateSegment.Text));
boundVariables.Add(_segments[i].Text, Uri.UnescapeDataString(segment));
break;
}
}

var queryStringVariables = new NameValueCollection();
var uriQuery = ParseQueryStringSegments(uri.Query).ToList();
var uriQuery = ParseQueryStringSegments(uri.Query);
var requestUriQuerySegments = ParseQueryStringSegments(uriQuery);

var queryParams = new Collection<string>();
Expand Down Expand Up @@ -428,11 +436,6 @@ static bool QuerySegmentValueIsDifferent(
return requestUriQuerySegments[templateQuerySegment.Key].Value != templateQuerySegment.Value;
}

string RemoveTrailingSlash(string str)
{
return str.LastIndexOf('/') == str.Length - 1 ? str.Substring(0, str.Length - 1) : str;
}

public override int GetHashCode()
{
var hash = 0;
Expand Down
74 changes: 45 additions & 29 deletions src/OpenRasta/UriTemplateTable.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,38 +52,49 @@ public void MakeReadOnly(bool allowDuplicateEquivalentUriTemplates)
_keyValuePairsReadOnly = _keyValuePairs.AsReadOnly();
}

string RemoveTrailingSlash(string str)
{
return str.LastIndexOf('/') == str.Length - 1 ? str.Substring(0, str.Length - 1) : str;
}

public Collection<UriTemplateMatch> Match(Uri uri)
{
var lastMaxLiteralSegmentCount = 0;
var matches = new Collection<UriTemplateMatch>();
foreach (var template in KeyValuePairs)
if (BaseAddress != null)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

{
// TODO: discard uri templates with fragment identifiers until tests are implemented
if (template.Key.Fragment.Any()) continue;
var potentialMatch = template.Key.Match(BaseAddress, uri);
var baseLeft = BaseAddress.GetLeftPart(UriPartial.Authority);
var baseSegments = BaseAddress.Segments.Select(RemoveTrailingSlash).ToArray();
foreach (var template in KeyValuePairs)
{
// TODO: discard uri templates with fragment identifiers until tests are implemented
if (template.Key.Fragment.Any()) continue;

if (potentialMatch == null) continue;
UriTemplateMatch potentialMatch = template.Key.Match(BaseAddress, baseLeft, baseSegments, uri);

// this calculates and keep only what matches the maximum possible amount of literal segments
var currentMaxLiteralSegmentCount = potentialMatch.RelativePathSegments.Count
- potentialMatch.WildcardPathSegments.Count;
for (var i = 0; i < potentialMatch.PathSegmentVariables.Count; i++)
if (potentialMatch.QueryParameters == null ||
potentialMatch.QueryStringVariables[potentialMatch.PathSegmentVariables.GetKey(i)] == null)
currentMaxLiteralSegmentCount -= 1;
if (potentialMatch == null) continue;

potentialMatch.Data = template.Value;
// this calculates and keep only what matches the maximum possible amount of literal segments
var currentMaxLiteralSegmentCount = potentialMatch.RelativePathSegments.Count
- potentialMatch.WildcardPathSegments.Count;
for (var i = 0; i < potentialMatch.PathSegmentVariables.Count; i++)
if (potentialMatch.QueryParameters == null ||
potentialMatch.QueryStringVariables[potentialMatch.PathSegmentVariables.GetKey(i)] == null)
currentMaxLiteralSegmentCount -= 1;

if (currentMaxLiteralSegmentCount > lastMaxLiteralSegmentCount)
{
lastMaxLiteralSegmentCount = currentMaxLiteralSegmentCount;
}
else if (currentMaxLiteralSegmentCount < lastMaxLiteralSegmentCount)
{
continue;
}
potentialMatch.Data = template.Value;

matches.Add(potentialMatch);
if (currentMaxLiteralSegmentCount > lastMaxLiteralSegmentCount)
{
lastMaxLiteralSegmentCount = currentMaxLiteralSegmentCount;
}
else if (currentMaxLiteralSegmentCount < lastMaxLiteralSegmentCount)
{
continue;
}

matches.Add(potentialMatch);
}
}

return SortByMatchQuality(matches).ToCollection();
Expand All @@ -103,15 +114,20 @@ IEnumerable<UriTemplateMatch> SortByMatchQuality(Collection<UriTemplateMatch> ma
public UriTemplateMatch MatchSingle(Uri uri)
{
UriTemplateMatch singleMatch = null;
foreach (var segmentKey in KeyValuePairs)
if (BaseAddress != null)
{
UriTemplateMatch potentialMatch = segmentKey.Key.Match(BaseAddress, uri);
if (potentialMatch != null && singleMatch != null)
throw new UriTemplateMatchException("Several matching templates were found.");
if (potentialMatch != null)
var baseLeft = BaseAddress.GetLeftPart(UriPartial.Authority);
var baseSegments = BaseAddress.Segments.Select(RemoveTrailingSlash).ToArray();
foreach (var segmentKey in KeyValuePairs)
{
singleMatch = potentialMatch;
singleMatch.Data = segmentKey.Value;
UriTemplateMatch potentialMatch = segmentKey.Key.Match(BaseAddress,baseLeft,baseSegments,uri);
if (potentialMatch != null && singleMatch != null)
throw new UriTemplateMatchException("Several matching templates were found.");
if (potentialMatch != null)
{
singleMatch = potentialMatch;
singleMatch.Data = segmentKey.Value;
}
}
}

Expand Down