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

Available Plugin listing is slow #37

Closed
psobolewskiPhD opened this issue May 27, 2024 · 6 comments
Closed

Available Plugin listing is slow #37

psobolewskiPhD opened this issue May 27, 2024 · 6 comments
Assignees
Milestone

Comments

@psobolewskiPhD
Copy link
Member

This might be a duplicate, since it's from the original napari-side PR. See for example this post from Grzegorz:
napari/napari#5198 (comment)
But as it's a high priority maybe it should be it's own issue anyways:
With the number of plugins now, the listing of available plugins is really slow.
It's also not cached, so close-reopen of the installer starts the process all over.

@goanpeca
Copy link
Contributor

goanpeca commented May 27, 2024

Hi @psobolewskiPhD thanks for opening the issue.

This is a tricky one as the current way we construct the listwidget list is bound to be slow as the size of items increases. A listview is pretty efficient but a listwidget where every item is in itself a widget of widgets will start to collapse as the list keeps increasing.

We intentionally throttle the filling of the list in order to not freeze the UI (which was the previous state som time ago).

However what you mention of the caching is indeed something that could improve the experience greatly as only a hard refresh could trigger a new reload of all the plugins (and a ping of the API) BUT, we would need to avoid the dialog from actually being destroyed/closed after the first open. This is the low hanging fruit here, and I would aim for that.

This would be a great topic to discuss on the bundle meeting :) @psobolewskiPhD @Czaki

@goanpeca goanpeca self-assigned this May 27, 2024
@goanpeca
Copy link
Contributor

goanpeca commented May 29, 2024

So to give a little more context:

The way we are (abu)using the QLIstWidget with QListItems that hold a custom widget is a non performant way that will not scale well for an unbounded growth of plugins in the ecosystem. The current approach was to throttle the loading of plugins 1 by 1 by 100ms to give the appearance of responsiveness to the dialog. If we tried to add 400 widgets at once the dialog would freeze for around 10-20 seconds depending on your machine, making it a very annoying User experience.

Some options to make this better moving forward include:

Option 1 (short term)

Make the widget list a proper list with no custom widgets. That is each item in the list can have:

  • A checkbox
  • A rich text that could be customized
  • An icon could be included as well to handle state spin progress etc
  • A final actions button that would popup something to configure versions and provide any additional info.

This way this widget would be performant and load the information "instantly" in the order of thousands and maybe more.

Option 2 (medium term)

Make the list widget a list view with no custom widgets. This requires the additional creation of a ListModel that the listView accesses. It is more involved that the previous one but will be more performant

That is each item in the list can have:

  • A checkbox
  • A rich text that could be customized
  • An icon could be included as well to handle state spin progress etc
  • A final actions button that would popup something to configure versions and provide any additional info.

This way this view would be performant and load the information "instantly" in the order of millions of items, which should cover most use cases of this plugin manager.

Option 3 (long term)

To provide a custom and complex widget, we need to use delegate and handle paint events with the primitives that Qt provides. This is more involved and way more complex to do and maintain, but would provide a performant way of drawing a complex looking widget within a list item, without the burden of actually creating a family of widgets. I am not sure this is worth the effort and I have not seen examples within the Qt docs that go beyond a custom but simple delegate.

Option 4 (short term) updated

Only populate the available plugins list upon direct search, that way we can still preserve the current complex widget, but it would only display widgets that match a given search criteria.


I would favor going to option 1 and eventually option 2 or option 4 which can be made progressively, however going from the current state to a more simple one might be seen as a regression, but these are the current limitations of the approach chosen. Moving forward it will only get less performant.

@psobolewskiPhD psobolewskiPhD added this to the 0.5.0 milestone May 29, 2024
@psobolewskiPhD
Copy link
Member Author

Something to test:
Duration of the timer:

self._add_items_timer.setInterval(100)

_add_items in batches:

if len(self._plugin_data) == 0:

@psobolewskiPhD
Copy link
Member Author

Also, just proving the number available vs. the length, would help a lot in terms of expectations, here:
image

@goanpeca
Copy link
Contributor

I think the way to go and solved this definitely would be to follow the approach over at

#51

We only search for plugins on demand and we populate the list with only a few of the results, not all. What would be a reasonable number to have as a search results limit? 10? 20?

@jaimergp
Copy link
Contributor

Let's open an issue to discuss this further because the initial mitigations have been take care of. Thanks!

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

No branches or pull requests

3 participants