#17320: Memory leaks with LP Solvers are back
-------------------------------------+-------------------------------------
Reporter: pmueller | Owner:
Type: defect | Status: new
Priority: major | Milestone: sage-6.4
Component: linear | Resolution:
programming | Merged in:
Keywords: | Reviewers:
Authors: | Work issues:
Report Upstream: N/A | Commit:
Branch: | bc20667184ca8209776d8c363df2aa3e65994287
u/nbruin/memory_leaks_with_lp_solvers_are_back| Stopgaps:
Dependencies: |
-------------------------------------+-------------------------------------
Comment (by nbruin):
Replying to [comment:9 ncohen]:
> It seems that only moving the "del" above was sufficient. I created a
branch at public/17320 that does that, tell me what you think !
It seems to me that only committing to the new model by assigning it to
`self.model` at the very end is a design feature: it means that if the
computation fails for any reason, the object `self` is unaffected and
hence still in a consistent state. It's also better for future
development, because as long as model is private, there is no lock
required to modify it. Once it's exported to `self.model`, you'd need to
lock self before `self.model` is changed. Python currently doesn't support
such multiprocessing programming anyway, but having changes in global
state as atomic as possible is generally a good feature, because it
reduces the possibility for unexpected interactions between multiple
threads (the GIL does get released every now and again ...)
With your change, the model that just experienced an error is now stored
in `self`. It could be convenient to have it available for further
analysis, so a priori it's not clear which design is more useful. It
depends on the details of what the code does and how it does it. Changing
the design just to avoid a try/finally is probably not a good idea,
however.
Furthermore, there is still this code between `model` allocation and
committing to `self.model=model`.
{{{
model.setLogLevel(self.model.logLevel())
# multithreading
import multiprocessing
model.setNumberThreads(multiprocessing.cpu_count())
model.branchAndBound()
}}}
which looks like it's quite nontrivial and hence could raise errors by
itself. You'd still have a leak then. So either guard that with a
`try/finally`, in which case you might as well stick with the guard all
the way, or move the `self.model=model` even higher.
--
Ticket URL: <http://trac.sagemath.org/ticket/17320#comment:14>
Sage <http://www.sagemath.org>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica,
and MATLAB
--
You received this message because you are subscribed to the Google Groups
"sage-trac" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
Visit this group at http://groups.google.com/group/sage-trac.
For more options, visit https://groups.google.com/d/optout.
