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

Update GMP and MPFR builds to use better compiler flags #33096

Merged
merged 1 commit into from
Oct 3, 2019

Conversation

staticfloat
Copy link
Member

@staticfloat staticfloat commented Aug 27, 2019

This incorporates GMP and MPFR, built using the new BB, that should have better performance attributes (GCC no longer defaulting to ancient processor variants), getting us back our bigfloat performance.

Closes #31759

Eligible for backporting wherever we think it might be helpful (1.3 for sure, 1.2 possibly?)

On my linux64 machine, comparing three builds of julia:

  • julia-release-1.2: builds GMP/MPFR from-source to dodge this issue
  • julia-master: triggers the slowdown
  • ./julia: contains this fix
$ cat test.jl
using BenchmarkTools

a = 5.0
b = big"5.0"
@btime $a + $b

$ for JULIA in julia-release-1.2 julia-master ./julia; do echo $JULIA; $JULIA test.jl; done
julia-release-1.2
  103.815 ns (2 allocations: 112 bytes)
julia-master
  190.769 ns (2 allocations: 112 bytes)
./julia
  107.202 ns (2 allocations: 112 bytes)

@KristofferC
Copy link
Member

Note, should revert #32102 on backport branch when backporting this.

@KristofferC KristofferC mentioned this pull request Sep 3, 2019
36 tasks
@KristofferC
Copy link
Member

@nanosoldier runbenchmarks(ALL, vs = ":master")

@nanosoldier
Copy link
Collaborator

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @ararslan

@KristofferC
Copy link
Member

Looks like the error on mac is real:

error during bootstrap:
LoadError("sysimg.jl", 3, LoadError("Base.jl", 307, LoadError("gmp.jl", 27, ErrorException("error compiling version: could not load library \"libgmp\"\ndlopen(libgmp.dylib, 1): Symbol not found: __gmpn_invert_limb_table\n  Referenced from: /Users/julia/buildbot/worker/package_macos64/build/usr/lib/libgmp.dylib\n  Expected in: flat namespace\n in /Users/julia/buildbot/worker/package_macos64/build/usr/lib/libgmp.dylib"))))
rec_backtrace at /Users/julia/buildbot/worker/package_macos64/build/src/stackwalk.c:94
record_backtrace at /Users/julia/buildbot/worker/package_macos64/build/src/task.c:224
jl_throw at /Users/julia/buildbot/worker/package_macos64/build/src/task.c:461
jl_rethrow_with_add at /Users/julia/buildbot/worker/package_macos64/build/src/codegen.cpp:791
jl_compile_linfo at /Users/julia/buildbot/worker/package_macos64/build/src/codegen.cpp:1205
jl_compile_method_internal at /Users/julia/buildbot/worker/package_macos64/build/src/gf.c:1879
_jl_invoke at /Users/julia/buildbot/worker/package_macos64/build/src/gf.c:2135 [inlined]
jl_apply_generic at /Users/julia/buildbot/worker/package_macos64/build/src/gf.c:2300
jl_apply at /Users/julia/buildbot/worker/package_macos64/build/src/./julia.h:1631 [inlined]
do_call at /Users/julia/buildbot/worker/package_macos64/build/src/interpreter.c:328
eval_stmt_value at /Users/julia/buildbot/worker/package_macos64/build/src/interpreter.c:368 [inlined]
eval_body at /Users/julia/buildbot/worker/package_macos64/build/src/interpreter.c:764
jl_interpret_toplevel_thunk_callback at /Users/julia/buildbot/worker/package_macos64/build/src/interpreter.c:888
Interpreter frame (ip: 1)
Core.CodeInfo(code=Array{Any, (4,)}[

@staticfloat

@staticfloat
Copy link
Member Author

So my investigation of this so far points to this being due to compiling with clang-8. I'm having a hard time figuring out where the incompatibility is coming from (since the symbol name seems to be identical across versions) my only hint so far is that the old libgmp.dylib (built with Apple clang-1001.0.46.4) gives only one symbol mapping for __gmpn_invert_limb_table:

$ nm libgmp.dylib | grep gmpn_invert_limb_table
000000000005ad98 s __gmpn_invert_limb_table

Whereas the new one (built with clang-8) gives two:

$ nm libgmp.dylib | grep gmpn_invert_limb_table
                 U __gmpn_invert_limb_table
000000000007ec40 s __gmpn_invert_limb_table

I do not yet grok the import of having that extra undefined symbol, but I can only guess that it's due to some internal compiler difference.

@KristofferC KristofferC mentioned this pull request Sep 11, 2019
18 tasks
@staticfloat
Copy link
Member Author

I believe, in my furious binary building for Gtk, I may have nudged around compiler options to the point that we no longer get double symbols. I've rebased this, let's see if it is indeed the case.

@ararslan
Copy link
Member

ararslan commented Oct 2, 2019

Just for good measure: @nanosoldier runbenchmarks(ALL, vs=":master")

@nanosoldier
Copy link
Collaborator

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @ararslan

@KristofferC
Copy link
Member

Nice!

@staticfloat
Copy link
Member Author

BigFloat speedups across the board!

@staticfloat staticfloat merged commit b685cf6 into master Oct 3, 2019
@staticfloat staticfloat deleted the sf/new_gmp_mpfr branch October 3, 2019 08:35
KristofferC pushed a commit that referenced this pull request Oct 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MPFR is ~2x slower than julia 1.1
4 participants