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

Add SnoopPrecompile workload #3193

Merged
merged 2 commits into from
Jan 22, 2023
Merged

Add SnoopPrecompile workload #3193

merged 2 commits into from
Jan 22, 2023

Conversation

timholy
Copy link
Contributor

@timholy timholy commented Jan 19, 2023

On my machine, this cuts the TTFX of a simple workload (using the
GLPK solver, which is not used here) from about 8.5s to about 3s.

The improvement was measured on Julia master; there may be some (much smaller) improvement for Julia 1.8 as well, but I haven't tested. The workload is the one in jump-dev/MathOptInterface.jl#2226, identical to the one in the @precompile_all_calls block except using GLPK as the solver.

On my machine, this cuts the TTFX of a simple workload (using the
GLPK solver, which is not used here) from about 8.5s to about 3s.
@timholy
Copy link
Contributor Author

timholy commented Jan 19, 2023

I'm not certain what you want to do about your current _precompile_ infrastructure. Perhaps the best option might be to expand this simple workload to accommodate those cases?

@codecov
Copy link

codecov bot commented Jan 20, 2023

Codecov Report

Base: 98.06% // Head: 98.06% // Increases project coverage by +0.00% 🎉

Coverage data is based on head (361c9b6) compared to base (a978323).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    jump-dev/JuMP.jl#3193   +/-   ##
=======================================
  Coverage   98.06%   98.06%           
=======================================
  Files          33       33           
  Lines        4598     4600    +2     
=======================================
+ Hits         4509     4511    +2     
  Misses         89       89           
Impacted Files Coverage Δ
src/JuMP.jl 96.82% <100.00%> (+0.05%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@timholy
Copy link
Contributor Author

timholy commented Jan 20, 2023

Perhaps the best option might be to expand this simple workload to accommodate those cases?

To clarify, I'm not familiar with what one needs to do to set up a toy problem with, e.g., a MOI.RotatedSecondOrderCone constraint:

MOI.RotatedSecondOrderCone,

My "showcase" for Julia 1.9 will not need breadth of coverage, but ideally someone who knows JuMP will eventually expand this to cover more cases and force "better" precompilation of all the things covered in the current precompile.jl infrastructure. At which point you could remove precompile.jl entirely.

@timholy
Copy link
Contributor Author

timholy commented Jan 21, 2023

I'm not aware of anything wrong with merging this. The other things are "nice to have" but someone could tackle those later.

Copy link
Member

@odow odow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can add this approach to the solver packages, using only the MOI interface. It'll still miss some of the JuMP -> Solver methods, but it should give a nice reduction. I'll have a play.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants