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

Load notifications without blocking #241

Merged
merged 9 commits into from
Jun 8, 2022
Merged

Conversation

jeremypw
Copy link
Collaborator

@jeremypw jeremypw commented Mar 3, 2022

Partially addresses #237
Partially address #123

This PR greatly improves the load speed of the wingpanel when there are thousands of sessions notifications. The wingpanel's appearance is no longer blocked while the notification listbox is filled with entries. The notification session file is no longer needlessly rewritten. A faster function for counting the number of session notifications is provided. The function for getting a list of session notifications is sped up.

The session notifications are added to the listbox asynchronously.
A spinner shows while the session notifications are being loaded.

This only addresses loading issues. The popover is still sluggish to appear when it contains >1000 notifications.

It is noted that each NotificationEntry is a complex widget so this limits the speed they can be constructed and displayed.

@jeremypw jeremypw requested a review from a team March 3, 2022 18:34
src/Indicator.vala Outdated Show resolved Hide resolved
@jeremypw
Copy link
Collaborator Author

jeremypw commented Mar 6, 2022

Using an Idle loop instead of a thread results in a noticeable delay in the wingpanel indicators appearing (ca 1 second on my machine with 1000 notifications) but this is still a lot faster than previously.

@tintou
Copy link
Member

tintou commented Mar 7, 2022

I don't think that we can really address the issue with GTK3, we need GTK4 for very large lists

@jeremypw
Copy link
Collaborator Author

jeremypw commented Mar 7, 2022

Maybe we should prompt the user to clear some notifications if the number gets unreasonably large? Or automatically dismiss the oldest ones.

@jeremypw jeremypw changed the title Load notifications more efficiently in a separate thread Load notifications without blocking Mar 7, 2022
# Conflicts fixed in:
#	src/Indicator.vala
#	src/Services/Session.vala
@jeremypw
Copy link
Collaborator Author

jeremypw commented Mar 7, 2022

@tintou Do you think this PR is enough of an improvement to merge for now and iterate later?

Copy link
Member

@danirabbit danirabbit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah until the Gtk4 port we won't get good performance out of the list, but this at least is nice not to block panel loading 👍

@danirabbit danirabbit merged commit 0d9914c into master Jun 8, 2022
@danirabbit danirabbit deleted the load-notifications-faster branch June 8, 2022 16:26
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