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.

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

Use SearchValues in PathString #49117

Merged
merged 2 commits into from Jul 5, 2023
Merged

Conversation

MihaZupan
Copy link
Member

  • Replace PathStringHelper.IsValidPathChar loops with SearchValues
  • Add a vectorized fast-path for longer sections of valid chars
  • Use existing Uri.IsHexEncoding helper instead of IsPercentEncodedChar
  • Remove some dead code and eliminate some redundant length checks

Contributes to #46484

I'll post some benchmark numbers later today, but you can easily expect a 10x improvement for long already-valid values.

@MihaZupan
Copy link
Member Author

MihaZupan commented Jun 30, 2023

Benchmark code

Method Path Mean Error StdDev Ratio
Before LongTestPath 283.907 ns 2.8201 ns 4.2210 ns 16.37
After LongTestPath 17.331 ns 0.0495 ns 0.0693 ns 1.00
Before LongT(...)rcent [24] 302.893 ns 0.7137 ns 1.0683 ns 11.90
After LongT(...)rcent [24] 25.450 ns 0.0900 ns 0.1291 ns 1.00
Before LongValidTestPath 244.094 ns 1.3209 ns 1.9771 ns 32.45
After LongValidTestPath 7.523 ns 0.0330 ns 0.0483 ns 1.00
Before TestPath 24.910 ns 0.2281 ns 0.3271 ns 1.83
After TestPath 13.643 ns 0.0851 ns 0.1193 ns 1.00

With longer inputs, you can get over 40x, and that'll also get a bit better before .NET 8 ships if we sprinkle Avx512 around SearchValues by then.

src/Http/Http.Abstractions/src/PathString.cs Outdated Show resolved Hide resolved
@Tratcher
Copy link
Member

components-e2e is being addressed: #49113

Copy link
Member

@halter73 halter73 left a comment

Choose a reason for hiding this comment

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

🚀

src/Http/Http.Abstractions/src/PathString.cs Outdated Show resolved Hide resolved
@mitchdenny
Copy link
Member

Also related: #49114

@Tratcher
Copy link
Member

Tratcher commented Jul 5, 2023

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@Tratcher Tratcher enabled auto-merge (squash) July 5, 2023 17:47
@Tratcher Tratcher self-assigned this Jul 5, 2023
@Tratcher Tratcher merged commit b287b92 into dotnet:main Jul 5, 2023
25 checks passed
@ghost ghost added this to the 8.0-preview7 milestone Jul 5, 2023
@amcasey amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Aug 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions Perf
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants