andre-martins / AD3

Alternating Directions Dual Decomposition
GNU Lesser General Public License v3.0
68 stars 38 forks source link

A possible source of memory leak? #26

Open zzsfornlp opened 5 years ago

zzsfornlp commented 5 years ago

Hi, I'm not pretty sure, but according to memory-check by valgrind, it seems that this line might be a possible source of memory leak: here Since active_set_[k] and configuration are not the same pointer? Please take a look, thanks!

vene commented 5 years ago

Thanks for the find! We'll look into it. Has this caused memory issues for you?

vene commented 5 years ago

At a glance I think you're correct: active_set_[k] and configuration are the same configuration, but different in-memory objects, so I think the highlighted line should go.

@andre-martins do you agree?

andre-martins commented 5 years ago

I believe so, is there a test we can run to double check?

André

Em seg, 5 de ago de 2019 às 19:53, Vlad Niculae notifications@github.com escreveu:

At a glance I think you're correct: activeset[k] and configuration are the same configuration, but different in-memory objects, so I think the highlighted line should go.

@andre-martins https://github.com/andre-martins do you agree?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/andre-martins/AD3/issues/26?email_source=notifications&email_token=AAKTLQ6E2TLU4P7HFOG526LQDAPEDA5CNFSM4IJFZHU2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD3RXASQ#issuecomment-518221898, or mute the thread https://github.com/notifications/unsubscribe-auth/AAKTLQ6U7WMEHIXQTYLK4B3QDAPEDANCNFSM4IJFZHUQ .

zzsfornlp commented 5 years ago

In my program, the memory gradually increase until it gets killed because of out-of-memory (after several hours). I checked mem-leak with valgrind, which gives the hint that some of the objects created at these two lines are lost: place1 and place2. Actually I'm not very familiar with the core algorithm parts, but I think maybe that line can be the problem.

After I removed that line, things seem to work well and there are no more OOM issue. Unfortunately, I tested with a relatively large system, which seems to be hard to be split out as a simple test case.

Is is possible to test with the parsing example at this repo, by repeating the decoding forever or checking with valgrind? Thanks!