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

Performance hitch when typing in the search box while versions tab is open #4046

Closed
JonnyOThan opened this issue Mar 1, 2024 · 2 comments · Fixed by #4049
Closed

Performance hitch when typing in the search box while versions tab is open #4046

JonnyOThan opened this issue Mar 1, 2024 · 2 comments · Fixed by #4049
Assignees
Labels
Bug Something is not working as intended GUI Issues affecting the interactive GUI Performance Something's slower than it should be

Comments

@JonnyOThan
Copy link
Contributor

JonnyOThan commented Mar 1, 2024

I'm pretty sure the search box isn't the only way to trigger this but I found a consistent repro:

  1. open the versions tab
  2. in the search box, start typing "ksp community fixes"
  3. When you hit the second 'm', CKAN will hang for a few seconds

The first time I did this the hang was probably ~20 seconds. Doing it again resulted in a shorter hang, but it's still several seconds long.

I'm running this on the latest dev version so I don't have symbols (huh apparently there are some symbols in here; just not jitted code maybe) but maybe this will help:
image

The thread that eventually unblocked the main thread is here:
image

Kinda hard to tell, but maybe calling ToList on a parallel enumerator is doing some bad stuff?

@JonnyOThan JonnyOThan added Bug Something is not working as intended GUI Issues affecting the interactive GUI labels Mar 1, 2024
@HebaruSan HebaruSan self-assigned this Mar 2, 2024
@HebaruSan HebaruSan added the Performance Something's slower than it should be label Mar 2, 2024
@HebaruSan
Copy link
Member

HebaruSan commented Mar 2, 2024

I think we have over-parallelization here. RelationshipResolver.ModList is parallel (for speeding up very large changesets), and Versions.checkInstallable calls it in parallel (via ModuleInstaller.CanInstall; that's what feeds the ToList call highlighted above). This nesting can do wacky things to threads; I hit this a couple of times while parallelizing other stuff. This probably also explains why the Versions tab doesn't start filling in right away.

That link recommends only parallelizing the outer loop.

@HebaruSan
Copy link
Member

... confirmed (more or less). Commenting this line makes the inner loop non-parallel, which makes the response to color the list instantaneous and fixes the hang-up issue in the OP:

... but I think we'll want to be able to make that conditional somehow in case there are other places that would still benefit from it being parallel...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is not working as intended GUI Issues affecting the interactive GUI Performance Something's slower than it should be
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants