Note regarding thenew-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

Hi@edwardneal, sorry for the delay in getting to this, I had missed the tag initially since this is under the networking area.

It's worth noting thatIPAddress andIPNetwork are not part of APIs that#81500 was approved for, and so before this can move forward there needs to be a new API proposal that specifically asks for implementingIUtf8SpanParsable on these types.

That proposal should be straightforward and I expect have no issues going through API review, at which point this can likely get merged. If you could assist in opening such a proposal, that should help the process go along faster.

Thanks@tannergooding. Sorry for the confusion here - I read the original issue as approving the addition ofIUtf8SpanParsable andIUtf8SpanFormattable to anything already implementingISpanFormattable, andIPAddress andIPNetwork both do so. Should I just raise the API proposal for the additionalIPNetwork.Parse(ReadOnlySpan<byte>) andIPNetwork.TryParse(ReadOnlySpan<byte>, out IPNetwork) methods andIPNetwork's explicit interface implementation, or should I also includeIPAddress in that?

should I also include IPAddress in that?

We want to include all new public APIs and interfaces in the proposal whether that is forIUtf8SpanParsable,IUtf8SpanFormattable, or both; as well as any overloads beyond this.

I read the original issue as approving the addition of IUtf8SpanParsable and IUtf8SpanFormattable to anything already implementing ISpanFormattable

The general spirit behind the feature fits that, but we still do explicit review and sign-off as there are often edge cases that we may warrant different consideration.

Given that we are blocked on API review, I would recommend to either change the PR to Draft or close it, until the API is approved.@edwardneal what do you prefer and can you please do it? Thanks!

@EgorBot

usingBenchmarkDotNet.Attributes;usingBenchmarkDotNet.Running;usingSystem;usingSystem.Net;publicclassBench{[Benchmark][Arguments("192.168.0.1")][Arguments("4294967295")][Arguments("037777777777")][Arguments("0xff.0x7f.0x20.0x01")][Arguments("192.168.0.0/16")][Arguments("::192.168.0.1")][Arguments("100:0:1:2:0:0:000:abcd")][Arguments("Fe08::1%13542")][Arguments("1:2:3:4:5:6:7:8::")]publicboolTryParse(strings)=>IPAddress.TryParse(s,out_);[Benchmark][Arguments("http://192.168.0.1:123/foo")][Arguments("http://[100:0:1:2:0:0:000:abcd]:123/foo")]publicstringUriHost(strings)=>newUri(s).Host;}

Thanks MihaZupan. That's quite a wide regression in::192.168.0.1, and I don't see a clear reason why. Some performance is left on the table inHexConverter.FromChar and similar (it doesn't cast its input to a uint, so the JIT doesn't elide the bounds check and we end up performing it twice) but we were using that method anyway - just indirectly, as a result of calling Uri.FromHex. In any event though, this method isn't called in the slower case - that test case's code path hasn't actually changed.

I've reverted the only other difference so that there's a common baseline, and I'll keep looking at this. I'm faintly suspicious that passing around an int rather than a char is causing the JIT to implicitly reintroduce bounds checks, but I wouldn't expect them to have such a large impact in this case.

Some performance is left on the table in HexConverter.FromChar and similar (it doesn't cast its input to a uint, so the JIT doesn't elide the bounds check and we end up performing it twice)

Can you point me exactly to the non-elided bound check? Last I checked, all users ofFromChar give it something JIT can see it's never negative. E.g. Uri.FromHex passeschar toHexConverter.FromChar, hence, never negative.

I agree.FromChar is defined as

[MethodImpl(MethodImplOptions.AggressiveInlining)]publicstaticintFromChar(intc){returnc>=CharToHexLookup.Length?0xFF:CharToHexLookup[c];}

c isn't cast to an unsigned type before the comparison toCharToHexLookup.Length, so if the caller passes it an int then a bounds check is performed.

That's not normally a problem - like you said, Uri.FromHex passes it a char, so when it's inlined it makes perfect sense that the JIT can see which datatype is really being passed around. I'm passing it an integer though, so the bounds check would likely remain.

I'm hesitant to say that it's the only cause of the performance regression because when I benchmarked it, the bounds checks only introduced a 0.2ns/call performance gap, even on a slower laptop. I'm going to test the idea over the next few days (edit: this'll likely be the weekend now, sorry.)

Updating on this: investigating the performance regressions led to a few places whereCreateTruncating of a non-constant value performed a few nanoseconds slower than I'd expected, in the middle of a number of hot loops. I've replaced this with a separateToUShort method, which performs a direct cast.

The UriHost benchmarks were stranger. As best I can tell,IPv4AddressHelper.Parse's resultant assembly was 200 bytes smaller when JITed under the PR environment than when JITed under .NET 9.0 RC2. This led to a number of different inlining decisions. I've brought the PR up to date with the main branch and re-run the benchmarks... they look roughly acceptable to me.

BenchmarkDotNet v0.14.0, Windows 10 (10.0.20348.2582) (Hyper-V)Intel Xeon Platinum 8370C CPU 2.80GHz, 1 CPU, 8 logical and 4 physical cores.NET SDK 9.0.100-rc.2.24474.11  [Host]   : .NET 9.0.0 (9.0.24.47305), X64 RyuJIT AVX-512F+CD+BW+DQ+VL+VBMI  Baseline : .NET 9.0.0 (9.0.24.47305), X64 RyuJIT AVX-512F+CD+BW+DQ+VL+VBMI  CoreRun  : .NET 10.0.0 (42.42.42.42424), X64 RyuJIT AVX-512F+CD+BW+DQ+VL+VBMI

BenchmarkDotNet v0.14.0, Windows 10 (10.0.20348.2582) (Hyper-V)Intel Xeon Platinum 8370C CPU 2.80GHz, 1 CPU, 8 logical and 4 physical cores.NET SDK 9.0.100-rc.2.24474.11  [Host]   : .NET 9.0.0 (9.0.24.47305), X64 RyuJIT AVX-512F+CD+BW+DQ+VL+VBMI  Baseline : .NET 9.0.0 (9.0.24.47305), X64 RyuJIT AVX-512F+CD+BW+DQ+VL+VBMI  CoreRun  : .NET 10.0.0 (42.42.42.42424), X64 RyuJIT AVX-512F+CD+BW+DQ+VL+VBMI

Pending any surprises from CI, I'll re-request the benchmark from EgorBot.

@EgorBot

usingBenchmarkDotNet.Attributes;usingBenchmarkDotNet.Running;usingSystem;usingSystem.Net;publicclassBench{[Benchmark][Arguments("192.168.0.1")][Arguments("4294967295")][Arguments("037777777777")][Arguments("0xff.0x7f.0x20.0x01")][Arguments("192.168.0.0/16")][Arguments("::192.168.0.1")][Arguments("100:0:1:2:0:0:000:abcd")][Arguments("Fe08::1%13542")][Arguments("1:2:3:4:5:6:7:8::")]publicboolTryParse(strings)=>IPAddress.TryParse(s,out_);[Benchmark][Arguments("http://192.168.0.1:123/foo")][Arguments("http://[100:0:1:2:0:0:000:abcd]:123/foo")]publicstringUriHost(strings)=>newUri(s).Host;}

That's great, thanks for your patience@MihaZupan and everyone who reviewed. There are a few optimisations which I want to pick up from the earlier revisions, but I'll definitely re-read the earlier conversations on this PR first - I'm fairly sure I dragged the review process out by missing the point of some of them!