Speed up autoremove search

[HebaruSan]

Problem

If you install many mods (200+) and click the checkbox to queue up one more to install, GUI takes a while to respond. I observed about 4.5 seconds with 282 mods, and a user reports 10-15 seconds with 363 mods.

Causes

A few Console.WriteLine statements revealed the following upon clicking:

2019-08-27T00:36:28.1441350+00:00 UpdateChangeSetAndConflicts
2019-08-27T00:36:28.1441350+00:00 ComputeUserChangeSet
2019-08-27T00:36:29.2111960+00:00 ComputeUserChangeSet after FindRemovableAutoInstalled
2019-08-27T00:36:29.2131961+00:00 UpdateChangeSetAndConflicts after ComputeUserChangeSet
2019-08-27T00:36:29.2131961+00:00 ComputeChangeSetFromModList
2019-08-27T00:36:29.2141962+00:00 ComputeChangeSetFromModList before FindReverseDependencies
2019-08-27T00:36:29.2141962+00:00 ComputeChangeSetFromModList before FindRemovableAutoInstalled
2019-08-27T00:36:30.2972581+00:00 ComputeChangeSetFromModList before RelationshipResolver
2019-08-27T00:36:30.3072587+00:00 ComputeChangeSetFromModList after RelationshipResolver
2019-08-27T00:36:30.3082587+00:00 ComputeChangeSetFromModList end
2019-08-27T00:36:30.3112589+00:00 UpdateChangeSetAndConflicts end

2019-08-27T00:36:30.3122590+00:00 UpdateChangeSetAndConflicts
2019-08-27T00:36:30.3122590+00:00 ComputeUserChangeSet
2019-08-27T00:36:31.4493240+00:00 ComputeUserChangeSet after FindRemovableAutoInstalled
2019-08-27T00:36:31.4503241+00:00 UpdateChangeSetAndConflicts after ComputeUserChangeSet
2019-08-27T00:36:31.4503241+00:00 ComputeChangeSetFromModList
2019-08-27T00:36:31.4503241+00:00 ComputeChangeSetFromModList before FindReverseDependencies
2019-08-27T00:36:31.4503241+00:00 ComputeChangeSetFromModList before FindRemovableAutoInstalled
2019-08-27T00:36:32.6093904+00:00 ComputeChangeSetFromModList before RelationshipResolver
2019-08-27T00:36:32.6143907+00:00 ComputeChangeSetFromModList after RelationshipResolver
2019-08-27T00:36:32.6153907+00:00 ComputeChangeSetFromModList end
2019-08-27T00:36:32.6153907+00:00 UpdateChangeSetAndConflicts end

Apparent problems:

• UpdateChangeSetAndConflicts is called twice, only once is needed
• FindRemovableAutoInstalled takes 1-1.5 seconds each time it is called (4x); this is most of the slowness

Looking at FindRemovableAutoInstalled reveals several opportunities for improvements:

CKAN/Core/Registry/Registry.cs

Lines 1131 to 1142 in a162c8b

	private static IEnumerable<InstalledModule> FindRemovableAutoInstalled(
	IEnumerable<InstalledModule> installedModules,
	IEnumerable<string> dlls,
	IDictionary<string, UnmanagedModuleVersion> dlc
	)
	{
	var instCkanMods = installedModules.Select(im => im.Module);
	// ToList ensures that the collection isn't modified while the enumeration operation is executing
	return installedModules.ToList().Where(
	im => FindReverseDependencies(new List<string> { im.identifier }, instCkanMods, dlls, dlc)
	.All(id => installedModules.First(orig => orig.identifier == id).AutoInstalled));
	}

• FindReverseDependencies is called on all installed modules, but it only needs to be called on auto-installed modules
• There's a linear array search for matching identifiers in the check that all rev deps are auto-installed (the .All call)
• FindReverseDependencies traverses the whole reverse dependency tree, but we only care about enough entries to find one that isn't auto-installed

Changes

• Now UpdateChangeSetAndConflicts is only called once per click
• Now FindReverseDependencies uses IEnumerable<> and yield return to generate its results lazily, so it will not traverse the entire reverse dependency tree if the calling code doesn't need it, avoiding costly FindUnsatisfiedDepends calls
• Now FindRemovableAutoInstalled only checks FindReverseDependencies for auto-installed modules, which should be a small fraction of the total
• Now FindRemovableAutoInstalled's linear array search is replaced with an IsSupersetOf call to take advantage of FindReverseDependencies's new lazy evaluation (it will short circuit as soon as we find one rev dep that isn't auto-installed)

In my test case, these changes brought the time to process a click from 4.5 seconds to 0.25 seconds, or about a 95% reduction. Mileage will vary with the details of installed mod lists.

@shdwlrd, if you'd like to try a test build with these changes:

• [link removed, download latest CKAN version]

Fixes #2846.

[DasSkelett]

Why use IEnumerables instead of explicit types?

[HebaruSan]

That's the only way I know to use yield return to implement a lazily evaluated sequence. If I change it back to HashSet:

See also: https://stackoverflow.com/a/10792443/2422988

[DasSkelett]

I see, some weird Microsoft philosophy.
Can't understand why they don't allow a signature with return types implementing the interface, I mean that's essentially what will be returned in the end, right?

[HebaruSan]

I mean that's essentially what will be returned in the end, right?

No, this function now never generates an object with a collection inside of it. It runs pieces of the code inside the function to return each element as it's requested. If the calling code doesn't try to access the full sequence, then it won't be generated.

[DasSkelett]

Can confirm huuge performance improvements, and in addition, everything still works.

[shdwlrd]

Thank you very much for finding and correcting the problem. CKAN is running much faster now.

[HebaruSan]

@shdwlrd, thanks for calling this problem to our attention!