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

Prevent memory leaks by cleaning up playlist listeners #218

Merged
merged 2 commits into from
Oct 21, 2023

Conversation

Borewit
Copy link
Owner

@Borewit Borewit commented Oct 7, 2023

Ths PR improves the de-registration of playlist listeners, to prevent memory leaks.

Although at various places the life-cycle management of playlist listeners improves, as well duplicate playlists lists has been removed, playlists are still being present in memory after closure of the playlist. It looks like at least a Future of the SwingWorker, which loads the playlist in the background keeps a reference to the playlist, preventing the garbage collector from cleaning it. I could not find a method to clear the Future or the SwingWorker. As the number of SwingWorker are limited as they are rotated from a pool, I assume the leak is limited.

May have resolved #219

@Borewit Borewit self-assigned this Oct 7, 2023
@Borewit Borewit added the debt Technical debt label Oct 7, 2023
@Borewit Borewit changed the title Cleanup listeners Prevent memory leaks by cleaning up playlist listeners Oct 7, 2023
@github-actions
Copy link

github-actions bot commented Oct 7, 2023

@touwys
Copy link

touwys commented Oct 8, 2023

Build run (#6442603706, attempt #1) artifacts

👉🏻 Should I be testing this build?


Additional information pertaining to the memory usage of this particular build:

  1. Build:

    image

  2. It appears that it is not so much a memory issue, as one of the CPU: Almost throughout the complete playlist repair process, listFix() will grab hold of 98-100% of the CPU processing power. It uses c. 39% RAM:

    image

  3. CPU Specs:

    image

@touwys
Copy link

touwys commented Oct 8, 2023

@Borewit

About the aforesaid post:

I cannot remember that the excessive CPU-usage was ever an issue before — even on the ancient PC. The available CPU-processing power and physical memory coped rather well with the job at hand. Can you think of anything that could've changed that state of affairs? More importantly, do you think it can be fixed, or improved upon?

@Borewit
Copy link
Owner Author

Borewit commented Oct 11, 2023

👉🏻 Should I be testing this build?

Yes that is useful, if it does not break anything it good to go.

I cannot remember that the excessive CPU-usage was ever an issue before

That has been introduced in #203, when we enabled parallel processing. It uses nearly uses all CPU power available to get the job done, which should not be an issue.

@Borewit Borewit merged commit b0ac1c3 into main Oct 21, 2023
4 checks passed
@Borewit Borewit deleted the cleanup-listeners branch October 21, 2023 09:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
debt Technical debt
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Playlist under edit cannot be refreshed after all the track matches were found
2 participants