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

Force Julia to specialize on ::Function arguments. #1264

Merged
merged 5 commits into from
Mar 8, 2021
Merged

Conversation

odow
Copy link
Member

@odow odow commented Mar 5, 2021

See the section in Performance Tips:
https://docs.julialang.org/en/v1/manual/performance-tips/index.html#Be-aware-of-when-Julia-avoids-specializing

This should help simplify (and improve) the precompilation files.

Benchmark

Using the script from jump-dev/JuMP.jl#2484

Before

(base) oscar@Oscars-MBP scripts % '/Applications/Julia-1.6.app/Contents/Resources/julia/bin/julia' --project=. precompile.jl
 13.389848 seconds (43.71 M allocations: 2.494 GiB, 7.98% gc time, 24.12% compilation time)

After

(base) oscar@Oscars-MBP scripts % '/Applications/Julia-1.6.app/Contents/Resources/julia/bin/julia' --project=. precompile.jl
 11.452046 seconds (37.67 M allocations: 2.103 GiB, 8.23% gc time, 26.85% compilation time)

@odow odow added Submodule: Utilities About the Utilities submodule Type: Performance labels Mar 5, 2021
@odow odow requested a review from blegat March 5, 2021 20:20
@odow
Copy link
Member Author

odow commented Mar 6, 2021

With the precompile statements

(base) oscar@Oscars-MBP scripts % '/Applications/Julia-1.6.app/Contents/Resources/julia/bin/julia' --project=. precompile.jl
  8.106619 seconds (22.41 M allocations: 1.234 GiB, 5.04% gc time, 27.18% compilation time)

@odow
Copy link
Member Author

odow commented Mar 6, 2021

There are two commits here. I can either split them in two PRs, or we can use "Rebase and merge" to add them as separate commits to master.

src/Utilities/precompile.jl Outdated Show resolved Hide resolved
src/precompile.jl Outdated Show resolved Hide resolved
@blegat
Copy link
Member

blegat commented Mar 7, 2021

If ::F, ...) where {F<:Function} is now the default unless we have a good reason to do ::Function, ..) in which case we should add a comment explaining why then we should probably add something saying this in the JuMP style guide

@odow odow mentioned this pull request Mar 7, 2021
@odow
Copy link
Member Author

odow commented Mar 7, 2021

So it's not the case that we want this as the default. The heuristic is there for a good reason: most times it is more expensive to compile the method than the added runtime benefit.

I've gone through and removed some of the cases that didn't do anything.

@odow
Copy link
Member Author

odow commented Mar 7, 2021

(base) oscar@Oscars-MBP scripts % '/Applications/Julia-1.6.app/Contents/Resources/julia/bin/julia' --project=. precompile.jl
 11.481412 seconds (37.60 M allocations: 2.098 GiB, 8.62% gc time, 26.81% compilation time)

@odow
Copy link
Member Author

odow commented Mar 8, 2021

Rebased onto master which contains the precompile statements:

(base) oscar@Oscars-MBP scripts % '/Applications/Julia-1.6.app/Contents/Resources/julia/bin/julia' --project=. precompile.jl
  8.253007 seconds (23.77 M allocations: 1.312 GiB, 5.16% gc time, 28.03% compilation time)

@odow odow merged commit efa7b88 into master Mar 8, 2021
@odow odow deleted the od/inference branch March 8, 2021 18:36
@blegat blegat added this to the v0.9.21 milestone May 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Submodule: Utilities About the Utilities submodule Type: Performance
Development

Successfully merging this pull request may close these issues.

2 participants