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

usingBenchmarkDotNet.Attributes;usingBenchmarkDotNet.Running;usingSystem.Numerics;usingSystem.Runtime.InteropServices;BenchmarkSwitcher.FromAssembly(typeof(Tests).Assembly).Run(args);[DisassemblyDiagnoser]publicclassTests{[Params(1000,10000,100000,1000000)]publicintN;strings1,s2,s3;[GlobalSetup]publicvoidSetup(){s1="1"+newstring('0',N);s2="1"+newstring('0',N-1)+"1";s3=newstring('9',N-1);}[Benchmark]publicBigIntegerPowerOfTen()=>BigInteger.Parse(s1);[Benchmark(Baseline=true)]publicBigIntegerPowerOfTenPlusOne()=>BigInteger.Parse(s2);[Benchmark]publicBigIntegerPowerOfTenMinusOne()=>BigInteger.Parse(s3);}

BenchmarkDotNet v0.13.12, Windows 11 (10.0.22631.3085/23H2/2023Update/SunValley3)13th Gen Intel Core i5-13500, 1 CPU, 20 logical and 14 physical cores.NET SDK 9.0.100-alpha.1.23615.4  [Host]     : .NET 9.0.0 (9.0.23.61410), X64 RyuJIT AVX2  Job-KEBJOQ : .NET 9.0.0 (42.42.42.42424), X64 RyuJIT AVX2  Job-VLAEBK : .NET 9.0.0 (42.42.42.42424), X64 RyuJIT AVX2

I updated the benchmark target to the latest commit.

I updated the benchmark target to the latest commit.

Thanks! Slowly getting through stuff here and just want to ensure no conflicts/accurate numbers/etc as they go through.

Looks like there are some related CI failures that need to get resolved.

Tests fail whenparseTest andToStringTest are run in parallel. I thinkRunWithFakeThreshold is the cause of failure.
I setDisableParallelization so thatRunWithFakeThreshold is not executed in parallel.

I was able to reproduce the error with the following code.

Number.s_naiveThreshold=0;BigInteger.Parse("99000000000");

@huoyaoyuan, if you have some time I'd appreciate if you could give this a once over as well, given you've also been touching/updating the parsing logic.

I'd ideally like to get#95402 done first and I know there's another PR out, but you recently changed that one back todraft

I'd ideally like to get #95402 done first and I know there's another PR out, but you recently changed that one back to draft

I've done the code update and measuring the performance. I'd expect to make it ready this weekend.

@kzrnm, sorry for the delay on this. Could you share updated numbers, just so we can confirm nothing has regressed given all the other work that's been done?

After that, I think we can get this merged.

@tannergooding I ran benchmarks on the latest branch.

BenchmarkDotNet v0.13.12, Windows 11 (10.0.22631.3810/23H2/2023Update/SunValley3)13th Gen Intel Core i5-13500, 1 CPU, 20 logical and 14 physical cores.NET SDK 9.0.100-preview.5.24307.3  [Host]   : .NET 9.0.0 (9.0.24.30607), X64 RyuJIT AVX2  ShortRun : .NET 9.0.0 (42.42.42.42424), X64 RyuJIT AVX2Job=ShortRun  IterationCount=3  LaunchCount=1  WarmupCount=3

@tannergooding Regressions:
Linux x64:dotnet/perf-autofiling-issues#37866,dotnet/perf-autofiling-issues#37802
Windows x64:dotnet/perf-autofiling-issues#37852 (BigInteger regressions),dotnet/perf-autofiling-issues#37817 (BigInteger regressions)
Linux arm64:dotnet/perf-autofiling-issues#38039

Mono related regressions:

• [Perf] Linux/arm64: 2 Regressions on 7/3/2024 6:29:56 PM perf-autofiling-issues#37939
• [Perf] Linux/x64: 3 Regressions on 7/3/2024 8:41:35 PM perf-autofiling-issues#37879
  Mono related improvements:
• [Perf] Linux/arm64: 2 Improvements on 7/3/2024 6:29:56 PM perf-autofiling-issues#37943
• [Perf] Linux/x64: 3 Improvements on 7/3/2024 8:41:35 PM perf-autofiling-issues#37884

Just noting on the regressions above that many of them are for very small inputs, like123. I've raised that these aren't "representative" of the cases we care about here since if users care about perf for numbers that are less than or equal to 128-bits, then we have existing dedicated types they can use.

Rather, we want to ensure thatBigInteger really shines for values above this cutoff. -- There is notably some consideration that we also don't want to overly regress the perf of BigInteger values with 128 or less bits, just that its not as important. For a case likeParse we could likely have a fast path that does something likeif (stringLength < 40) { ParseAsInt128 } else { ParseAsBigInt }, since any string with this few digits must fit into Int128 (and a corresponding cutoff for hex numbers, etc). This would likely fix the regression without hurting the improved cases.