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

Jump to bottom

POC: remove unneeeded Uri / UriBuilder allocations #1914

Closed
wants to merge 1 commit into from
Closed

POC: remove unneeeded Uri / UriBuilder allocations #1914

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
82 changes: 56 additions & 26 deletions Refit/RequestBuilderImplementation.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -754,53 +754,50 @@ bool paramsContainsCancellationToken
// NB: The URI methods in .NET are dumb. Also, we do this
// UriBuilder business so that we preserve any hardcoded query
// parameters as well as add the parameterized ones.
var urlTarget = BuildRelativePath(basePath, restMethod, paramList);

var uri = new UriBuilder(new Uri(BaseUri, urlTarget));
ParseExistingQueryString(uri, ref queryParamsToAdd);
var urlVsb = new ValueStringBuilder(stackalloc char[StackallocThreshold]);

BuildRelativePath(ref urlVsb, basePath, restMethod, paramList);

// var uri = new UriBuilder(new Uri(BaseUri, urlTarget));
// ParseExistingQueryString(uri, ref queryParamsToAdd);

if (queryParamsToAdd is not null && queryParamsToAdd.Count != 0)
{
uri.Query = CreateQueryString(queryParamsToAdd);;
}
else
{
uri.Query = null;
CreateQueryString(ref urlVsb, queryParamsToAdd, !restMethod.ContainsQuery);
}

ret.RequestUri = new Uri(
uri.Uri.GetComponents(UriComponents.PathAndQuery, restMethod.QueryUriFormat),
urlVsb.ToString(),
UriKind.Relative
);
return ret;
};
}

string BuildRelativePath(string basePath, RestMethodInfoInternal restMethod, object[] paramList)
void BuildRelativePath(ref ValueStringBuilder vsb, string basePath, RestMethodInfoInternal restMethod, object[] paramList)
{
basePath = basePath == "/" ? string.Empty : basePath;
vsb.Append(basePath);

var pathFragments = restMethod.FragmentPath;
if (pathFragments.Count == 0)
{
return basePath;
return;
}
if (string.IsNullOrEmpty(basePath) && pathFragments.Count == 1)
{
Debug.Assert(pathFragments[0].IsConstant);
return pathFragments[0].Value!;
vsb.Append(pathFragments[0].Value!);
return;
}

#pragma warning disable CA2000
var vsb = new ValueStringBuilder(stackalloc char[StackallocThreshold]);
#pragma warning restore CA2000
vsb.Append(basePath);

foreach (var fragment in pathFragments)
{
AppendPathFragmentValue(ref vsb, restMethod, paramList, fragment);
}

return vsb.ToString();
}

void AppendPathFragmentValue(ref ValueStringBuilder vsb, RestMethodInfoInternal restMethod, object[] paramList,
Expand Down Expand Up @@ -1173,13 +1170,13 @@ var value in ParseEnumerableQueryParameterValue(
}
}

static void ParseExistingQueryString(UriBuilder uri, ref List<KeyValuePair<string, string?>>? queryParamsToAdd)
internal static void ParseExistingQueryString(string uri, ref List<KeyValuePair<string, string?>>? queryParamsToAdd)
{
if (string.IsNullOrEmpty(uri.Query))
if (string.IsNullOrEmpty(uri))
return;

queryParamsToAdd ??= [];
var query = HttpUtility.ParseQueryString(uri.Query);
var query = HttpUtility.ParseQueryString(uri);
var index = 0;
foreach (var key in query.AllKeys)
{
Expand All @@ -1193,17 +1190,19 @@ static void ParseExistingQueryString(UriBuilder uri, ref List<KeyValuePair<strin
}
}

static string CreateQueryString(List<KeyValuePair<string, string?>> queryParamsToAdd)
internal static void CreateQueryString(ref ValueStringBuilder vsb, List<KeyValuePair<string, string?>> queryParamsToAdd, bool firstQuery)
{
// Suppress warning as ValueStringBuilder.ToString calls Dispose()
#pragma warning disable CA2000
var vsb = new ValueStringBuilder(stackalloc char[StackallocThreshold]);
#pragma warning restore CA2000
var firstQuery = true;
foreach (var queryParam in queryParamsToAdd)
{
if(queryParam is not { Key: not null, Value: not null })
continue;

if (firstQuery)
{
// query starts with ?
vsb.Append('?');
}

if(!firstQuery)
{
// for all items after the first we add a & symbol
Expand All @@ -1224,6 +1223,37 @@ static string CreateQueryString(List<KeyValuePair<string, string?>> queryParamsT
if (firstQuery)
firstQuery = false;
}
}

internal static string CreateQueryString2(List<KeyValuePair<string, string?>> queryParamsToAdd, bool firstQuery)
{
var vsb = new StringBuilder();
foreach (var queryParam in queryParamsToAdd)
{
if(queryParam is not { Key: not null, Value: not null })
continue;
if(!firstQuery)
{
// for all items after the first we add a & symbol
vsb.Append('&');
}
// var key = Uri.EscapeDataString(queryParam.Key);
var key = queryParam.Key;
#if NET6_0_OR_GREATER
// if first query does not start with ? then prepend it
if (vsb.Length == 0 && key.Length > 0 && key[0] != '?')
{
// query starts with ?
vsb.Append('?');
}
#endif
vsb.Append(key);
vsb.Append('=');
// vsb.Append(Uri.EscapeDataString(queryParam.Value ?? string.Empty));
vsb.Append(queryParam.Value ?? string.Empty);
if (firstQuery)
firstQuery = false;
}

return vsb.ToString();
}
Expand Down
74 changes: 73 additions & 1 deletion Refit/RestMethodInfo.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ internal class RestMethodInfoInternal
public string RelativePath { get; }
public bool IsMultipart { get; }
public string MultipartBoundary { get; private set; }
public bool ContainsQuery { get; private set; }
public string? StrippedUrl { get; private set; }

public RestMethodInfo RestMethodInfo { get; }
public ParameterInfo? CancellationToken { get; }
public UriFormat QueryUriFormat { get; }
Expand Down Expand Up @@ -86,12 +89,21 @@ public RestMethodInfoInternal(
DetermineReturnTypeInfo(methodInfo);
DetermineIfResponseMustBeDisposed();

QueryUriFormat = methodInfo.GetCustomAttribute<QueryUriFormatAttribute>()?.UriFormat
?? UriFormat.UriEscaped;

PreFormatUrl();





// Exclude cancellation token parameters from this list
ParameterInfoArray = methodInfo
.GetParameters()
.Where(static p => p.ParameterType != typeof(CancellationToken))
.ToArray();
(ParameterMap, FragmentPath) = BuildParameterMap(RelativePath, ParameterInfoArray);
(ParameterMap, FragmentPath) = BuildParameterMap(StrippedUrl!, ParameterInfoArray);
BodyParameterInfo = FindBodyParameter(ParameterInfoArray, IsMultipart, hma.Method);
AuthorizeParameterInfo = FindAuthorizationParameter(ParameterInfoArray);

Expand Down Expand Up @@ -179,6 +191,66 @@ public RestMethodInfoInternal(
|| ReturnResultType == typeof(IApiResponse);
}

static readonly Uri BaseUri = new ("http://api");

private void PreFormatUrl()
{
// Extract path query
// If no query early exit and use stripped

// If query then:
// Check query for fragments
// If fragment early return and ensure implementation does runtime stripping
// If not fragments then parse
// Re add fragments if any exist -- Might cause issues for invalid queries as parser may ignore them

var ur = new Uri(BaseUri, RelativePath, true);
var urb = new UriBuilder(ur);
var aq = ur.Query;
var q = urb.Query;

if (string.IsNullOrEmpty(ur.Query))
{
StrippedUrl = RelativePath;
return;
}

var l = new List<KeyValuePair<string, string?>>();
RequestBuilderImplementation.ParseExistingQueryString(ur.Query, ref l);

string q1 = string.Empty;
if (l.Count > 0)
{
ContainsQuery = true;

if (q.Contains("name="))
{
}

q1 = RequestBuilderImplementation.CreateQueryString2(l, true);
// if (q.Contains("name="))
// {
// throw new Exception(q1);
// }
#if NET6_0_OR_GREATER
StrippedUrl = urb.Path + q1;
#else
StrippedUrl = urb.Path + '?' + q1;
#endif
return;
}
else
{
StrippedUrl = urb.Uri.GetComponents(UriComponents.PathAndQuery, QueryUriFormat);
return;
}


if (q.Contains("name="))
{
}
}

public bool HasHeaderCollection => HeaderCollectionParameterIndex >= 0;

public bool HeaderCollectionAt(int index) => HeaderCollectionParameterIndex >= 0 && HeaderCollectionParameterIndex == index;
Expand Down
Loading