[Perf -19%] System.Net.NetworkInformation.Tests.PhysicalAddressTests.PAShort

[DrewScoggins]

Run Information

Architecture	x64
OS	Windows 10.0.18362
Changes	diff

Regressions in System.Net.NetworkInformation.Tests.PhysicalAddressTests

Benchmark	Baseline	Test	Test/Base	Modality	Baseline Outlier
PAShort	14.44 ns	17.14 ns	1.19		False

Historical Data in Reporting System

Repro

git clone https://github.com/dotnet/performance.git
py .\performance\scripts\benchmarks_ci.py -f netcoreapp5.0 --filter "System.Net.NetworkInformation.Tests.PhysicalAddressTests*"

Details

Histogram

System.Net.NetworkInformation.Tests.PhysicalAddressTests.PAShort

[14.317 ; 14.884) | @@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@
[14.884 ; 15.542) | @@@@@@@@@
[15.542 ; 16.023) | @@@
[16.023 ; 16.671) | @
[16.671 ; 17.238) | @@@@@@@@@@@@@@@@@@@@@@@
[17.238 ; 17.945) | @@@
[17.945 ; 18.526) | @@

Docs

Profiling workflow for dotnet/runtime repository
Benchmarking workflow for dotnet/runtime repository

Tagging subscribers to this area: @dotnet/ncl
Notify danmosemsft if you want to be subscribed.

[karelz]

Triage: Not important for 5.0, still might be nice to see what caused it, but there haven't been any changes in the space ...
Fine to just close it as "OK" in future.

[DrewScoggins]

@tannergooding who mentioned that he thought there was some change in this space.

[wfurt]

hmm That lines up with #37546 on Jul 17. That change did not go through ncl.

[danmoseley]

It does seem very likely #37546. It's the only plausible commit in this interval . cc @tkp1n in case he is interested.

[tkp1n]

The additional ~3ns can easily be explained by the added overhead of using the new public hex encoding API instead of an internal method without as many input checks.

The easy fix would be to revert this line from:
return Convert.ToHexString(_address.AsSpan());
back to
return HexConverter.ToString(_address.AsSpan(), HexConverter.Casing.Upper);

@danmosemsft Feel free to assign this issue to me, if you want me to open a PR and investigate whether the proposed change would actually help.

I've also opened #39702 (some time ago) to vectorize hex encoding/decoding. This would provide a more sustainable solution that'll also help other places in the framework. Until now, no one from the team seemed to care.