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

Investigate Performance of Math Functions in .NET Standard 2.1+ #297

Closed
NightOwl888 opened this issue Jun 17, 2020 · 2 comments
Closed

Investigate Performance of Math Functions in .NET Standard 2.1+ #297

NightOwl888 opened this issue Jun 17, 2020 · 2 comments
Labels
good-first-issue Good for newcomers help-wanted Extra attention is needed is:enhancement New feature or request pri:normal up-for-grabs This issue is open to be worked on by anyone
Milestone

Comments

@NightOwl888
Copy link
Contributor

As part of our effort to improve performance (#261), Microsoft has added support for some of the functions we use in MathUtil.

    // Hyperbolic Functions
    public static double Acosh(double);                 // Compute area hyperbolic cosine
    public static double Asinh(double);                 // Compute area hyperbolic sine
    public static double Atanh(double);                 // Compute area hyperbolic tangent

    public static double Log(double, double);

We should run benchmarks to see if the built-in implementation is faster than ours, and if so reference the built-in Math class inside of MathUtil in .NET Standard 2.1+ as well as consider the same optimizations to MathUtil for other target frameworks.

It also looks like public static int Log(long, long) may be able to be optimized.

@NightOwl888 NightOwl888 added up-for-grabs This issue is open to be worked on by anyone help-wanted Extra attention is needed good-first-issue Good for newcomers is:enhancement New feature or request pri:normal labels Jun 17, 2020
@NightOwl888 NightOwl888 added this to the 4.8.0 milestone Jun 17, 2020
@jeme
Copy link
Contributor

jeme commented Sep 16, 2020

I threw a very quick-n-dirty benchmark together just to get an early idea.

BenchmarkDotNet=v0.12.1, OS=Windows 10.0.18363.1016 (1909/November2018Update/19H2)
Intel Core i7-6700 CPU 3.40GHz (Skylake), 1 CPU, 8 logical and 4 physical cores
.NET Core SDK=5.0.100-preview.7.20366.15
  [Host]     : .NET Core 3.1.6 (CoreCLR 4.700.20.26901, CoreFX 4.700.20.31603), X64 RyuJIT
  DefaultJob : .NET Core 3.1.6 (CoreCLR 4.700.20.26901, CoreFX 4.700.20.31603), X64 RyuJIT

Method Mean Error StdDev
MathUtil_Acosh 115.0 μs 2.26 μs 3.58 μs
Math_Acosh 243.6 μs 4.74 μs 6.94 μs
MathUtil_Asinh 111.5 μs 1.55 μs 1.45 μs
Math_Asinh 306.2 μs 3.28 μs 2.91 μs
MathUtil_Log 139.7 μs 2.79 μs 4.42 μs
Math_Log 180.5 μs 2.39 μs 1.99 μs
MathUtil_Atanh 649.9 μs 10.32 μs 8.06 μs
Math_Atanh 519.9 μs 10.33 μs 11.48 μs

There is quite a few things to sort out, like platformes etc. but just wanted to add the early indications.
This is the numbers for running the computation over 10.000 randomly selected doubles from 0 to ~int.max...

@NightOwl888
Copy link
Contributor Author

@jeme

Thanks for the info. These results are similar to the benchmark that I did earlier. I find the results a bit surprising since Microsoft implemented these functions in C++, but apparently ours are more efficient except for Atanh.

When I posted this task, I was working under the assumption that these methods were being used by other Lucene.NET components, thus having a potential impact on performance. But according to code lens, the only methods that are referenced (other than in tests) are Log(long, int) and Gcd(long, long), neither of which have a counterpart in .NET.

Gcd(long, long) is the only one that actually has an impact in the latest codecs and it has already been modified to use J2N's version of TrailingZeroCount() which will call the hardware intrinsic method on .NET Core 3.x and Windows if we add a .NET Core 3.x target framework to Lucene.NET.

That being said, since there doesn't appear to be anything impactful we can improve on MathUtil itself, we should focus our optimization efforts elsewhere.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good-first-issue Good for newcomers help-wanted Extra attention is needed is:enhancement New feature or request pri:normal up-for-grabs This issue is open to be worked on by anyone
Projects
None yet
Development

No branches or pull requests

2 participants