Skip to content

Use more pattern matching in Uri#97584

Closed
MihaZupan wants to merge 1 commit intodotnet:mainfrom
MihaZupan:uri-pattern-match
Closed

Use more pattern matching in Uri#97584
MihaZupan wants to merge 1 commit intodotnet:mainfrom
MihaZupan:uri-pattern-match

Conversation

@MihaZupan
Copy link
Member

@MihaZupan MihaZupan commented Jan 27, 2024

No description provided.

@MihaZupan MihaZupan added this to the 9.0.0 milestone Jan 27, 2024
@MihaZupan MihaZupan self-assigned this Jan 27, 2024
@ghost
Copy link

ghost commented Jan 27, 2024

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

Issue Details

null

Author: MihaZupan
Assignees: MihaZupan
Labels:

area-System.Net
Milestone: 9.0.0

@danmoseley
Copy link
Member

danmoseley commented Jan 27, 2024

Maybe I'm out of date, but what is the improvement from using this syntax? It's not consistency as we now use both.

This was referenced Jan 27, 2024
Closed
Closed
@MihaZupan
Copy link
Member Author

MihaZupan commented Jan 27, 2024

I much prefer it for readability in many cases.
But it looks like the performance is meaningfully worse here, which is unfortunate for a more readable syntax that we encourage users to switch to.

@MihaZupan MihaZupan closed this Jan 27, 2024
@stephentoub
Copy link
Member

stephentoub commented Jan 27, 2024 •
edited
Loading

But it looks like the performance is meaningfully worse here

Do you have an example for what resulted in worse perf? In general, that shouldn't be the case, and we should fix whatever is resulting in the degradation.

@MihaZupan
Copy link
Member Author

MihaZupan commented Jan 27, 2024 •
edited
Loading

Running the dotnet/performance suite:

Benchmark results
Method Toolchain input Mean Error Ratio
ParseAbsoluteUri main ? 255.423 ns 2.1193 ns 1.00
ParseAbsoluteUri pr ? 264.953 ns 5.8306 ns 1.04
DnsSafeHost main ? 201.743 ns 5.6943 ns 1.00
DnsSafeHost pr ? 218.065 ns 1.9770 ns 1.08
GetComponents main ? 16.714 ns 0.0945 ns 1.00
GetComponents pr ? 18.036 ns 0.3221 ns 1.08
PathAndQuery main ? 1.503 ns 0.0137 ns 1.00
PathAndQuery pr ? 1.810 ns 0.0510 ns 1.20
BuilderToString main ? 109.226 ns 8.4738 ns 1.00
BuilderToString pr ? 102.749 ns 1.3287 ns 0.95
UriBuilderReplacePort main ? 94.191 ns 1.5391 ns 1.00
UriBuilderReplacePort pr ? 95.959 ns 1.4414 ns 1.02
EscapeDataString main {{{{{{{{{{{{(...){{{{{{{{{{{{ [1000] 6,281.305 ns 87.2309 ns 1.00
EscapeDataString pr {{{{{{{{{{{{(...){{{{{{{{{{{{ [1000] 6,154.104 ns 75.5977 ns 0.98
CombineAbsoluteRelative main /new/path 142.783 ns 1.2317 ns 1.00
CombineAbsoluteRelative pr /new/path 147.263 ns 1.8481 ns 1.03
UnescapeDataString main %E4%BD%A0%E5%A5%BD 44.656 ns 0.3969 ns 1.00
UnescapeDataString pr %E4%BD%A0%E5%A5%BD 44.968 ns 0.3986 ns 1.01
EscapeDataString main a{üa{üa{üa{ü(...)a{üa{üa{üa{ü [999] 9,177.623 ns 86.6012 ns 1.00
EscapeDataString pr a{üa{üa{üa{ü(...)a{üa{üa{üa{ü [999] 9,173.818 ns 117.7571 ns 1.00
EscapeDataString main aaaaaaaaaaaa(...)aaaaaaaaaaaa [1000] 37.504 ns 0.0904 ns 1.00
EscapeDataString pr aaaaaaaaaaaa(...)aaaaaaaaaaaa [1000] 37.712 ns 0.1888 ns 1.01
UnescapeDataString main abc%20def%20ghi%20 36.714 ns 1.1608 ns 1.00
UnescapeDataString pr abc%20def%20ghi%20 34.856 ns 0.0741 ns 0.95
Ctor main http://dot.net 70.001 ns 0.3920 ns 1.00
Ctor pr http://dot.net 76.544 ns 2.0060 ns 1.09
CtorIdnHostPathAndQuery main http://dot.ne(...)alue#fragment [43] 275.439 ns 1.8090 ns 1.00
CtorIdnHostPathAndQuery pr http://dot.ne(...)alue#fragment [43] 317.664 ns 2.7076 ns 1.15
Ctor main http://höst.with.ünicode 315.809 ns 1.6437 ns 1.00
Ctor pr http://höst.with.ünicode 317.948 ns 0.6515 ns 1.01
CtorIdnHostPathAndQuery main http://höst.w(...)alue#fragment [53] 2,439.020 ns 17.2119 ns 1.00
CtorIdnHostPathAndQuery pr http://höst.w(...)alue#fragment [53] 2,464.614 ns 14.6206 ns 1.01
CtorIdnHostPathAndQuery main http://host/ 158.195 ns 0.4417 ns 1.00
CtorIdnHostPathAndQuery pr http://host/ 165.208 ns 0.8415 ns 1.04
CtorIdnHostPathAndQuery main http://host/p(...)s?key=ünicode [50] 623.105 ns 10.5771 ns 1.00
CtorIdnHostPathAndQuery pr http://host/p(...)s?key=ünicode [50] 656.604 ns 4.7513 ns 1.05
CtorIdnHostPathAndQuery main http://host/p(...)es?key=va lue [49] 397.447 ns 4.9882 ns 1.00
CtorIdnHostPathAndQuery pr http://host/p(...)es?key=va lue [49] 432.754 ns 4.6648 ns 1.09
CtorIdnHostPathAndQuery main http://host/p(...)3&key4=value4 [64] 349.319 ns 9.7977 ns 1.00
CtorIdnHostPathAndQuery pr http://host/p(...)3&key4=value4 [64] 381.831 ns 8.5119 ns 1.10
CtorIdnHostPathAndQuery main http://host/p(...)=%C3%BCnicode [61] 661.306 ns 3.8569 ns 1.00
CtorIdnHostPathAndQuery pr http://host/p(...)=%C3%BCnicode [61] 704.674 ns 4.4607 ns 1.07
CtorIdnHostPathAndQuery main http://host/p(...)?key=va%20lue [57] 301.804 ns 4.3096 ns 1.00
CtorIdnHostPathAndQuery pr http://host/p(...)?key=va%20lue [57] 348.459 ns 2.1292 ns 1.16
Ctor main http://xn--hs(...)n--nicode-2ya [38] 117.371 ns 0.9235 ns 1.00
Ctor pr http://xn--hs(...)n--nicode-2ya [38] 117.082 ns 1.1367 ns 1.00
CtorIdnHostPathAndQuery main http://xn--hs(...)alue#fragment [67] 355.952 ns 2.7841 ns 1.00
CtorIdnHostPathAndQuery pr http://xn--hs(...)alue#fragment [67] 387.186 ns 7.0384 ns 1.09
Ctor main https://a.much.longer.domain.name 127.841 ns 3.5481 ns 1.00
Ctor pr https://a.much.longer.domain.name 146.038 ns 2.0395 ns 1.15
CtorIdnHostPathAndQuery main https://a.muc(...)alue#fragment [62] 361.930 ns 4.3985 ns 1.00
CtorIdnHostPathAndQuery pr https://a.muc(...)alue#fragment [62] 392.181 ns 3.1871 ns 1.08
Ctor main https://contoso.com 75.513 ns 0.6868 ns 1.00
Ctor pr https://contoso.com 84.819 ns 0.9464 ns 1.12
Ctor main https://CONTOSO.com 75.050 ns 0.8408 ns 1.00
Ctor pr https://CONTOSO.com 84.753 ns 1.0033 ns 1.13
CtorIdnHostPathAndQuery main https://conto(...)alue#fragment [48] 290.362 ns 2.4067 ns 1.00
CtorIdnHostPathAndQuery pr https://conto(...)alue#fragment [48] 328.075 ns 2.1609 ns 1.13
CtorIdnHostPathAndQuery main https://CONTO(...)alue#fragment [48] 290.552 ns 2.1273 ns 1.00
CtorIdnHostPathAndQuery pr https://CONTO(...)alue#fragment [48] 330.605 ns 4.1867 ns 1.14
EscapeDataString main üüüüüüüüüüüü(...)üüüüüüüüüüüü [1000] 9,644.291 ns 97.6889 ns 1.00
EscapeDataString pr üüüüüüüüüüüü(...)üüüüüüüüüüüü [1000] 9,582.920 ns 114.6866 ns 0.99

Codegen-wise they're all size regressions

Size diffs 
Top method regressions (bytes):
          93 (8.87 % of base) : System.Private.Uri.dasm - System.Uri:CheckCanonical(ulong,byref,int,ushort):int:this (FullOpts)
          91 (3.32 % of base) : System.Private.Uri.dasm - System.Uri:CheckAuthorityHelper(ulong,int,int,byref,byref,System.UriParser,byref):int:this (FullOpts)
          77 (11.26 % of base) : System.Private.Uri.dasm - System.Uri:CreateHostString():this (FullOpts)
          74 (4.94 % of base) : System.Private.Uri.dasm - System.Uri:PrivateParseMinimal():int:this (FullOpts)
          66 (8.37 % of base) : System.Private.Uri.dasm - System.Uri:GetHostViaCustomSyntax():this (FullOpts)
          65 (11.05 % of base) : System.Private.Uri.dasm - System.UncNameHelper:IsValid(ulong,int,byref,ubyte):ubyte (FullOpts)
          53 (6.73 % of base) : System.Private.Uri.dasm - System.Uri:ResolveHelper(System.Uri,System.Uri,byref,byref):System.Uri (FullOpts)
          50 (1.64 % of base) : System.Private.Uri.dasm - System.Uri:CombineUri(System.Uri,System.String,int):System.String (FullOpts)
          47 (4.70 % of base) : System.Private.Uri.dasm - System.IPv6AddressHelper:Parse(System.ReadOnlySpan`1[ushort],System.Span`1[ushort],int,byref) (FullOpts)
          47 (4.70 % of base) : System.Net.Primitives.dasm - System.IPv6AddressHelper:Parse(System.ReadOnlySpan`1[ushort],System.Span`1[ushort],int,byref) (FullOpts)
          45 (55.56 % of base) : System.Private.Uri.dasm - System.Uri:IsExcludedCharacter(ushort):ubyte (FullOpts)
          43 (71.67 % of base) : System.Private.Uri.dasm - System.Uri:IsBadFileSystemCharacter(ushort):ubyte:this (FullOpts)
          42 (4.29 % of base) : System.Private.Uri.dasm - System.Uri:CreateUriInfo(ulong):this (FullOpts)
          37 (12.89 % of base) : System.Net.Security.dasm - System.IPv4AddressHelper:IsValidCanonical(ulong,int,byref,ubyte,ubyte):ubyte (FullOpts)
          37 (12.89 % of base) : System.Private.Uri.dasm - System.IPv4AddressHelper:IsValidCanonical(ulong,int,byref,ubyte,ubyte):ubyte (FullOpts)
          37 (12.89 % of base) : System.Net.Quic.dasm - System.IPv4AddressHelper:IsValidCanonical(ulong,int,byref,ubyte,ubyte):ubyte (FullOpts)
          37 (12.89 % of base) : System.Net.Primitives.dasm - System.IPv4AddressHelper:IsValidCanonical(ulong,int,byref,ubyte,ubyte):ubyte (FullOpts)
          36 (73.47 % of base) : System.Private.Uri.dasm - System.Uri:IsReservedCharacter(ushort):ubyte:this (FullOpts)
          21 (3.18 % of base) : System.Net.Security.dasm - System.IPv4AddressHelper:ParseNonCanonical(ulong,int,byref,ubyte):long (FullOpts)
          21 (3.18 % of base) : System.Private.Uri.dasm - System.IPv4AddressHelper:ParseNonCanonical(ulong,int,byref,ubyte):long (FullOpts)
          21 (3.18 % of base) : System.Net.Quic.dasm - System.IPv4AddressHelper:ParseNonCanonical(ulong,int,byref,ubyte):long (FullOpts)
          21 (3.18 % of base) : System.Net.Primitives.dasm - System.IPv4AddressHelper:ParseNonCanonical(ulong,int,byref,ubyte):long (FullOpts)
          19 (1.51 % of base) : System.Private.Uri.dasm - System.Uri:GetLocalPath():System.String:this (FullOpts)
          18 (32.73 % of base) : System.Private.Uri.dasm - System.UriHelper:IsGenDelim(ushort):ubyte (FullOpts)
          11 (22.45 % of base) : System.Private.Uri.dasm - System.UriHelper:IsLWS(ushort):ubyte (FullOpts)
          10 (6.85 % of base) : System.Private.Uri.dasm - System.IPv4AddressHelper:ParseHostNumber(System.ReadOnlySpan`1[ushort],int,int):int (FullOpts)
          10 (6.85 % of base) : System.Net.Primitives.dasm - System.IPv4AddressHelper:ParseHostNumber(System.ReadOnlySpan`1[ushort],int,int):int (FullOpts)
           9 (2.57 % of base) : System.Private.Uri.dasm - System.Uri:GetCombinedString(System.Uri,System.String,ubyte,byref) (FullOpts)
           8 (0.27 % of base) : System.Private.Uri.dasm - System.Uri:ParseRemaining():this (FullOpts)
           5 (0.47 % of base) : System.Private.Uri.dasm - System.Uri:InternalIsWellFormedOriginalString():ubyte:this (FullOpts)

I know Roslyn will emit different approaches here (e.g. binary search instead of just N comparisons), but I haven't looked into which parts were actually contributing most to the regressions.

@github-actions github-actions bot locked and limited conversation to collaborators Feb 27, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

@MihaZupan MihaZupan

Labels

area-System.Net

Projects

None yet

Milestone

9.0.0

Development

Successfully merging this pull request may close these issues.

3 participants

@MihaZupan @danmoseley @stephentoub