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

fix issue where millions of ISetEntries would pile up and never be removed #53

Closed
wants to merge 1 commit into from

Conversation

wkritzinger
Copy link

After running on our production server for a few days we experienced slowdowns and a heap dump revealed millions of ISetEntries so I investigated and found out that garbage collected values of ISetEntries cannot be removed anymore with the existing method so I changed it a bit to allow removal of ISetEntries themselves.

@bennidi
Copy link
Owner

bennidi commented Oct 27, 2013

Hi morghus,
very sorry to hear that you had trouble in your production environment. I hope it did not create to much of irritation and was at least a bit exciting to trace down. And thanks for reporting. I looked over your code and it showed me clearly where the problem was. But I think that besides fixing the issue at hand the source of the problem was actually something else, namely that the remove method of the set implementation was not the right way to go. I simply chose it because I thought that it already correctly handled locking and I wanted to rely on that. I fixed the code in a different way now and would really very much appreciate if you could test it in your application (now that you know so much about this specific behavior :) Also, I would really like to design better tests for these kind of issues but I think it's really hard to do so. Do you have any good ideas how to improve test coverage here? And please understand my late reply, as I was on holiday the last weeks. So, please tell me whether you can cross check my fix as in the current source code before I make a release. Thanks!

@wkritzinger
Copy link
Author

It wasn't a big deal, we just had to restart the application every few days. After my fix that stopped. I looked over your fix and it seemed to work. We'll start using your version as soon as you release it. As for tests, a test would need access to the map in order to verify that the garbage collected items have been properly removed. System.gc() is usually not sufficient to reliably cause a full GC so I always cause a OutOfMemoryError with a StringBuilder to make sure everything has been GC'd.

@bennidi bennidi closed this Jan 10, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants