-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Optimize threshold of BigInteger.Parse #97101
Conversation
dfb63a9
to
314d139
Compare
Tagging subscribers to this area: @dotnet/area-system-numerics Issue Details
Benchmark result
Benchmark code
using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Running;
using System.Numerics;
using System.Runtime.InteropServices;
BenchmarkSwitcher.FromAssembly(typeof(Tests).Assembly).Run(args);
[DisassemblyDiagnoser]
public class Tests
{
public record Data(int Length)
{
public string Text = new string('9', Length);
public override string ToString() => $"{Length}";
}
public IEnumerable<object> NumberStrings()
{
yield return new Data(10);
yield return new Data(200);
yield return new Data(500);
yield return new Data(1000);
yield return new Data(2000);
yield return new Data(3000);
yield return new Data(3100);
yield return new Data(3190);
yield return new Data(3200);
yield return new Data(3210);
yield return new Data(3300);
yield return new Data(4000);
yield return new Data(5000);
yield return new Data(7500);
yield return new Data(10000);
yield return new Data(15000);
yield return new Data(20000);
yield return new Data(21000);
yield return new Data(25000);
yield return new Data(50000);
yield return new Data(100000);
}
[Benchmark]
[ArgumentsSource(nameof(NumberStrings))]
public BigInteger Parse(Data data) => BigInteger.Parse(data.Text);
}
|
@@ -668,7 +668,7 @@ internal static | |||
#else | |||
internal const | |||
#endif | |||
int s_naiveThreshold = 20000; | |||
int s_naiveThreshold = 3200; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is ~10627 bits
. The other thresholds you measured were 200
(~661 bits
) and 20000
(~66436 bits
).
Could you also measure:
308
(~1024 bits
)- This is roughly enough to fit any integral
double
(64-bit binary float
)
- This is roughly enough to fit any integral
616
(~2048 bits
)1233
(~4096 bits
)- This is approx the upper bound of most RSA key lengths
2466
(~8192 bits
)4932
(~16384 bits
)- This is roughly enough to fit any integral
quad
(128-bit binary float
)
- This is roughly enough to fit any integral
This likely needs measurement across a few other machines as well. |
I added some test cases. Benchmark result
|
Looking at this, the numbers are all fairly close. We will want to measure on some other hardware as well, but I think that Either covers the majority of most common inputs and allows for the less naive algorithm to be used for large/uncommon inputs. |
314d139
to
dad0ef2
Compare
Thanks for the contribution! Going to merge this and wait for some more numbers in the official perf benchmarks. The numbers looked roughly similar to what you gave above on my Zen4 based box and this is a trivial 1-liner we can revert if needed. |
BigInteger.Parse(new string('9', n))
is slow when n is less than or equal to 20000. I have adjusted the value ofNumber.s_naiveThreshold
. It seems optimal to sets_naiveThreshold
to ~3200.Benchmark result
Benchmark code
Related
#55121 e93dde5