Skip to content

Conversation

@danirabbit
Copy link
Member

Can be rebase merged

@danirabbit danirabbit requested a review from a team March 19, 2025 18:10
@Oowoosh0 Oowoosh0 force-pushed the danirabbit/listview branch from b964052 to 866f2e7 Compare March 21, 2025 16:51
@Oowoosh0
Copy link
Contributor

Oowoosh0 commented Mar 22, 2025

Sorry there is one more thing that I hadn't noticed before, but this leaks memory.
New TrackRows are constantly created as you scroll the ListView up or down, but they never get destroyed. They also don't get destroyed when emptying the list. We probably need to use the unbind or teardown signal from the SignalListItemFactory to handle that.

@Oowoosh0 Oowoosh0 mentioned this pull request Mar 23, 2025
@danirabbit
Copy link
Member Author

@Oowoosh0 good catch! I'm still learning ListView but that makes a lot of sense. I think it's working as expected now. If not, let me know how you're testing and I'll keep poking at it :)

@danirabbit danirabbit requested a review from Oowoosh0 March 23, 2025 16:33
@danirabbit danirabbit force-pushed the danirabbit/listview branch from ec2bb03 to 6c57279 Compare March 23, 2025 16:33
@Oowoosh0
Copy link
Contributor

Oowoosh0 commented Mar 24, 2025

For testing I used the very professional method of opening a system monitor, opening over 700 music files in the app and then crazily scrolling up and down. Which makes the memory usage slowly creep up over time. It does not do so in the version still using ListBox. Although TrackRows seems to be destroyed correctly actually.
I'll look more into it, but we could also still go ahead and merge it since it only goes up slowly and you would need to crazily scroll around for a long time before it becomes a problem.

@Oowoosh0
Copy link
Contributor

I think it is the Bindings. New Bindings are being created whenever a different AudioObject is set in a TrackRow but they never get destroyed. I thought unbind() would take care of that but for some reason it does not seem to do so :/

@Oowoosh0
Copy link
Contributor

@danirabbit connecting and disconnecting to the notify signal manually and not using bind_property actually gets rid of the leak. Calling unbind() for some reason does not free the memory used by a Binding and bind_property creates new Bindings every time a new AudioObject is set to a TrackRow. If you haven't done any more work on this branch then I can push that change.

@danirabbit
Copy link
Member Author

@Oowoosh0 sure go ahead :) Thanks!

@danirabbit danirabbit enabled auto-merge (squash) March 27, 2025 17:35
@danirabbit danirabbit merged commit 6c401ee into main Mar 27, 2025
3 checks passed
@danirabbit danirabbit deleted the danirabbit/listview branch March 27, 2025 17:36
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