Skip to content

Conversation

@MihaZupan
Copy link
Member

  • Removed the unused _schemeDelimiter field
  • Current version has methods SetFieldsFromUri and Init doing exactly the same thing, but Init also calls SetFieldsFromUri. Removed Init to avoid doing the same work twice.
  • For the full ctor that takes string extraValue, avoid stripping ? and # only to add them right back in property setters (avoids unnecessary substring+concat allocations)
  • Rewrote ToString with ValueStringBuilder

Should be functionally equivalent.

private const string Scheme = "https", Host = "example.com", Path = "/random/path?someQuery=value", Extra = "?someQuery=value&foo=bar#some-fragment";
private const int Port = 443;
private static readonly Uri _uri = new($"{Scheme}://{Host}{Path}{Extra}");
private static readonly UriBuilder _builder = new(_uri);

[Benchmark]
public Uri Passthrough() => new UriBuilder(_uri).Uri;

[Benchmark] // What we do in HttpConnectionPool ctor
public string HttpAuthority() => new UriBuilder(Uri.UriSchemeHttp, Host, Port).Uri.IdnHost;

[Benchmark]
public Uri FullCtor() => new UriBuilder(Scheme, Host, Port, Path, Extra).Uri;

[Benchmark]
public string BuilderToString() => _builder.ToString();
Method Toolchain Mean Error StdDev Ratio Gen 0 Allocated
Passthrough base 1,911.8 ns 19.78 ns 18.50 ns 1.00 0.2251 952 B
Passthrough new 1,706.2 ns 7.91 ns 7.40 ns 0.89 0.1736 736 B
HttpAuthority base 1,151.0 ns 14.41 ns 13.48 ns 1.00 0.1526 640 B
HttpAuthority new 1,059.8 ns 8.65 ns 7.22 ns 0.92 0.0992 424 B
FullCtor base 2,276.1 ns 11.72 ns 9.79 ns 1.00 0.3090 1304 B
FullCtor new 2,191.1 ns 4.51 ns 4.22 ns 0.96 0.2289 968 B
BuilderToString base 188.1 ns 2.61 ns 2.32 ns 1.00 0.0973 408 B
BuilderToString new 122.7 ns 0.39 ns 0.37 ns 0.65 0.0477 200 B

There are other tricks we can add to reduce allocations:

  • Lazily access Uri Properties
  • Keep track of which components changed, allowing for optimizations in ToString like fetching PathAndQuery instead of Path, Query

@MihaZupan MihaZupan added this to the 6.0.0 milestone Apr 25, 2021
@MihaZupan MihaZupan requested a review from a team April 25, 2021 00:03
@ghost
Copy link

ghost commented Apr 25, 2021

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details
  • Removed the unused _schemeDelimiter field
  • Current version has methods SetFieldsFromUri and Init doing exactly the same thing, but Init also calls SetFieldsFromUri. Removed Init to avoid doing the same work twice.
  • For the full ctor that takes string extraValue, avoid stripping ? and # only to add them right back in property setters (avoids unnecessary substring+concat allocations)
  • Rewrote ToString with ValueStringBuilder

Should be functionally equivalent.

private const string Scheme = "https", Host = "example.com", Path = "/random/path?someQuery=value", Extra = "?someQuery=value&foo=bar#some-fragment";
private const int Port = 443;
private static readonly Uri _uri = new($"{Scheme}://{Host}{Path}{Extra}");
private static readonly UriBuilder _builder = new(_uri);

[Benchmark]
public Uri Passthrough() => new UriBuilder(_uri).Uri;

[Benchmark] // What we do in HttpConnectionPool ctor
public string HttpAuthority() => new UriBuilder(Uri.UriSchemeHttp, Host, Port).Uri.IdnHost;

[Benchmark]
public Uri FullCtor() => new UriBuilder(Scheme, Host, Port, Path, Extra).Uri;

[Benchmark]
public string BuilderToString() => _builder.ToString();
Method Toolchain Mean Error StdDev Ratio Gen 0 Allocated
Passthrough base 1,911.8 ns 19.78 ns 18.50 ns 1.00 0.2251 952 B
Passthrough new 1,706.2 ns 7.91 ns 7.40 ns 0.89 0.1736 736 B
HttpAuthority base 1,151.0 ns 14.41 ns 13.48 ns 1.00 0.1526 640 B
HttpAuthority new 1,059.8 ns 8.65 ns 7.22 ns 0.92 0.0992 424 B
FullCtor base 2,276.1 ns 11.72 ns 9.79 ns 1.00 0.3090 1304 B
FullCtor new 2,191.1 ns 4.51 ns 4.22 ns 0.96 0.2289 968 B
BuilderToString base 188.1 ns 2.61 ns 2.32 ns 1.00 0.0973 408 B
BuilderToString new 122.7 ns 0.39 ns 0.37 ns 0.65 0.0477 200 B

There are other tricks we can add to reduce allocations:

  • Lazily access Uri Properties
  • Keep track of which components changed, allowing for optimizations in ToString like fetching PathAndQuery instead of Path, Query
Author: MihaZupan
Assignees: -
Labels:

area-System.Net

Milestone: 6.0.0

Copy link
Member

@ManickaP ManickaP left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

@MihaZupan
Copy link
Member Author

/azp run runtime

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@MihaZupan MihaZupan merged commit aaf171c into dotnet:main May 13, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Jun 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants