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

MPFR is ~2x slower than julia 1.1 #31759

Closed
KristofferC opened this issue Apr 18, 2019 · 7 comments · Fixed by #33096
Closed

MPFR is ~2x slower than julia 1.1 #31759

KristofferC opened this issue Apr 18, 2019 · 7 comments · Fixed by #33096
Assignees
Labels
bignums BigInt and BigFloat external dependencies Involves LLVM, OpenBLAS, or other linked libraries performance Must go faster regression Regression in behavior compared to a previous version
Milestone

Comments

@KristofferC
Copy link
Member

Using a commit with MPFR 4.0.2 but before the BinaryBuilder PR I get

julia> a = 5.0; b = big"5.0"
5.0

julia> @btime $a + $b;
  57.579 ns (2 allocations: 112 bytes)

while on #31727 I get

julia> @btime $a + $b;
  100.848 ns (2 allocations: 112 bytes)

My guess is thus that the BB built MPFR library is slower than the one we used to built locally. This is reported by Nanosoldier at #31727 (comment) for the 1.2 backport branch.

@KristofferC KristofferC added performance Must go faster regression Regression in behavior compared to a previous version bignums BigInt and BigFloat labels Apr 18, 2019
@KristofferC KristofferC added this to the 1.2 milestone Apr 18, 2019
@KristofferC KristofferC changed the title MPFR is ~2x slower than 1.1 MPFR is ~2x slower on julia 1.2 than julia 1.1 Apr 18, 2019
@staticfloat
Copy link
Member

I think there are two possibilities here:

  • The BB tarballs are being overly conservative in their architecture choice. Looking into what our GCC shards are configured for, they are defaulting to -march=x86-64 which is extremely conservative. The lower bound we advertise is -march=core2, so I will make sure to include that proper default in the next shard update (which I've conveniently been working on for the past few days), so once the new compiler shards are ready, we'll rebuild MPFR and see if that helps.

  • Newer versions of GCC are smarter than the 4.8.5 we're using as the default compiler. The main reason we use 4.8.5 is to make C++ compatibility as simple as possible. We do not yet have a nice, automatic way to determine that a binary dependency does not rely upon libstdc++ and auto-build it with GCC8, but we should. In the meantime, we could try rebuilding MPFR with GCC8, and then see if this improves at all.

@ararslan ararslan added the external dependencies Involves LLVM, OpenBLAS, or other linked libraries label Apr 23, 2019
@KristofferC
Copy link
Member Author

KristofferC commented May 6, 2019

Bump. This is release blocking AFAIU so would be nice to have some progress / update here. Should we go back to just building MPFR from source?

@staticfloat
Copy link
Member

It took me almost three weeks to iron out issues, but I finally got my new BB shards building, so we can take a look at this a little more closely; Compiling with a better default -march flag actually gives us a pretty decent speedup:

Original 1.1 performance (compiled with GCC 7.1):

julia> using BenchmarkTools
       a = 5.0
       b = big"5.0"
       @btime $a + $b;
  71.648 ns (2 allocations: 112 bytes)

Current master performance (compiled with GCC 4.8.5, -march=x86-64)

julia> using BenchmarkTools
       a = 5.0
       b = big"5.0"
       @btime $a + $b;
  183.323 ns (2 allocations: 112 bytes)

My branch performance (compiled with GCC 4.8.5, -march=core2 on x86_64):

julia> using BenchmarkTools
       a = 5.0
       b = big"5.0"
       @btime $a + $b;
  113.536 ns (2 allocations: 112 bytes)

Compiling with GCC 8 doesn't change anything, so I think the second bullet point is irrelevant. I'm still investigating the rest of the performance issues; I will try rebuilding GMP with the new BB shards to see if that is the cause of the remainder of the slowdown.

@KristofferC KristofferC mentioned this issue May 20, 2019
58 tasks
@KristofferC KristofferC modified the milestones: 1.2, 1.3 May 23, 2019
@KristofferC
Copy link
Member Author

1.2 got changed to use source builds.

@StefanKarpinski
Copy link
Member

(For MPFR and GMP only, just to mitigate the performance regression.)

@JeffBezanson JeffBezanson modified the milestones: 1.3, 1.4 Aug 15, 2019
@KristofferC
Copy link
Member Author

Why was the milestone here changed? From my memory, changing to source builds was only on 1.2 backport branch so this seems to be still in effect. Changing back milestone until it has been confirmed.

@KristofferC KristofferC modified the milestones: 1.4, 1.3 Aug 17, 2019
@KristofferC KristofferC changed the title MPFR is ~2x slower on julia 1.2 than julia 1.1 MPFR is ~2x slower than julia 1.1 Aug 19, 2019
@KristofferC KristofferC modified the milestones: 1.3, 1.4 Aug 26, 2019
@KristofferC
Copy link
Member Author

Moved milestone since I backported the disable BB PR to the 1.3-rc branch. If we get it fixed for 1.3 then great, but no need for the milestone now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bignums BigInt and BigFloat external dependencies Involves LLVM, OpenBLAS, or other linked libraries performance Must go faster regression Regression in behavior compared to a previous version
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants