Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
24 changes: 13 additions & 11 deletions src/JsonApiDotNetCore/Builders/LinkBuilder.cs
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
using System;
using System.Text;
using JsonApiDotNetCore.Extensions;
using JsonApiDotNetCore.Internal;
using JsonApiDotNetCore.Services;
using Microsoft.AspNetCore.Http;

Expand All @@ -20,20 +24,18 @@ public string GetBasePath(HttpContext context, string entityName)
: $"{r.Scheme}://{r.Host}{GetNamespaceFromPath(r.Path, entityName)}";
}

private string GetNamespaceFromPath(string path, string entityName)
private static string GetNamespaceFromPath(string path, string entityName)
{
var nSpace = string.Empty;
var segments = path.Split('/');

for (var i = 1; i < segments.Length; i++)
var sb = new StringBuilder();
var entityNameSpan = entityName.AsSpan();
var subSpans = path.SpanSplit('/');
for (var i = 1; i < subSpans.Count; i++)
{
if (segments[i].ToLower() == entityName)
break;

nSpace += $"/{segments[i]}";
var span = subSpans[i];
if (entityNameSpan.SequenceEqual(span)) break;
sb.Append($"/{span.ToString()}");
}

return nSpace;
return sb.ToString();
Copy link
Contributor

@jaredcnance jaredcnance Apr 28, 2018

Choose a reason for hiding this comment

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

Interestingly, as written this performs almost identically to the Split version. I've found one optimization that cuts this in half by removing the StringBuilder. I also think we can eliminate most of the allocations within SpanSplitter since I don't think it needs to be stateful. We really just need to read over a string and find the place where /{entityName} exists (followed by / or EOL), then we Slice everything before that. So, we shouldn't need to allocate anything until the final .Slice(/*...*/).ToString(). I'll push my changes sometime this weekend.

Copy link
Contributor

Choose a reason for hiding this comment

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

These are the results I get when running benchmarks (will submit code soon):

BenchmarkDotNet=v0.10.10, OS=Mac OS X 10.12
Processor=Intel Core i5-5257U CPU 2.70GHz (Broadwell), ProcessorCount=4
.NET Core SDK=2.1.4
  [Host]     : .NET Core 2.0.5 (Framework 4.6.0.0), 64bit RyuJIT
  Job-LPGSMZ : .NET Core 2.0.5 (Framework 4.6.0.0), 64bit RyuJIT

LaunchCount=3  TargetCount=20  WarmupCount=10  
Method Mean Error StdDev Median Gen 0 Allocated
UsingSplit (original) 1,257.5 ns 36.05 ns 76.83 ns 1,229.1 ns 0.9251 1456 B
UsingSpanWithStringBuilder (current PR) 1,770.0 ns 68.65 ns 144.81 ns 1,767.0 ns 0.9460 1488 B
UsingSpanWithoutStringBuilder 871.3 ns 17.22 ns 36.69 ns 858.9 ns 0.4368 688 B
UsingSpanWithoutSpanSplitter 317.2 ns 25.54 ns 56.06 ns 300.5 ns 0.0863 136 B

}

public string GetSelfRelationLink(string parent, string parentId, string child)
Expand Down
19 changes: 19 additions & 0 deletions src/JsonApiDotNetCore/Extensions/StringExtensions.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
using System;
using System.Collections.Generic;
using System.Text;
using JsonApiDotNetCore.Internal;

namespace JsonApiDotNetCore.Extensions
{
Expand Down Expand Up @@ -50,5 +53,21 @@ public static string Dasherize(this string str)
}
return str;
}

public static IEnumerable<int> IndexesOf(this string str, char delimeter)
{
var indexes = new List<int>();
for (var i = str.IndexOf(delimeter); i > -1 ; i = str.IndexOf(delimeter, i+1))
{
indexes.Add(i);
}
return indexes;
}

public static SpanSplitter SpanSplit(this string str, char delimeter)
{
return SpanSplitter.Split(str, delimeter);
}

}
}
16 changes: 9 additions & 7 deletions src/JsonApiDotNetCore/Internal/Query/RelatedAttrFilterQuery.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System;
using System.Linq;
using JsonApiDotNetCore.Extensions;
using JsonApiDotNetCore.Models;
using JsonApiDotNetCore.Services;

Expand All @@ -8,20 +9,21 @@ namespace JsonApiDotNetCore.Internal.Query
public class RelatedAttrFilterQuery : BaseFilterQuery
{
private readonly IJsonApiContext _jsonApiContext;

public RelatedAttrFilterQuery(
IJsonApiContext jsonApiCopntext,
FilterQuery filterQuery)
{
_jsonApiContext = jsonApiCopntext;

var relationshipArray = filterQuery.Attribute.Split('.');

var relationship = GetRelationship(relationshipArray[0]);
var filterQueryAttribute = filterQuery.Attribute;
var filterQuerySubSpans = filterQueryAttribute.SpanSplit('.');
var subSpan1 = filterQuerySubSpans[0].ToString();
var subSpan2 = filterQuerySubSpans[1].ToString();
var relationship = GetRelationship(subSpan1);
if (relationship == null)
throw new JsonApiException(400, $"{relationshipArray[1]} is not a valid relationship on {relationshipArray[0]}.");
throw new JsonApiException(400, $"{subSpan2} is not a valid relationship on {subSpan1}.");

var attribute = GetAttribute(relationship, relationshipArray[1]);
var attribute = GetAttribute(relationship, subSpan2);

if (attribute == null)
throw new JsonApiException(400, $"'{filterQuery.Attribute}' is not a valid attribute.");
Expand Down
69 changes: 69 additions & 0 deletions src/JsonApiDotNetCore/Internal/SpanSplitter.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Linq;
using JsonApiDotNetCore.Extensions;

namespace JsonApiDotNetCore.Internal
{
public readonly ref struct SpanSplitter
{
private readonly ReadOnlySpan<char> _span;
private readonly List<int> _delimeterIndexes;
private readonly List<Tuple<int, int>> _substringIndexes;

public int Count => _substringIndexes.Count();
public ReadOnlySpan<char> this[int index] => GetSpanForSubstring(index + 1);

private SpanSplitter(ref string str, char delimeter)
{
_span = str.AsSpan();
_delimeterIndexes = str.IndexesOf(delimeter).ToList();
_substringIndexes = new List<Tuple<int, int>>();
BuildSubstringIndexes();
}

public static SpanSplitter Split(string str, char delimeter)
{
return new SpanSplitter(ref str, delimeter);
}

[EditorBrowsable(EditorBrowsableState.Never)]
public override bool Equals(object obj) => throw new NotSupportedException();

[EditorBrowsable(EditorBrowsableState.Never)]
public override int GetHashCode() => throw new NotSupportedException();

[EditorBrowsable(EditorBrowsableState.Never)]
public override string ToString() => throw new NotSupportedException();

private ReadOnlySpan<char> GetSpanForSubstring(int substringNumber)
{
if (substringNumber > Count)
{
throw new ArgumentOutOfRangeException($"There are only {Count} substrings given the delimeter and base string provided");
}

var indexes = _substringIndexes[substringNumber - 1];
return _span.Slice(indexes.Item1, indexes.Item2);
}

private void BuildSubstringIndexes()
{
var start = 0;
var end = 0;
foreach (var index in _delimeterIndexes)
{
end = index;
if (start > end) break;
_substringIndexes.Add(new Tuple<int, int>(start, end - start));
start = ++end;
}

if (end <= _span.Length)
{
_substringIndexes.Add(new Tuple<int, int>(start, _span.Length - start));
}
}
}
}
7 changes: 7 additions & 0 deletions src/JsonApiDotNetCore/JsonApiDotNetCore.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
<PackageReference Include="Microsoft.AspNetCore.Mvc" Version="$(AspNetCoreVersion)" />
<PackageReference Include="Microsoft.EntityFrameworkCore" Version="$(EFCoreVersion)" />
<PackageReference Include="Microsoft.Extensions.Logging" Version="$(MicrosoftLoggingVersion)" />
<PackageReference Include="System.Memory" Version="4.5.0-preview2-26406-04" />
<PackageReference Include="System.ValueTuple" Version="$(TuplesVersion)" />
</ItemGroup>

Expand All @@ -31,6 +32,12 @@
<PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Release|AnyCPU'">
<DocumentationFile>bin\Release\netstandard2.0\JsonApiDotNetCore.xml</DocumentationFile>
</PropertyGroup>
<PropertyGroup Condition="'$(Configuration)|$(TargetFramework)|$(Platform)'=='Release|netstandard2.0|AnyCPU'">
<LangVersion>7.2</LangVersion>
</PropertyGroup>
<PropertyGroup Condition="'$(Configuration)|$(TargetFramework)|$(Platform)'=='Debug|netstandard2.0|AnyCPU'">
<LangVersion>7.2</LangVersion>
</PropertyGroup>
<ItemGroup Condition="$(IsWindows)=='true'">
<PackageReference Include="docfx.console" Version="2.33.0" />
</ItemGroup>
Expand Down
8 changes: 6 additions & 2 deletions src/JsonApiDotNetCore/Middleware/RequestMiddleware.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
using System;
using System.Threading.Tasks;
using JsonApiDotNetCore.Extensions;
using JsonApiDotNetCore.Internal;
using Microsoft.AspNetCore.Http;
using Microsoft.Extensions.Primitives;
Expand Down Expand Up @@ -54,8 +56,10 @@ private static bool IsValidAcceptHeader(HttpContext context)

private static bool ContainsMediaTypeParameters(string mediaType)
{
var mediaTypeArr = mediaType.Split(';');
return (mediaTypeArr[0] == Constants.ContentType && mediaTypeArr.Length == 2);
const char delimeter = ';';
var subSpans = mediaType.SpanSplit(delimeter);
if (subSpans.Count == 0) return false;
return subSpans.Count == 2 && subSpans[0].ToString() == Constants.ContentType;
Copy link
Contributor

Choose a reason for hiding this comment

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

So, it appears that the simple overhead of the SpanSplitter type definition and its members has enough cost that we don't actually improve the performance. In general, it appears we can get much better performance by not creating a new abstraction. While I feel like your approach was elegant, it unfortunately did not have the desired result.

In the coming commit, I was able to get this down to 0 allocations.

BenchmarkDotNet=v0.10.10, OS=Mac OS X 10.12
Processor=Intel Core i5-5257U CPU 2.70GHz (Broadwell), ProcessorCount=4
.NET Core SDK=2.1.4
  [Host]     : .NET Core 2.0.5 (Framework 4.6.0.0), 64bit RyuJIT
  DefaultJob : .NET Core 2.0.5 (Framework 4.6.0.0), 64bit RyuJIT

Method Mean Error StdDev Gen 0 Allocated
UsingSplit 157.11 ns 2.9374 ns 2.6039 ns 0.2134 336 B
UsingSpan (Current PR) 364.75 ns 7.1680 ns 6.3542 ns 0.2389 376 B
Proposal (6568c37) 37.91 ns 0.7256 ns 0.6787 ns - 0 B

}

private static void FlushResponse(HttpContext context, int statusCode)
Expand Down
10 changes: 7 additions & 3 deletions src/JsonApiDotNetCore/Services/JsonApiContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
using System.Linq;
using JsonApiDotNetCore.Builders;
using JsonApiDotNetCore.Configuration;
using JsonApiDotNetCore.Extensions;
using JsonApiDotNetCore.Internal;
using JsonApiDotNetCore.Internal.Generics;
using JsonApiDotNetCore.Internal.Query;
Expand Down Expand Up @@ -64,7 +65,7 @@ public IJsonApiContext ApplyContext<T>(object controller)
throw new JsonApiException(500, $"A resource has not been properly defined for type '{typeof(T)}'. Ensure it has been registered on the ContextGraph.");

var context = _httpContextAccessor.HttpContext;
var path = context.Request.Path.Value.Split('/');
var requestPath = context.Request.Path.Value;

if (context.Request.Query.Count > 0)
{
Expand All @@ -75,10 +76,13 @@ public IJsonApiContext ApplyContext<T>(object controller)
var linkBuilder = new LinkBuilder(this);
BasePath = linkBuilder.GetBasePath(context, _controllerContext.RequestEntity.EntityName);
PageManager = GetPageManager();
IsRelationshipPath = path[path.Length - 2] == "relationships";

var pathSpans = requestPath.SpanSplit('/');
IsRelationshipPath = pathSpans[pathSpans.Count - 2].ToString() == "relationships";
Copy link
Contributor

Choose a reason for hiding this comment

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

BenchmarkDotNet=v0.10.10, OS=Mac OS X 10.12
Processor=Intel Core i5-5257U CPU 2.70GHz (Broadwell), ProcessorCount=4
.NET Core SDK=2.1.4
  [Host]     : .NET Core 2.0.5 (Framework 4.6.0.0), 64bit RyuJIT
  DefaultJob : .NET Core 2.0.5 (Framework 4.6.0.0), 64bit RyuJIT

Method Mean Error StdDev Gen 0 Allocated
Original 421.08 ns 19.3905 ns 54.0529 ns 0.4725 744 B
Current PR 697.65 ns 11.9282 ns 11.1576 ns 0.5999 944 B
Proposal (fbe1d1b) 52.23 ns 0.8052 ns 0.7532 ns - 0 B


return this;
}

private PageManager GetPageManager()
{
if (Options.DefaultPageSize == 0 && (QuerySet == null || QuerySet.PageQuery.PageSize == 0))
Expand Down
21 changes: 14 additions & 7 deletions src/JsonApiDotNetCore/Services/QueryParser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
using System.Linq;
using JsonApiDotNetCore.Configuration;
using JsonApiDotNetCore.Controllers;
using JsonApiDotNetCore.Extensions;
using JsonApiDotNetCore.Internal;
using JsonApiDotNetCore.Internal.Query;
using JsonApiDotNetCore.Models;
Expand Down Expand Up @@ -93,16 +94,16 @@ protected virtual List<FilterQuery> ParseFilterQuery(string key, string value)
// expected input = filter[id]=1
// expected input = filter[id]=eq:1
var queries = new List<FilterQuery>();
var openBracketIndex = key.IndexOf(OPEN_BRACKET);
var closedBracketIndex = key.IndexOf(CLOSE_BRACKET);
var propertyNameSlice = key.AsSpan().Slice(openBracketIndex + 1, closedBracketIndex - openBracketIndex - 1);
var propertyName = propertyNameSlice.ToString();

var propertyName = key.Split(OPEN_BRACKET, CLOSE_BRACKET)[1];

var values = value.Split(COMMA);
foreach (var val in values)
var spanSplitter = value.SpanSplit(COMMA);
for (var i = 0; i < spanSplitter.Count; i++)
{
(var operation, var filterValue) = ParseFilterOperation(val);
queries.Add(new FilterQuery(propertyName, filterValue, operation));
queries.Add(BuildFilterQuery(spanSplitter[i], propertyName));
}

return queries;
}

Expand Down Expand Up @@ -235,5 +236,11 @@ protected virtual AttrAttribute GetAttribute(string propertyName)
throw new JsonApiException(400, $"Attribute '{propertyName}' does not exist on resource '{_controllerContext.RequestEntity.EntityName}'", e);
}
}

private FilterQuery BuildFilterQuery(ReadOnlySpan<char> query, string propertyName)
{
var (operation, filterValue) = ParseFilterOperation(query.ToString());
return new FilterQuery(propertyName, filterValue, operation);
}
}
}
Loading