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

Memory leaks #131

Closed
odow opened this issue Jun 16, 2021 · 23 comments
Closed

Memory leaks #131

odow opened this issue Jun 16, 2021 · 23 comments

Comments

@odow
Copy link
Member

odow commented Jun 16, 2021

I took a brief look into @NLaws discourse issue https://discourse.julialang.org/t/memory-consumption-growth-with-many-large-milps-in-jump/61895/52. I don't have an Xpress license, so I didn't run any code. I just skimmed the finalize stuff.

This is concerning. What is free? Does it need to free the license?

Xpress.jl/src/Xpress.jl

Lines 50 to 56 in 6f95f15

init() # Call XPRSinit for initialization
# free is not strictly necessary since destroyprob is called
# inthe finalizer.
# the user can call free if needed.
# leaving it uncommented results in many print errors becaus
# it is called prior to finalizers.
# atexit(free) # Call free when process terminates

Is destroyprob sufficient to free all memory allocated by Xpress?

Xpress.jl/src/helper.jl

Lines 53 to 63 in f15573f

mutable struct XpressProblem <: CWrapper
ptr::Lib.XPRSprob
logfile::String
function XpressProblem(ptr::Lib.XPRSprob; finalize_env::Bool = true, logfile = "")
model = new(ptr, logfile)
if finalize_env
finalizer(destroyprob, model)
end
return model
end
end

What happens if destroyprob is called twice? Does it error? That would prevent someone manually calling finalize.

Maybe you need something like

finalizer(model) do m
    if m.ptr != C_NULL
        destroyprob(m)
        m.ptr = C_NULl
    end
    return
end
@joaquimg
Copy link
Member

joaquimg commented Jun 16, 2021

I don't have an Xpress license, so I didn't run any code. I just skimmed the finalize stuff.

You can get something here:

https://content.fico.com/xpress-optimization-community-license?utm_source=FICO-Community&utm_medium=xpress-optimization-licensing

What is free? Does it need to free the license?

Free releases the license. I thought it only did that, but docs says it free memory!!!!

https://www.fico.com/fico-xpress-optimization/docs/latest/solver/optimizer/HTML/XPRSfree.html

Not sure about the usage though.

From what I understand, free will kill the env. And I understand that the env is a singleton. so you cant have multiple.
So if we put free in the finalizer and you have a second model loaded, finalizing one model will kill both.

If that is the case we could have:

function Xpress.reset()
    Xpress.free()
    Xpress.init()
end

And the caller must be careful to not have any active instance.

finalizer suggestion

Seems reasonable, but I just ran thousands of problems and called finalize and no issue with that. The extra code does not hurt though.

@odow
Copy link
Member Author

odow commented Jun 16, 2021

Okay so there may be memory persisting between solves, that survive destroyprob? @NLaws should ask FICO support.

Seems reasonable, but I just ran thousands of problems and called finalize and no issue with that.

Yes, I think the solution is:

function run_reopt(inputs)
    model = direct_model(Xpress.Optimizer())
    # ... do stuff
    results = get_results(model)
    xpress = backend(model)
    finalize(xpress)
    return results
end

You can get something here:

The related problem is this https://jump.dev/announcements/2021/02/22/agreement/

image

@joaquimg
Copy link
Member

Okay so there may be memory persisting between solves, that survive destroyprob? @NLaws should ask FICO support.

agreed

The related problem is this https://jump.dev/announcements/2021/02/22/agreement/

agreed^2

@joaquimg
Copy link
Member

This parameter might be relevant also:
https://www.fico.com/fico-xpress-optimization/docs/latest/solver/optimizer/HTML/TOTALMEMORY.html

+1 for contacting FICO at least for suggestions on how to manage memory.
For instance,
what is the difference between:
https://www.fico.com/fico-xpress-optimization/docs/latest/solver/optimizer/HTML/XPRSunloadprob.html
and
https://www.fico.com/fico-xpress-optimization/docs/latest/solver/optimizer/HTML/XPRSdestroyprob.html
?

also,
is free really needed.

@odow
Copy link
Member Author

odow commented Jun 17, 2021

image

suggest @NLaws should be fine explicitly calling finalize(backend(model))

@NLaws
Copy link
Contributor

NLaws commented Jun 17, 2021

Yes, I think the solution is:

function run_reopt(inputs)
    model = direct_model(Xpress.Optimizer())
    # ... do stuff
    results = get_results(model)
    xpress = backend(model)
    finalize(xpress)
    return results
end

@odow Should I also include GC.gc() ?

@joaquimg
Copy link
Member

I would do first experiments including gc

NLaws pushed a commit to NREL/REopt_API that referenced this issue Jun 17, 2021
@joaquimg
Copy link
Member

@NLaws can you add a call to the XPRSpostsolve function after your solve?
I recently noticed it frees some memory.

@NLaws
Copy link
Contributor

NLaws commented Oct 25, 2021

@joaquimg certainly! How do I do that with JuMP (or MathOptInterface)?

@joaquimg
Copy link
Member

try this: #157

@NLaws
Copy link
Contributor

NLaws commented Oct 25, 2021

What is the model in the code that you referenced? Can I access it from the JuMP.Model?

@joaquimg
Copy link
Member

Can just check out on that PR.
Or, if you are using direct_model (as in: #131 (comment))
You can do Xpress.postsolve(backend(model).inner) with with the JuMP model

@NLaws
Copy link
Contributor

NLaws commented Oct 25, 2021

Thanks @joaquimg. I have the change ready to deploy in our API and I will see if I can make a comparison of the memory growth with and without Xpress.postsolve.

@joaquimg
Copy link
Member

joaquimg commented Nov 6, 2021

Any news @NLaws ?

@NLaws
Copy link
Contributor

NLaws commented Nov 15, 2021

@joaquimg I ran the same test (REoptLite run 1,000 times in a for loop) with postsolve and did not see any memory use improvement. However, I ran it with Xpress 8.0 and I am now working on running the same scenario in Xpress 8.12

@joaquimg
Copy link
Member

Note that Xpress 8.13 is available.

@NLaws
Copy link
Contributor

NLaws commented Nov 16, 2021

see https://discourse.julialang.org/t/memory-consumption-growth-with-many-large-milps-in-jump/61895/64?u=nlaws for results with postsolve (still see memory growth issue)

@odow
Copy link
Member Author

odow commented Nov 16, 2021

Did you contact FICO for support? What'd they say?

@NLaws
Copy link
Contributor

NLaws commented Nov 17, 2021

Did you contact FICO for support? What'd they say?

I contacted FICO regarding upgrading Xpress, but I don't know what to ask them about the memory issue. If you go back in the thread on discourse you can see that solving a problem in a loop using Xpress alone does not lead to a memory "leak". The issue seems related to threading in Julia perhaps. What would you suggest that I ask FICO support?

@joaquimg
Copy link
Member

Did you run xpress on Mosel or C?

The reference test should be with the C API.

To simplify such test we could create a .lp file and initialize xpress with it and solve in a loop both in Julia and C.

@NLaws
Copy link
Contributor

NLaws commented Nov 17, 2021

I ran the knapsack.mos problem using Mosel with a bash loop. How do I convert the .mos to an .lp file and use the C API to run the .lp in a loop?

@odow
Copy link
Member Author

odow commented Mar 21, 2024

I've gone through and made a bunch of code-tidying changes recently. I didn't come across anything that might have led to a memory leak, so I think I'm going to mark this as a bug in Xpress.

The only potential is if memory is retained in the license environment. In which we might need something like:

function force_reset()
    Xpress.Lib.XPRSfree()
    Xpress.Lib.XPRSinit(C_NULL)
    return
end

I'm very reticent to add this to the finalizer of XpressProblem because we have no control over when the finalizer runs.

People probably need to manually call this in their own application.

@odow
Copy link
Member Author

odow commented Apr 3, 2024

I'm very tempted to just close this. I don't think there is much that we can do.

@joaquimg joaquimg closed this as not planned Won't fix, can't repro, duplicate, stale Apr 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants