Skip to content

Optimize UriHelper.GetRelativeUrl#783

Merged
kevingosse merged 2 commits into
masterfrom
kevin/urihelper
Jul 8, 2020
Merged

Optimize UriHelper.GetRelativeUrl#783
kevingosse merged 2 commits into
masterfrom
kevin/urihelper

Conversation

@kevingosse

@kevingosse kevingosse commented Jul 6, 2020

Copy link
Copy Markdown
Contributor

Rewrote the method to get rid of some intermediate string allocations.

We could probably do much better if we had a netcoreapp target, it's worth considering.

Benchmark:

UriHelper.GetRelativeUrl(new Uri("https://localhost:5101/mvc/queries/raw"))
BenchmarkDotNet=v0.12.0, OS=Windows 10.0.19041
Intel Core i7-9750H CPU 2.60GHz, 1 CPU, 12 logical and 6 physical cores
  [Host]     : .NET Framework 4.8 (4.8.4180.0), X64 RyuJIT
  Job-VTGAFA : .NET Framework 4.8 (4.8.4180.0), X64 RyuJIT
  Job-MNUBEA : .NET Core 3.1.5 (CoreCLR 4.700.20.26901, CoreFX 4.700.20.27001), X64 RyuJIT
Method Runtime Mean Error StdDev Ratio Gen 0 Gen 1 Gen 2 Allocated
Old .NET 4.7.2 1,239.0 ns 19.69 ns 17.46 ns 1.00 0.1030 - - 658 B
New .NET 4.7.2 660.6 ns 4.33 ns 3.61 ns 0.53 0.0429 - - 273 B
Old .NET Core 3.1 579.6 ns 11.57 ns 11.88 ns 1.00 0.0782 - - 496 B
New .NET Core 3.1 303.4 ns 2.92 ns 2.58 ns 0.52 0.0420 - - 264 B

@kevingosse
Optimize UriHelper.CleanUriSegment
840aa2a 
Reduce the number of intermediate strings.
@kevingosse
kevingosse requested a review from a team as a code owner July 6, 2020 11:41
lucaspimentel
lucaspimentel reviewed Jul 7, 2020
View reviewed changes
Comment thread src/Datadog.Trace/Util/UriHelpers.cs
lucaspimentel
lucaspimentel approved these changes Jul 7, 2020
View reviewed changes

@lucaspimentel lucaspimentel left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. Just one request to restore a lost comment.

@kevingosse
kevingosse merged commit 3c2e450 into master Jul 8, 2020
@kevingosse
kevingosse deleted the kevin/urihelper branch July 8, 2020 13:17
@zacharycmontoya zacharycmontoya added this to the 1.18.2 milestone Jul 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lucaspimentel lucaspimentel lucaspimentel approved these changes
@zacharycmontoya zacharycmontoya zacharycmontoya approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

1.18.2

Development

Successfully merging this pull request may close these issues.

3 participants

@kevingosse @lucaspimentel @zacharycmontoya