Fix very slow response to checkbox changes #2354
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
Checking or unchecking a checkbox in the main mod list can be very slow, sometimes taking over a minute! Some slowness was observed previously, but it got worse with v1.24.0.
Causes
My investigation found three sources of slowness.
Redundant ZIP validation
This line took 8+ seconds in one test that I observed, while everything else was quick:
CKAN/GUI/MainChangeset.cs
Lines 49 to 54 in 624839f
The culprit is
IsCachedZip
. This function performs validation of the ZIP file in the cache for the given module, including CRC checks. This could easily take several seconds for a large download. As of #2277,Main.UpdateChangesDialog
was running it for every mod listed in the (un)installation confirmation dialog so it could determine whether to tell the user the mod was already "(cached)".If the mod isn't in the cache, this call is fast. That's why the problem seemed more severe for uninstallation: if you're uninstalling a mod, that means it's installed, which means it's probably in your cache.
Double reaction to clicks
Relatedly, all of the post-click work is being done twice. When the user clicks,
ModList_CellValueChanged
callsGUIMod.SetInstallChecked
, which setsDataGridViewCheckBoxCell.Value
, which callsModList_CellValueChanged
again! It stops after two iterations becauseGUIMod.IsInstallChecked
is set and checked, but each time through,ModList_CellValueChanged
recalculates all the dependencies and conflicts. So the baseline response time for clicking a checkbox is twice as long as it should be.Slow lookups in
Registry.LatestAvailableWithProvides
This function is responsible for the core of a lot of CKAN's relationship logic. It takes as its parameters a module identifier and two constraints, a game version and a relationship specification. Its job is to return every
CkanModule
that could reasonably satisfy the given requirement (but only the latest one per identifier).Unfortunately it works by searching the entire registry for modules compatible with the given version, then exhaustively checking every one of them for a matching
"provides"
property. A typical call to this function takes 400 milliseconds, and we need a lot of them for many common operations. For example, if you click to install USI-LS, you'll see around 3 seconds of delay while the registry is searched and searched again for each of its 4 dependencies.Changes
Fixes #2310.
Less ZIP validation
Now we use
IsMaybeCachedZip
instead. This function just checks whether a file associated with the requested URL exists in the cache, without running the extra validation checks. This makes more sense now that we prevent invalid files from entering the cache as of #2243.The same change is made for the same reasons to a similar call in
UpdateRecommendedDialog
, affecting recommended and suggested mods, and forModuleInstaller
's installation confirmation prompt.Single reaction to clicks
GUIMod.SetInstallChecked
no longer setsDataGridViewCheckBoxCell.Value
if it already has the desired value.SetInstallChecked
is a bit funny in that the source of its input is not clearly defined; it might be called to react to a change in the grid (when all that should happen is that we setGUIMod.IsInstallChecked
), or it might be called to initiate a change to the grid (whenDataGridViewCheckBoxCell.Value
needs to be changed). The behavior now should be better in the reactive case without harming the initiating case.Index the
"provides"
propertyNow instead of exhaustively searching the registry in
LatestAvailableWithProvides
, we precalculate an index for it to use when the registry is first initialized. This index contains a mapping from provided identifiers to providingAvailableModules
. When we need to look up a provided module, the index lets us jump to the specific set ofAvailableModules
that we need to consider, and we simply ask them to provide us their latest modules matching our constraints. We then filter these to ensure that the latest version actually provides our target identifier, in case their"provides"
relationships have since been modified to no longer do so.This change reduces the typical run time of
LatestAvailableWithProvides
from 400 milliseconds to essentially 0. There are positive knock-on effects throughout the code base, wherever this function is used:Recommendations obey version requirements
Building on these changes, since
LatestAvailableWithProvides
now always obeys the version properties of a relationship, the GUI code for optional dependencies is updated to pass the specific versions found based on those relationships to the installer. This is done by tracking suggested and recommended modules as fullCkanModule
objects instead of string identifiers, which also saves a few redundant lookups here and there.This is also fixed in ConsoleUI, which now also shows the versions of optional dependencies.
Fixes #2319.
Other changes
A bunch of the mod list code in
Main.cs
is moved toMainModList.cs
to make it easier to find when investigating this sort of thing. No functional changes are made to this code.GUIUser
is split out fromMain.cs
into its ownGUIUser.cs
file. No functional changes here either.The GUI change preview screen could give wrong versions for dependencies, because we were using
GUIMod.Version
, which is always the latest compatible version. Now it's fixed by usingCkanModule.version
instead, which is the true version being installed.