Regressions in System.Numerics.Tests.Perf_BigInteger

[performanceautofiler]

Run Information

Architecture	x64
OS	Windows 10.0.19042
Baseline	6c09f34947026ed988fd3013fdaa624a5fab0f26
Compare	9dcde5f8a182a9202a50afa92fec463f6654ecd9
Diff	Diff

Regressions in System.Numerics.Tests.Perf_BigInteger

Benchmark	Baseline	Test	Test/Base	Test Quality	Edge Detector	Baseline IR	Compare IR	IR Ratio	Baseline ETL	Compare ETL
Subtract - Duration of single invocation	49.13 ns	72.31 ns	1.47	0.07	True

Test Report

Repro

git clone https://github.com/dotnet/performance.git
py .\performance\scripts\benchmarks_ci.py -f net6.0 --filter 'System.Numerics.Tests.Perf_BigInteger*'

Payloads

Baseline
Compare

Histogram

System.Numerics.Tests.Perf_BigInteger.Subtract(arguments: 1024,1024 bits)

Docs

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

[ghost]

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

Issue Details

---

Run Information

Architecture	x64
OS	Windows 10.0.19042
Baseline	6c09f34947026ed988fd3013fdaa624a5fab0f26
Compare	9dcde5f8a182a9202a50afa92fec463f6654ecd9
Diff	Diff

Regressions in System.Numerics.Tests.Perf_BigInteger

Benchmark	Baseline	Test	Test/Base	Test Quality	Edge Detector	Baseline IR	Compare IR	IR Ratio	Baseline ETL	Compare ETL
Subtract - Duration of single invocation	49.13 ns	72.31 ns	1.47	0.07	True

Test Report

Repro

git clone https://github.com/dotnet/performance.git
py .\performance\scripts\benchmarks_ci.py -f net6.0 --filter 'System.Numerics.Tests.Perf_BigInteger*'

Payloads

Baseline
Compare

Histogram

System.Numerics.Tests.Perf_BigInteger.Subtract(arguments: 1024,1024 bits)

Docs

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

Author:	performanceautofiler[bot]
Assignees:	-
Labels:	area-System.Numerics, untriaged
Milestone:	-

[EgorBo]

Likely regressed by #35565 (cc @sakno)

[sakno]

Correct, the regression caused by PR. Situation was previously highlighted/predicted during the discussion with reviewers. Probably, the root cause is a ArrayPool<T>.Rent behavior which is slower than array allocation using new operator.

/cc @GrabYourPitchforks

[jeffhandley]

We might end up deciding that this is an acceptable trade-off from the changes made, as several other scenarios were improved in exchange. We'll leave the issue open for the time-being though and if time-permits, we'll consider an investigation into the ArrayPool<T>.Rent cost incurred.

[kunalspathak]

arm64 regression: dotnet/perf-autofiling-issues#1816

[adamsitnik]

The benchmark has regressed also for other arguments:

[jeffhandley]

@sakno and @kunalspathak -- do we need to investigate this any further, or are we OK closing this as expected?

[sakno]

@jeffhandley , regression is expected for all operands which are greater than 512 bits (64 bytes) in size. The current stackalloc threshold is 64 bytes. Large operands require memory pooling using ArrayPool<byte>.Shared. Note that any changes in TlsOverPerCoreLockedStacksArrayPool class have impact on BigInteger performance. Currently, TlsOverPerCoreLockedStacksArrayPool.Rent + Span<byte>.Clear are slower than new byte[size].

As for me, this is acceptable. However, we can consider NativeMemory.Alloc method (which is equivalent to malloc in C) as alternative. Anyway, we have strong control over buffer lifetime inside of BigInteger methods. But this suggestion requires confirmation and discussion.

[jeffhandley]

Thanks, @sakno. Closing as expected/accepted.