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

UI lags a long time when loading a rather big playlist #11724

Closed
gui-lux opened this issue Jul 8, 2023 · 12 comments · Fixed by #11851
Closed

UI lags a long time when loading a rather big playlist #11724

gui-lux opened this issue Jul 8, 2023 · 12 comments · Fixed by #11851

Comments

@gui-lux
Copy link

gui-lux commented Jul 8, 2023

Bug Description

Loading a 3k items playlist takes roughly a minute.
Meanwhile, UI is stuck and various controls like bas/mid/tre knobs do not respond as long as the UI is stuck.
2.3.x used to load these playlists within a second.
Did a tail -f ~/.mixxx/mixxx.log while loading a playlist and nothing came out, and no excessive cpu usage observed.

Setup used :

  • rpi4 (4GB) + 15.6 hdmi touch screen
  • denon mc4000
  • 3k items playlist (1k is enough to reproduce)

Version

2.4-beta

OS

DietPiOS (debian bullseye based)

@gui-lux gui-lux added the bug label Jul 8, 2023
@gui-lux gui-lux changed the title UI lags a long time when loading a rather big playlist v2.4 - UI lags a long time when loading a rather big playlist Jul 8, 2023
@JoergAtGithub JoergAtGithub added this to the 2.4.0 milestone Jul 8, 2023
@ronso0 ronso0 changed the title v2.4 - UI lags a long time when loading a rather big playlist UI lags a long time when loading a rather big playlist Jul 12, 2023
@ronso0
Copy link
Member

ronso0 commented Jul 12, 2023

I can confirm this. Tested with ~1k tracks, no noticeable delay, tested 12k tracks, whoops, still waiting for Ui to recover...
select takes ~40 ms but UI update takes ages.

@ronso0
Copy link
Member

ronso0 commented Jul 12, 2023

same with or without cover art column (or just the # column enabled)

@ronso0
Copy link
Member

ronso0 commented Jul 12, 2023

and no excessive cpu usage observed.

nope, here 1 core is fully utilized.

@ronso0
Copy link
Member

ronso0 commented Jul 12, 2023

to clarify:
a crate with the same track count loads blazing fast (again visible columsn seem to be irrrlevant)

@ronso0
Copy link
Member

ronso0 commented Jul 12, 2023

I was curious and tried to find the significant difference in loading playlists vs. crates.
Appearantly it's the INNER JOIN here

QString queryString = QString(
"CREATE TEMPORARY VIEW IF NOT EXISTS %1 AS "
"SELECT %2 FROM PlaylistTracks "
"INNER JOIN library ON library.id = PlaylistTracks.track_id "
"WHERE PlaylistTracks.playlist_id = %3")
.arg(escaper.escapeString(playlistTableName),
columns.join(","),
QString::number(playlistId));

required to fetch columns from both library and PlaylistTracks.
And no table indices to speed it up.

@ronso0
Copy link
Member

ronso0 commented Aug 21, 2023

Nope, that wild guess was nonsense.

The culprit is in PlaylistDAO::removeHiddenTracks, more specifically the changes of #2900 > 7993234 fixing #10025

query.prepare(QStringLiteral(
"SELECT p1.position FROM PlaylistTracks AS p1 "
"WHERE p1.id NOT IN ("
"SELECT p2.id FROM PlaylistTracks AS p2 "
"INNER JOIN library ON library.id=p2.track_id "
"WHERE p2.playlist_id=p1.playlist_id "
"AND library.mixxx_deleted=0) "
"AND p1.playlist_id=:id"));

If I revert these changes huge playlists are loaded instantly, so obviously the new query has a performance impact.

I tried to detect if cleanup is needed by first checking whether the playlist includes hidden tracks but since I'm no SQL pro I fail to detect orpahend tracks (track ids in the PlaylistTracks table but not in library).
Anyone has an idea how to proceed? How to fix the bug but keep the DAO performant?

@Swiftb0y
Copy link
Member

I'm no SQL pro either, explain query plan might help here. My uneducated guess is that the performance hit is likely due to the nested SELECT, forcing SQLite into an $O(n^2)$ (where $n$ is the number of tracks in PlaylistTracks) algorithm. Getting rid of the inner SELECT would be best. If that's not possible, then removing the dependency of the inner SELECT on p1 (see the WHERE clause) would likely be the second most effective as it doesn't have be recalculated in each outer SELECT. And then making sure we have indices of the columns being accessed could result in another algorithmic improvement ( $O(n) \to O(log(n))$ ).

@Swiftb0y
Copy link
Member

Okay, so running EXPLAIN QUERY PLAN on the statement yields the following:

id	parent	notused	detail
3	0	0	SEARCH p1 USING INDEX idx_PlaylistTracks_playlist_id_track_id (playlist_id=?)
11	0	0	CORRELATED LIST SUBQUERY 1
14	11	0	SEARCH p2 USING COVERING INDEX idx_PlaylistTracks_playlist_id_track_id (playlist_id=?)
21	11	0	SEARCH library USING INTEGER PRIMARY KEY (rowid=?)

So, since none of them use SCANs, the search is not doing linear search on the entire table fortunately. The unfortunately, the last two queries, are CORRELATED and thus depend on the parent query, causing them to be run for each output row of the first query.

This is probably the source of the slowdown.

@Swiftb0y
Copy link
Member

Can you try changing the code to:

query.prepare(QStringLiteral( 
         "SELECT p1.position FROM PlaylistTracks AS p1 " 
         "WHERE p1.id NOT IN (" 
         "SELECT p2.id FROM PlaylistTracks AS p2 " 
         "INNER JOIN library ON library.id=p2.track_id " 
         "WHERE p2.playlist_id=:id " 
         "AND library.mixxx_deleted=0) " 
         "AND p1.playlist_id=:id")); 

(the difference is only in the WHERE clause).

That removes the correlation and should allow for the same optimizations that made the query before 7993234 fast enough.

@ronso0
Copy link
Member

ronso0 commented Aug 21, 2023

Woooh, that fixes it! Thanks!

However, I can't really test if this query removes orphaned tracks, because it seems this is done during startup already??
Makes me wonder if the PlaylistDAO::removeHiddenTracks should look just for hidden files, not for orphans.

How I tested:

  • have a playlist ABC with 3058 tracks
  • edit mixxxdb: PlaylistTracks > set 4 track_ids to ids not in library
  • start Mixxx, unfold Playlists (don't select ABC!): ABC title says 3054

Same for hidden tracks:

  • pick some track ids from ABC
  • edit mixxxdb: library -> set mixxx_deleted to 1 for those tracks
  • start Mixxx, unfold Playlists (don't select ABC!): ABC track count is reduced accordingly

@ronso0
Copy link
Member

ronso0 commented Aug 21, 2023

@Swiftb0y Do you intend to open a PR, or should I do it (even though I can't explain why it is faster now)?

@Swiftb0y
Copy link
Member

However, I can't really test if this query removes orphaned tracks, because it seems this is done during startup already??

Not sure either. To be honest I don't fully understand the query either, nor am I very familiar with mixxx's library infrastructure. But removing orphaned entries at startup is probably the better behavior.

Do you intend to open a PR, or should I do it (even though I can't explain why it is faster now)?

I don't care either way. But I can open a PR and include a little bit of explanation why the query is faster while staying equivalent in the commit message. I didn't bother testing the change but you already seem to have done a thorough job doing so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants