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

Remove unsafe code and make scheme parsing faster in Uri #90087

Merged
merged 3 commits into from Aug 10, 2023

Conversation

MihaZupan
Copy link
Member

The previous logic had two similar helpers to match strings to schemes (CheckKnownSchemes and CheckSchemeSyntax). CheckKnownSchemes was only used on 32-bit targets, and CheckSchemeSyntax wasn't checking half of the known schemes.

I merged the two helpers into one while removing the unsafe code and recognizing all known schemes on all platforms.
I also changed ParseSchemeCheckImplicitFile to work on a string instead of pointers since the unsafeness wasn't buying us anything. The net result is safer, faster, and fewer LOC.

Method Toolchain uriString Mean Ratio Allocated Alloc Ratio
Parse main https://a 58.63 ns 1.00 56 B 1.00
Parse pr https://a 46.50 ns 0.79 56 B 1.00
Parse main telnet://a 96.12 ns 1.00 96 B 1.00
Parse pr telnet://a 60.24 ns 0.63 56 B 0.58
Parse main wss://a 54.88 ns 1.00 56 B 1.00
Parse pr wss://a 53.77 ns 0.98 56 B 1.00

(not needed for 8.0)

@MihaZupan MihaZupan self-assigned this Aug 7, 2023
@ghost
Copy link

ghost commented Aug 7, 2023

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

Issue Details

The previous logic had two similar helpers to match strings to schemes (CheckKnownSchemes and CheckSchemeSyntax). CheckKnownSchemes was only used on 32-bit targets, and CheckSchemeSyntax wasn't checking half of the known schemes.

I merged the two helpers into one while removing the unsafe code and recognizing all known schemes on all platforms.
I also changed ParseSchemeCheckImplicitFile to work on a string instead of pointers since the unsafeness wasn't buying us anything. The net result is safer, faster, and fewer LOC.

Method Toolchain uriString Mean Ratio Allocated Alloc Ratio
Parse main https://a 58.63 ns 1.00 56 B 1.00
Parse pr https://a 46.50 ns 0.79 56 B 1.00
Parse main telnet://a 96.12 ns 1.00 96 B 1.00
Parse pr telnet://a 60.24 ns 0.63 56 B 0.58
Parse main wss://a 54.88 ns 1.00 56 B 1.00
Parse pr wss://a 53.77 ns 0.98 56 B 1.00

(not needed for 8.0)

Author: MihaZupan
Assignees: MihaZupan
Labels:

area-System.Net

Milestone: -

@EgorBo
Copy link
Member

EgorBo commented Aug 7, 2023

Nice example when a cleaner and safer code is faster 👍

@MihaZupan MihaZupan added this to the 8.0.0 milestone Aug 10, 2023
@MihaZupan MihaZupan merged commit 6f6e248 into dotnet:main Aug 10, 2023
103 checks passed
@dotnet dotnet locked as resolved and limited conversation to collaborators Sep 9, 2023
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.

None yet

3 participants