-
Notifications
You must be signed in to change notification settings - Fork 5.1k
Use generic math for floating point formatting #102683
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
Conversation
Tagging subscribers to this area: @dotnet/area-system-numerics |
Can you do some performance testing on Mono? The general process for doing so is documented here: https://github.com/dotnet/performance/blob/main/docs/benchmarking-workflow-dotnet-runtime.md CC. @SamMonoRT |
adding @steveisok and @vitek-karas for Mono scenario followup. |
@fanyang-mono can you please help with this? |
@EgorBot -arm64 -intel -mono using System.Collections.Generic;
using System.Linq;
using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Running;
namespace BenchmarkGround
{
public class Program
{
public IEnumerable<object> Values => [0.0, 1.0, 1234.5678, 1.09876543211234567890e30];
public IEnumerable<object> Formats => ["G", "F0", "F10", "E"];
public IEnumerable<object[]> ValueAndFormats => Values.SelectMany(v => Formats.Select(f => new object[] { v, f }));
[Benchmark]
[ArgumentsSource(nameof(Values))]
public string DefaultToString(double value) => value.ToString();
[Benchmark]
[ArgumentsSource(nameof(ValueAndFormats))]
public string ToStringFormat(double value, string format) => value.ToString(format);
}
} |
Benchmark results on Intel
|
Benchmark results on Arm64
|
@EgorBot -intel -mono --envvars MONO_ENV_OPTIONS:"--interpreter" using System.Collections.Generic;
using System.Linq;
using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Running;
namespace BenchmarkGround
{
public class Program
{
public IEnumerable<object> Values => [0.0, 1.0, 1234.5678, 1.09876543211234567890e30];
public IEnumerable<object> Formats => ["G", "F0", "F10", "E"];
public IEnumerable<object[]> ValueAndFormats => Values.SelectMany(v => Formats.Select(f => new object[] { v, f }));
[Benchmark]
[ArgumentsSource(nameof(Values))]
public string DefaultToString(double value) => value.ToString();
[Benchmark]
[ArgumentsSource(nameof(ValueAndFormats))]
public string ToStringFormat(double value, string format) => value.ToString(format);
}
} |
Benchmark results on Intel
|
Yeah looks like Mono-JIT and Mono interp are fine with the changes in this PR, at least for the provided benchmark. Unfortunately the bot doesn't yet support LLVM-AOT to check that as well. |
There are instructions here to run microbenchmarks with Mono AOT. (https://github.com/dotnet/performance/blob/main/docs/benchmarking-workflow-dotnet-runtime.md#run-the-benchmarks-with-aot) Please test it before merge. Usually, AOT takes the most hit from library changes. |
The instruction I used for benchmark:
Results: PR:
main:
Looks no significant regression. |
I can get this merged after the Mono team signs off and n the perf numbers, just want to make sure we’re not missing any important scenarios |
@fanyang-mono, are there any other areas you want to see tested or does this look good for the Mono side? |
These tests should be sufficient for mono. :) |
@@ -354,33 +354,33 @@ public override int GetHashCode() | |||
|
|||
public override string ToString() | |||
{ | |||
return Number.FormatDouble(m_value, null, NumberFormatInfo.CurrentInfo); | |||
return Number.FormatFloat(m_value, null, NumberFormatInfo.CurrentInfo); |
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.
Not going to block on this, but we should probably call this FormatFloatingPoint
instead and it would be good to fix it in a follow up PR.
Float
is going to be confused with the C# keyword float
, which should be called Single
in the BCL API surface area
private static ulong ExtractFractionAndBiasedExponent<TNumber>(TNumber value, out int exponent) | ||
where TNumber : unmanaged, IBinaryFloatParseAndFormatInfo<TNumber> | ||
{ | ||
ulong bits = TNumber.FloatToBits(value); |
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.
Similar here, this should probably be FloatingPointToBits
Extracted from #98643