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

library redesign GUI lag #1714

Merged

Conversation

Be-ing
Copy link
Contributor

@Be-ing Be-ing commented Jun 18, 2018

I'm doubtful this should actually be merged, but I am opening this pull request to bring attention to the issue. After a weekend of Git bisecting and cherry picking fixes for build failures, I found 4cc9dc5 to be the commit that introduced the big lag whenever a Track object changed. This is easily testable by loading a track, not playing the deck, and setting a new hotcue. The lag scales with the number of tracks in the database.

I think these signals could be handled much more efficiently by updating the parts of the TracksTreeModel relevant to the changed Tracks instead of executing this expensive database query to rebuild the entire model. @daschuer, @uklotzde, @jmigual, or @gramanas, would you like to try implementing that?

This causes a severe GUI lag whenever any Track is changed. It is
easily testable by loading a track, not playing the deck, and
setting a new hotcue. The lag scales with the number of tracks
in the database.
@Be-ing
Copy link
Contributor Author

Be-ing commented Jun 18, 2018

If we cannot manage to fix this by the end of June, I think we could merge this hack to get the library redesign finally into 2.2 beta and work on fixing the problem during the beta period.

@daschuer
Copy link
Member

Thank you for this hard to find bug and probably hours staring at running builds.

Can't we just merge this as it is? The reintroduced bug is most likely the smaller issue. We can also cut out a 2.1 branch soon. Master still builds with Qt4, but I have no idea if Mixxx 2.2 will actually have Qt5 and this branch, facing the number of pendung complains we see.

@Be-ing Be-ing merged commit 4df7cae into mixxxdj:jmigual-library-redesign Jun 18, 2018
@Be-ing
Copy link
Contributor Author

Be-ing commented Jun 18, 2018

You're right, there's no point keeping the library redesign branch in such an unusable state for the relatively minor bug this reintroduces. With this merged, people can test the redesign branch more thoroughly and hopefully catch more bugs. We released 2.1 beta with a much more serious bug (GUI hanging on batch analysis), so if we don't manage to fix this by the end of June I don't think it should hold up merging the library redesign branch for 2.2.

@Be-ing
Copy link
Contributor Author

Be-ing commented Jun 18, 2018

Thank you for this hard to find bug and probably hours staring at running builds.

It really helps to now have a computer that can do a clean build of Mixxx in under 10 minutes. :)

@WaylonR
Copy link
Contributor

WaylonR commented Jun 18, 2018

O.O whats your computers spec, Be-ing?

@Be-ing
Copy link
Contributor Author

Be-ing commented Jun 18, 2018

Intel Core i7 8550U, 16 GB DDR4 RAM, 1 TB M.2 SSD

@jmigual
Copy link
Contributor

jmigual commented Jun 18, 2018

Hi @Be-ing I can try to take a look at it next week and see how can I solve it.

Caching the loaded data from the SQL query may solve it but it may lead to other problems.

@Be-ing Be-ing mentioned this pull request Jun 23, 2018
11 tasks
@Be-ing
Copy link
Contributor Author

Be-ing commented Jun 23, 2018

Great, thank you @jmigual.

Caching the loaded data from the SQL query

Isn't that data already cached by the tree model?

@Be-ing Be-ing deleted the library-redesign-gui-lag branch June 25, 2018 00:05
@jmigual
Copy link
Contributor

jmigual commented Jun 30, 2018

True it is, I will be using it, thanks for pointing it out!

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.

4 participants