aspnetcore icon indicating copy to clipboard operation
aspnetcore copied to clipboard

Removed bound checks in SharedUrlHelper.IsLocalUrl and use JIT unrolling for checks

Open gfoidl opened this issue 9 months ago • 5 comments

The method is used for HTTP Redirects to check if the url is local or not.

The changes to the code are to help the JIT to elide the bound checks, and to collapse some checks (e.g. for '~' and '/') to only one check.

Benchmarking gives on my machine:

Method Url Mean Error StdDev Ratio RatioSD
Default / 1.080 ns 0.0682 ns 0.1377 ns 1.02 0.19
PR / 1.159 ns 0.0334 ns 0.0279 ns 1.09 0.15
Default /foo 3.688 ns 0.1248 ns 0.3662 ns 1.01 0.14
PR /foo 1.976 ns 0.0816 ns 0.1875 ns 0.54 0.07
Default ~/bar 4.307 ns 0.1295 ns 0.1330 ns 1.00 0.04
PR ~/bar 1.687 ns 0.0532 ns 0.0444 ns 0.39 0.02

gfoidl avatar Apr 07 '25 09:04 gfoidl

Thanks for your PR, @@gfoidl. Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

What's the impact on allocations if you add [MemoryDiagnoser] to your benchmark?

martincostello avatar Apr 07 '25 10:04 martincostello

There are either way no allocations at all.

gfoidl avatar Apr 07 '25 11:04 gfoidl

Looks like this PR hasn't been active for some time and the codebase could have been changed in the meantime. To make sure no conflicting changes have occurred, please rerun validation before merging. You can do this by leaving an /azp run comment here (requires commit rights), or by simply closing and reopening.

No assigned reviewers?

ladeak avatar Aug 31 '25 11:08 ladeak