Replies: 4 comments 13 replies
|
The current benchmark cannot isolate a .NET 10 runtime regression because Split them into comparable benchmarks: run [MemoryDiagnoser]
[DisassemblyDiagnoser]
public class HexBenchmarks { /* same API per runtime */ }That will separate allocation changes from decoder/codegen changes and show whether the long-input slowdown is in |
|
namespace HexBench;
public class HexEncoder
{
public byte[] DecodeData(string encoded)
{
ArgumentNullException.ThrowIfNull(encoded);
return Convert.FromHexString(encoded);
}
public void DecodeData(string encoded, Span<byte> output)
{
if (encoded == null)
throw new ArgumentNullException(nameof(encoded));
if (encoded.Length % 2 == 1)
throw new FormatException("Invalid Hex String");
if (output.Length < (encoded.Length >> 1))
throw new ArgumentException("output should be bigger", nameof(output));
var decoded = Convert.FromHexString(encoded);
decoded.CopyTo(output);
}
}
using BenchmarkDotNet.Attributes;
namespace HexBench;
[MemoryDiagnoser]
[DisassemblyDiagnoser]
public class HexBenchmarks
{
const string str = "d54994ece1d11b19785c7248868696250ab195605b469632b7bd68130e880c9a";
const string uint256_str = "00000000ffffffffffffffffffffffffffffffffffffffffffffffffffffffff";
string random_str_10kb = null!;
byte[] bytes = null!;
HexEncoder hex = new HexEncoder();
[GlobalSetup]
public void Setup()
{
bytes = hex.DecodeData(str);
var randomBytes = new byte[10 * 1024];
Random.Shared.NextBytes(randomBytes);
random_str_10kb = Convert.ToHexString(randomBytes);
}
[Benchmark]
public void DecodeData()
{
hex.DecodeData(str);
}
[Benchmark]
[Arguments(uint256_str)] // 32 bytes
[Arguments(uint256_str + uint256_str)] // 64 bytes
[Arguments(uint256_str + uint256_str + uint256_str)] // 96 bytes
public void DecodeDataSpan(string hexString)
{
Span<byte> tmp = stackalloc byte[hexString.Length / 2];
hex.DecodeData(hexString, tmp);
}
[Benchmark]
public void DecodeDataLongSpan()
{
Span<byte> tmp = stackalloc byte[random_str_10kb.Length / 2];
hex.DecodeData(random_str_10kb, tmp);
}
}and my results are:
Thanks. I will check it out more in detail. |
|
@tannergooding Is it possible that this PR #117965 affects performance for .NET 10? |
|
This revised benchmark is now comparable for One remaining correction: Convert.FromHexString(hexString, tmp, out _, out _);That separates the decoder regression from allocation and |
|
The regression comes from dotnet/runtime#117965, which changed the decoder's generic ASCII check from dotnet/runtime#130086 already resolved this for .NET 11 and could be considered for backport to .NET 10 CC. @JulieLeeMSFT for if that would meet the bar |
|
That's why I raised it for a bar check because it can be a regression for some users with extremely large inputs. What I was calling out is that this remains a tiny regression and it is incredibly unlikely to be a real world bottleneck or even show up as some For example, on my box I see The entire works of Shakespear in contrast is around 1/5th that size and takes Typical user machines are likely to see worse numbers than my high end box, but the practical consideration remains the same and hence the additional ask for concrete user input on if this is a bottleneck for them to help with justifying the backport. -- I'd also note that there is at least an additional 2x perf on the table even in .NET 11 for such large inputs and if it were properly tuned potentially 4-8x, I just haven't checked beyond the basic SIMD kernel optimizations. This is an optimization that only benefits large data though, because it helps ensure the CPU and memory properly saturate. The concrete data and information is important because relative regression numbers really don't tell an interesting story. |
|
i.e. This isn't defending anything, it's asking for the required details to ensure that the backport request can be properly filled out because it could be impactful and it could be a non-issue, depending on many different factors We already have a minimal 10 line change that works and is viable to backport if the bar check is met: #130086 |
|
To be honest, I fail to understand your point here with very long replies defending this. It's clearly an oversight and that happens and it's normal. In my example a 7kb file regressed by almost 5x (2 us -> 9 us, clearly not nanoseconds). It's not unusual to embed files in XML/JSON/databases as hex or base64 strings, so it's definitely not about just small strings. We've had a few reports from internal customers that they observed perf regressions after udpating to net10, so I am trying to make sure regressions aren't left unnoticed. This one effectively killed the vectorization we did for this API a few releases back.
That change touches JIT and could be a bit more risky thant just changing == to .Equals IMO. |
For example, Bitcoin transaction IDs are commonly represented in HEX. Public keys and private keys are often represented in hex too. Given that Bitcoin blockchain is rather big now, slower HEX operations can affect performance of some applications. However, it's somewhat hard to quantify it because it depends on actual use case. Anyway, I would sort of expect many other applications to use HEX format extensively because it's just pretty common format. |
Its about actually understanding and fixing the issue at hand rather than guessing at a patch which may not actually handle the issue (i.e. why my response went into the detail it did).
It's really not that long of a reply and covers what the regression is, why it exists, and why it may or may not be an issue. It asks for additional data so that we can make an appropriate judgement on whether or not a backport is then needed. |
|
For the sake of completeness, I measured even .NET 11 and my results are:
This is to demonstrate that the regression for .NET 10 is non-trivial (50%+, and even 200%+). .NET 11 gives nice improvements. |
Uh oh!
There was an error while loading. Please reload this page.
Disclaimer: This is extracted from this PR: MetacoSA/NBitcoin#1333 (comment)
I basically measure this method execution times when executed using .NET 8 and .NET 10:
I created a small repro project
hex-project.zip
that can be benchmarked using:
I get this result on a Windows 11 machine:
You can notice that many
DecodeData*benchmarks are slower on .NET 10 than on .NET 8. However, notablyEncodeDatais faster on .NET 101I wonder if I did something wrong or if there is really a performance regression somewhere.
Thank you!
Footnotes
I use the latest released .NET 10 build. ↩
All reactions