Skip to content
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

Replace DoubleUtil.IsNaN with double.IsNaN #4338

Closed
omariom opened this issue Mar 28, 2021 · 1 comment
Closed

Replace DoubleUtil.IsNaN with double.IsNaN #4338

omariom opened this issue Mar 28, 2021 · 1 comment
Assignees
Labels
Enhancement Requested Product code improvement that does NOT require public API changes/additions Performance Performance related issue

Comments

@omariom
Copy link
Contributor

omariom commented Mar 28, 2021

BCL's double.IsNaN got faster long ago. In .NET Core it is a single instruction.

The usage of DoubleUti.IsNaN should be replaced by double.IsNaN.

Codegen difference

Benchmarks

    [SimpleJob(RuntimeMoniker.Net48),
     //SimpleJob(RuntimeMoniker.NetCoreApp31),
     SimpleJob(RuntimeMoniker.NetCoreApp50),
    ]
    [DisassemblyDiagnoser]
    public class IsNaNBenchmarks
    {
        [Benchmark(Baseline = true)]
        [Arguments(123.45)]
        [Arguments(double.NaN)]
        public bool Clr(double value) => double.IsNaN(value);

        [Benchmark()]
        [Arguments(123.45)]
        [Arguments(double.NaN)]
        public bool Wpf(double value) => IsNaN(value);

        public static bool IsNaN(double value)
        {
            NanUnion t = new NanUnion();
            t.DoubleValue = value;

            ulong exp = t.UintValue & 0xfff0000000000000;
            ulong man = t.UintValue & 0x000fffffffffffff;

            return (exp == 0x7ff0000000000000 || exp == 0xfff0000000000000) && (man != 0);
        }

        [StructLayout(LayoutKind.Explicit)]
        private struct NanUnion
        {
            [FieldOffset(0)]
            internal double DoubleValue;
            [FieldOffset(0)]
            internal ulong UintValue;
        }
    }

Results:

BenchmarkDotNet=v0.12.1, OS=Windows 10.0.19042
Intel Core i7-8809G CPU 3.10GHz (Kaby Lake G), 1 CPU, 8 logical and 4 physical cores
.NET Core SDK=6.0.100-preview.2.21155.3
[Host] : .NET Core 5.0.4 (CoreCLR 5.0.421.11614, CoreFX 5.0.421.11614), X64 RyuJIT
.NET 4.8 : .NET Framework 4.8 (4.8.4341.0), X64 RyuJIT
.NET Core 5.0 : .NET Core 5.0.4 (CoreCLR 5.0.421.11614, CoreFX 5.0.421.11614), X64 RyuJIT

Method Job Runtime value Mean Error StdDev Median Ratio RatioSD Code Size
Clr .NET 4.8 .NET 4.8 NaN 1.4431 ns 0.3717 ns 1.0903 ns 0.7239 ns 1.00 0.00 67 B
Wpf .NET 4.8 .NET 4.8 NaN 1.5641 ns 0.0412 ns 0.0365 ns 1.5708 ns 1.63 0.73 116 B
Clr .NET Core 5.0 .NET Core 5.0 NaN 0.0281 ns 0.0278 ns 0.0247 ns 0.0231 ns ? ? 14 B
Wpf .NET Core 5.0 .NET Core 5.0 NaN 1.5333 ns 0.0370 ns 0.0328 ns 1.5350 ns ? ? 106 B
Clr .NET 4.8 .NET 4.8 123.45 0.6543 ns 0.0348 ns 0.0308 ns 0.6531 ns 1.00 0.00 67 B
Wpf .NET 4.8 .NET 4.8 123.45 1.8958 ns 0.0343 ns 0.0287 ns 1.9000 ns 2.91 0.14 116 B
Clr .NET Core 5.0 .NET Core 5.0 123.45 0.0909 ns 0.0276 ns 0.0258 ns 0.0981 ns 1.00 0.00 14 B
Wpf .NET Core 5.0 .NET Core 5.0 123.45 1.4530 ns 0.0390 ns 0.0346 ns 1.4525 ns 17.92 6.38 106 B
@ryalanms ryalanms added Enhancement Requested Product code improvement that does NOT require public API changes/additions Performance Performance related issue and removed Untriaged labels Mar 29, 2021
@ryalanms ryalanms added this to the 6.0.0 milestone Mar 29, 2021
@dipeshmsft
Copy link
Member

Closing issue as the corresponding PR has been merged.

@ghost ghost removed this from the 6.0.0 milestone Oct 24, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Nov 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Enhancement Requested Product code improvement that does NOT require public API changes/additions Performance Performance related issue
Projects
None yet
Development

No branches or pull requests

4 participants