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(library): fix PlaylistDAO::removeHiddenTracks performance #11851

Merged
merged 1 commit into from
Aug 21, 2023

Conversation

Swiftb0y
Copy link
Member

@Swiftb0y Swiftb0y commented Aug 21, 2023

fixes #11724

The query consists of an outer query and inner subquery. In the previous form, the inner query had a dependency on p1.playlist_id of the outer query. Which could change for each row of the outer select statement. This made the inner query a CORRELATED LIST SUBQUERY, which caused the subquery to be re-evaluated for each row of the outer query. This dependency however is unnecessary because of the AND p1.playlist_id=:id restriction in the outer query. So in theory, p1.playlist_id never changes. Unfortunately, the SQLite was not able to make that deduction which lead to the dependency. Manually specifying that the p1.playlist_id is constant for the entire subquery, removes the dependecy to the outer query, resulting in a plain LIST SUBQUERY which can be evaluated once and then reused for all rows of the outer subquery. This results in reduced algorithmic complexity and thus drastically improved performance for large tables.

The query consists of an outer query and inner subquery. In the
previous form, the inner query had a dependency on `p1.playlist_id`
of the outer query. Which could change for each row of the outer
select statement. This made the inner query a `CORRELATED LIST
SUBQUERY`, which caused the subquery to be re-evaluated for each
row of the outer query. This dependency however is unnecessary
because of the `AND p1.playlist_id=:id` restriction in the outer
query. So in theory, `p1.playlist_id` never changes. Unfortunately,
the SQLite was not able to make that deduction which lead to the
dependency. Manually specifying that the `p1.playlist_id` is
constant for the entire subquery, removes the dependecy to the outer
query, resulting in a plain `LIST SUBQUERY` which can be evaluated
once and then reused for all rows of the outer subquery. This
results in reduced algorithmic complexity and thus drastically
improved performance for large tables.
@ronso0 ronso0 linked an issue Aug 21, 2023 that may be closed by this pull request
@ronso0
Copy link
Member

ronso0 commented Aug 21, 2023

Just to get an idea of the impact of this tiny fix:
opening a playlists with 100 and 3000 tracks takes

branch 100 3000
2.4 60 ms 13384 ms
this 12 ms 30 ms

🎉

As mentioned in #11724 (comment) I can't tell whether that query actually does what it should since the playlists seem to be cleaned up during startup already (and i don't rescan the collection on startup).

@ronso0 ronso0 added this to the 2.4.0 milestone Aug 21, 2023
@ronso0
Copy link
Member

ronso0 commented Aug 21, 2023

LGTM, thanks a lot!

@ronso0 ronso0 merged commit a5bdd6f into mixxxdj:2.4 Aug 21, 2023
@Swiftb0y Swiftb0y deleted the fix/gh11724 branch August 21, 2023 20:45
@Swiftb0y
Copy link
Member Author

Just to get an idea of the impact of this tiny fix:
opening a playlists with 100 and 3000 tracks takes

The effect of algorithmic complexity in action ;)

LGTM, thanks a lot!

Thank even more for testing and figuring out what line and commit caused the regression. That cut the time required ten fold.

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

Successfully merging this pull request may close these issues.

UI lags a long time when loading a rather big playlist
2 participants