TRAP Caching: Skip uploading of small caches #1265
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
edoardopirovano
force-pushed
the
edoardo/minimum-cache-size
branch
from
8893b68 to
62557ff
Compare
henrymercer
reviewed
Sep 23, 2022
edoardopirovano
force-pushed
the
edoardo/minimum-cache-size
branch
from
62557ff to
b98b2de
Compare
Since we haven't enabled this feature yet for any external users, nor made any promises about the specifics of its behaviour, I think it's reasonable to put in a change like this without another change note. Happy to add one if you disagree, though. |
henrymercer
approved these changes
Sep 23, 2022
This was referenced Sep 29, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Our telemetry shows that in a fair amount of cases we get some very small TRAP caches because the repository contains only a very small cacheable amount of the language being analyzed. In this case, we get slight performance regressions because the extra network calls just aren't worth doing for so little information. Let's set a minimum size of cache that we will consider worth uploading, so that we don't waste time uploading (and then redownloading) caches that are just too small to be worth it.
Empirically, 10MB is a good threshold for this because it is around 2MB more than the size of the TRAP for the JavaScript standard library, so this represents us actually caching some useful things beyond this. Other languages might have a different threshold for what is considered a big enough cache to be useful - we can explore that further (guided by our telemetry) once we get to implementing this feature for other languages.
There'll still be some small caches in the internal repos that had the feature flag on, but these should disappear on their own after around a week since that's the retention time of the GitHub Actions cache.
Merge / deployment checklist