search

elementary / appcenter

Pay-what-you-can app store for elementary OS
https://elementary.io
GNU General Public License v3.0
542 stars 103 forks source link

Fast search #2179

Open leolost2605 opened 5 months ago

leolost2605 commented 5 months ago

Moves search from a slow and synchronous search to a fast and incremental search. This can range from just feeling a bit snappier for optimal searches for the old system like "firefox" to going from a 20 second search leading to a "Not responding" popup to a near instantaneous search.

We already have a list of available packages so why go the route via multiple reassignments and list creations etc. via the appstream pool if we can just do it ourselves. Yields the same search results and greatly improves performance. Also switch to a Gtk.ListView so we can handle the great number of apps we sometimes get. I've left the synchronous search in place since it's easier to user with search provider etc.

In the videos the part where I search for flat is admittedly an extreme case since almost all apps are returned there but we can still see that the new search doesn't take longer than a second (vs. 20 - 30 or so with GNOME Software or our old search) to completely fill the list.

Old (I didn't stop typing, the UI just completely blocked): Kooha-2024-06-20-21-10-14.webm

New: Kooha-2024-06-20-21-18-55.webm

GNOME Software: Kooha-2024-06-20-21-16-31.webm

danirabbit commented 5 months ago

I get a ton of new warnings on start with this branch about:

** (io.elementary.appcenter:128313): CRITICAL **: 13:36:51.569: app_center_core_package_get_is_local: assertion 'self != NULL' failed

** (io.elementary.appcenter:128313): CRITICAL **: 13:36:51.584: app_center_core_package_get_name: assertion 'self != NULL' failed

** (io.elementary.appcenter:128313): CRITICAL **: 13:36:51.584: app_center_core_package_get_summary: assertion 'self != NULL' failed
leolost2605 commented 5 months ago

Ok the warnings should be fixed now. I also added some additional null checks that shouldn't trigger if we do it right but better safe than sorry :)

leolost2605 commented 5 months ago

The reason for making package settable was to enable recycling these widgets which is necessary for a performant listview with many packages. The big issue is that when using property setters and gobject style construction those will run before the construct block which means if we try to access widgets created there they are still null, which is why - to avoid duplicate code - I decided to break some gobject style construction . We could use notifies instead of using property setters to update stuff but that might increase complexity even more idk? I think in the long run it would probably be best to just port everything to listview/gridview and eventually get rid of constructing with a package alltogether.

danirabbit commented 5 months ago

@leolost2605 would it make more sense then to make a new clean search list row widget that has a settable package instead of modifying all the way up through the abstract widgets that are used elsewhere?

leolost2605 commented 5 months ago

That would lead to a lot of duplicate code though (or loss of functionality). Maybe we should first port everything to the new views and then just hook up this fast search?