-
Notifications
You must be signed in to change notification settings - Fork 16
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 GC-safe region around Highs_run
#178
Conversation
Not currently, but it may do in future.
There are other calls, but we don't use them at the moment. Let me take a deeper look at this PR. I would prefer that we put the region around Line 1815 in 7316ac9
and left the automatically built libhighs.jl alone
|
Prevent a long-running `Highs_run` call from blocking GC (and freezing the other Julia threads).
e632c5f
to
459ef05
Compare
Moved the region to |
Did you find this instance because of some benchmark? I guess this might actually be a problem for most of the JuMP solvers |
Codecov ReportAll modified lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #178 +/- ##
==========================================
+ Coverage 83.31% 83.34% +0.03%
==========================================
Files 3 3
Lines 1630 1633 +3
==========================================
+ Hits 1358 1361 +3
Misses 272 272
☔ View full report in Codecov by Sentry. |
I work at RelationalAI; we're using Julia to build our product. The freeze I mention happened to us. 😢 This is likely a problem with the other solvers as well, as well as any package that wraps an external library (i.e. uses |
Can I request a new patch release with this fix? |
Ah 😄 What other solvers do you need me to look at?
Yip |
Let me get back on that one. Thanks very much! |
Here are other
Cc: @bachdavi |
I think these are all safe:
|
Prevent a long-running
Highs_run
call from blocking GC (and freezing other Julia threads). Ref: JuliaLang/julia#51574Questions:
Highs_run
call back into Julia?