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 some memory leaks from recreating UserControls #1044

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

thailyn
Copy link
Contributor

@thailyn thailyn commented Jun 23, 2019

Procurement recreates its stash and recipe views whenever they need updating. This is inefficient and causes memory leaks (and this is readily apparent after automatically (and often) refreshing tabs). This PR attempts to refresh those tabs instead of recreating them. I believe I have addressed all the issues with reusing the same objects.

Note that the trading view is also recreated whenever tabs are refreshed; that is not addressed in this PR.

@nqrcqn
Copy link
Contributor

nqrcqn commented Jun 24, 2019

I'm testing this, Currency Breakdown doesn't seem to change after refresh despite having more chaos than before

@nqrcqn
Copy link
Contributor

nqrcqn commented Jun 24, 2019

And it seems character inventories don't get refreshed, but regular stashes do.

@thailyn
Copy link
Contributor Author

thailyn commented Jun 25, 2019

Thank you so much for testing out these changes! I just pushed another commit to this PR that should fix updating inventories; I had long suspected, but had not confirmed, that inventories were not being reported correctly in the "refresh all tabs" and similar windows, and that seems to indeed be the case (now fixed (though kinda hacky)). I'll look into the currency breakdown not being updated properly shortly. Thanks again!

@aydjay
Copy link
Contributor

aydjay commented Jun 25, 2019

I'll paw through this PR once you've marked it ready to merge, looks like we have a lot of outstanding PR's that need to be merged @Stickymaddness

We do not want to create a new RecipeView every time we need to refresh
the recipes; this leads to leaking RecipeView objects, which have
subscribed event listeners.  This isn't the most elegant solution, but
it gets the job done.
Also add a method to clear the contents of the RefreshView between times
it is displayed.  Recreating the RefreshView every time is a small
memory leak.
Recreating the StashView every time we want to refresh the stash is a
memory leak, and makes Procurement lag while it rebuilds the stash.  As
an added bonus, now the last-selected stash tab remains selected when
revisiting the stash.
The onStashLoaded events were not being fired when loading character
inventories, so 1) there was no loading message for these "tabs" when
refreshing [used] tabs, and 2) the appropriate actions to take when
reloading tabs were not happening.
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