Make Number.FormatBigInteger JIT-firendly #113165
Conversation
dotnet-policy-service
bot
added
the
community-contribution
|
Let's first check if that was the issue: |
|
@EgorBot -amd -arm -windows_intel using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Running;
using System.Numerics;
BenchmarkSwitcher.FromAssembly(typeof(Program).Assembly).Run(args);
[MemoryDiagnoser(false)]
[HideColumns("Job", "Error", "StdDev", "Median", "RatioSD")]
public class Tests
{
private char[] _dest = new char[1_000_000];
private BigInteger _value =
BigInteger.Pow(BigInteger.Parse(string.Concat(Enumerable.Repeat("1234567890", 30))), 1_000);
[Benchmark(Baseline = true)]
public string Stringify() => _value.ToString();
[Benchmark]
public bool TryFormat() => _value.TryFormat(_dest, out _);
} |
src/libraries/System.Runtime.Numerics/src/System/Number.BigInteger.cs
Outdated
Show resolved
Hide resolved
|
@kzrnm While my bot is likely broken for Linux atm (fixing it now), the Windows-x64 job couldn't confirm your theory - EgorBot/runtime-utils#310 (comment) the numbers for before/after are the same. |
|
When running build.cmd/build.sh, crossgen is properly applied to only Since I wasn't sure how dotnet/runtime handles publishing, I ran The When inspecting the latter with ILSpy, it displays "This assembly contains unmanaged code." |
|
Ah, I didn't realize BigInteger is not in corelib (although, there is an internal copy there). Anyway, it doesn't sound like a correct solution to me. AO bypasses all Tier1 optimizations including Dynamic PGO. Today the only method in BCL that has it is |
|
@EgorBo I don't think the fact that AO isn't used elsewhere is a valid reason not to use it for BigInteger. For example, SpanHelpers can be useful for lightweight applications, making R2R a suitable choice. However, BigInteger is purely a numerical computation type, and users are likely to prioritize faster calculations over a slight improvement in application startup time. Additionally, the regression is occurring because Dynamic PGO and OSR are not being applied, causing execution to remain in Tier-0. While this might be an issue caused by JIT, considering the use case of
Are you referring to |
AO is a workaround, there must be a valid, well explained reason for it to apply. E.g in case of Do we have an issue explaining the exact cause here?
It should be eventually applied if it's hot enough - if it does not, it's a serious bug we need to investigate. If you need performance right at the start, you should consider disabling TieredCompilation completely or switch to NativeAOT. Generally, AO is expected to decrease the steady state performance - various "is type initialized" checks are typically not elided and no PGO is applied. |
kzrnm
force-pushed
the
BigIntegerRegression111708
branch
from
c769ae3 to
96b47bc
Compare
|
Further testing has yielded even better results. By extracting the inner loop into a separate method, the code achieves higher performance without needing AO.
Benchmark code |
Maybe that is the reason? the fact that you're measuring a short run and it doesn't properly promote to the final tier? |
|
Hmm, when measured with the Default job, the main version with 100,000 digits became as fast as the AO version. However, with 1,000,000 digits, the execution time was too long for proper warm-up to take place. Moreover, suggesting that "for use cases where a single execution of a massive computation takes several seconds, TiredCompilation should be disabled" is an extremely niche piece of knowledge. It is desirable for the program to perform efficiently even when processing a large input only once.
|
kzrnm
changed the title
|
I noticed that Tier1 performance has regressed. It looks like a different approach might be needed, so I'll go ahead and close this for now.
using BenchmarkDotNet.Attributes;
using System.Numerics;
[DisassemblyDiagnoser]
[MemoryDiagnoser(false)]
[HideColumns("Job", "Error", "StdDev", "Median", "RatioSD")]
public class ToStringTest
{
char[] _dest = new char[1000010];
[Params([
// 100,
// 1000,
// 10000,
100000,
// 1000000,
])]
public int N;
BigInteger b, s;
[GlobalSetup]
public void Setup()
{
s = BigInteger.Parse(new string('9', 1 << 8));
b = BigInteger.Parse(new string('9', N));
}
[Benchmark] public void DecimalString() => b.TryFormat(_dest, out _);
[Benchmark]
public void DecimalString2()
{
for (int i = 0; i < 10000; i++)
{
s.TryFormat(_dest, out _);
}
b.TryFormat(_dest, out _);
}
} |
|
@EgorBo
using BenchmarkDotNet.Attributes;
using System.Numerics;
[DisassemblyDiagnoser]
[MemoryDiagnoser(false)]
[HideColumns("Job", "Error", "StdDev", "Median", "RatioSD")]
public class ToStringTest
{
char[] _dest = new char[1000010];
[Params([
100000,
])]
public int N;
BigInteger b, s;
[GlobalSetup]
public void Setup()
{
s = BigInteger.Parse(new string('9', 1 << 8));
b = BigInteger.Parse(new string('9', N));
}
[Benchmark] public void DecimalStringTier0() => b.TryFormat(_dest, out _);
[Benchmark]
public void DecimalStringTier1()
{
for (int i = 0; i < 10000; i++)
{
s.TryFormat(_dest, out _);
}
b.TryFormat(_dest, out _);
}
} |
Fixes #111708
After
MemoryMarshalwas moved fromSystem.MemorytoSystem.Runtimein #106360, crossgen is properly applied toNumber.FormatBigInteger. However, the loop logic inFormatBigIntegeris significantly slow in tier-0. For more information #111708 (comment)Discussions about applying AO have taken place in issues like #71261, #79645, #109087 (comment), etc., but these were mainly about frequently called methods like SpanHelpers. Given this, I don't think applying AO to BigInteger would cause any startup performance issues.