Force early cleanup of an instance when the instance list is re-sliced#7
Force early cleanup of an instance when the instance list is re-sliced#7jdamick wants to merge 1 commit into
Conversation
|
The other option would be to switch to |
|
@chrisohaver / @miekg wdyt? |
|
been a couple weeks, trying someone else.. @johnbelamaric wdyt? |
|
@thevilledev can u merge? this prevents memory leakage .. |
thevilledev
left a comment
There was a problem hiding this comment.
Hey @jdamick - I don't have merge permissions to the repo, but I can help with the review. I would prefer slices.Delete as it makes the intent obvious. Other than that, LGTM.
76ef215 to
ed369b5
Compare
…d on Stop() & Restart() to prevent holding memory Signed-off-by: Jeffrey Damick <jdamick@amazon.com>
|
@thevilledev i looked into replacing with slice but it would require changing the go.mod to bring the language support up to 1.18 for slices support. So I've left it as-is. |
|
Yup, just realised it as well. I did some further debugging through a memtest. This runs 10k restarts and GC + heap profile afterwards: go test -run TestRestartDoesNotLeakInstances -v
go tool pprof -http=:8888 restart_heap.pb.gz
# or a diff of two runs
go tool pprof -top -diff_base=base.pb.gz new.pb.gzHeap size and object counts remain stable before and after the proposed change. With the append-reslice a pointer is removed and len shortened. @jdamick are you able to share more details about your issue, and possibly how that could be reproduced? |
On Stop() & Restart() to prevent holding memory for a potentially long time, nil out the instance before re-slicing to force early cleanup.