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

Cache listings of legacy cache dirs #2563

Merged
merged 1 commit into from
Nov 14, 2018

Conversation

HebaruSan
Copy link
Member

@HebaruSan HebaruSan commented Nov 2, 2018

Problem

In a comment in #2562, @7ranceaddic7 noted a significant performance degradation with the current master HEAD version compared to 1.25.3. I audited the changes since then and identified 3 possible causes. This is the first.

Cause

NetFileCache maintains a cachedFiles string array of files currently in the cache. This list is searched to find files matching a given module's download property.

When we consolidated the instance-specific caches in #2535, existing instance-specific cache folders were treated as a fallback if a file was not found in the main cache dir. However, the files in those fallback folders were not added to cachedFiles because I wasn't sure how best to do that or if it was necessary. Instead, those files were enumerated from the directories on the fly every time.

On closer inspection, this might cause a big slowdown because GUI checks IsMaybeCachedZip for every single mod on load, which implies O(n * m) disk operations, where n is the number of mods in the registry and m is the number of files in the legacy caches. If they were added to cachedFiles instead, then those disk accesses could be avoided and replaced with faster memory accesses.

Changes

Now we consolidate all of the files in all of the caches into the single cachedFiles array. It turns out this is much easier and simpler than what I did previously. The cache will work the same as before, except faster because it will no longer have to hit the disk for legacy cache dirs.

And as long as we're touching this code, cachedFiles is now a dictionary mapping from URL hashes to cache entries. This eliminates the need to loop through every entry and check whether it starts with our target hash, which should result in a general performance improvement for cache-related tasks.

@HebaruSan HebaruSan added Easy This is easy to fix Core (ckan.dll) Issues affecting the core part of CKAN Pull request labels Nov 2, 2018
@7ranceaddic7
Copy link

Additional information:

I just performed an update on a mod (BobsPanicBox). I have two 1.4.5 installs - one with one without MHE. While performing the update with CKANv1.25.4 and found the update process performance degraded as well. The CKANv1.25.3 performed the same update quickly.

I can test for actual time, but the performance differential was markedly noticeable.

@Olympic1
Copy link
Member

Olympic1 commented Nov 4, 2018

@7ranceaddic7 Did you got around to test this fix out?

@politas
Copy link
Member

politas commented Nov 14, 2018

Oooh, sounds good. I have noticed a significant performance hit since the One True Cache system was added. After every function that downloads files, it takes ages for it to get back to the modlist. And ok, maybe I don't need 1,270 items in my cache taking up a half a Terabyte...

@politas politas merged commit b3a5c98 into KSP-CKAN:master Nov 14, 2018
politas added a commit that referenced this pull request Nov 14, 2018
@HebaruSan HebaruSan deleted the fix/cache-legacy-caches branch November 14, 2018 08:43
@politas
Copy link
Member

politas commented Nov 14, 2018

Wow, that really sped up the cache delay! Now hardly noticeable!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Core (ckan.dll) Issues affecting the core part of CKAN Easy This is easy to fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants