search

Arlodotexe / strix-music

Combine any music sources into a single library. It's your music. Play it your way.
http://www.strixmusic.com
139 stars 4 forks source link

Runaway RAM usage / perf hit caused by ZuneTrackCollectionItem #227

Closed Arlodotexe closed 2 years ago

Arlodotexe commented 2 years ago

Describe the bug

In ZuneTrackCollectionItem, we're getting all tracks from the parent collection, then attaching listeners to the event.

https://github.com/Arlodotexe/strix-music/blob/7ff97476cdc0219a8fa397dcc237403118acf9bb/src/Shells/StrixMusic.Shells.ZuneDesktop/Controls/Views/Collection/ZuneTrackCollectionItem.cs#L93

This is an O(n^2) operation, doubled since we have 2 track lists that load at once in Zune. For 1,033 songs, it allocations 2,100,000 event handlers, at the cost of ~256Kb each. For 1,033 songs, this results in a memory increase of 200-400Mb that isn't needed.

Affected area

Regression

This issue was introduced in PR #174

Steps to reproduce

1. Load up a core with a LOT of songs
2. Load the Zune Desktop shell
3. Go to the Collection View. Observe the freezing and increased memory usage

Visual repro steps

No response

Expected behavior

The app should not freeze while switching to the collection view in the Zune Desktop shell.

Additional context

Issue is not present in Groove: image

Issue is not present when you avoid attaching the event (even with an empty handler): image

amaid commented 2 years ago

@Arlodotexe More precisely the main cause of the reason is the events being attached to the LibraryViewModel on first time track loading. We don't need to attach any events to the tracks when they are loaded from library. "Zune artist column for tracks" functionality only exists on a selection of artist and album collection, limiting the event attachments to that brings down the memory allocation to just ~380 MB: image

Arlodotexe commented 2 years ago

@amaid That lines up with what I expected. The ITrackCollection that is loaded into ZuneTrackCollection can be the Library, an Album, a Playlist, etc.,

"Zune artist column for tracks" functionality only exists on a selection of artist and album collection,

Please double check the original Zune Desktop app (v4.8) and make sure this statement is correct.

We'll still need to fix the O(n^2) allocations that are done whenever ZuneTrackCollection is used.