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

proposal: cmd/go: make build cache trimming cutoff configurable #69879

Closed
matthalp opened this issue Oct 14, 2024 · 15 comments
Closed

proposal: cmd/go: make build cache trimming cutoff configurable #69879

matthalp opened this issue Oct 14, 2024 · 15 comments
Milestone

Comments

@matthalp
Copy link

Proposal Details

The current policy assumes that caching is being used by a single developer. However, there are scenarios where the cache is shared across multiple developers and the cache size accumulation can have meaningful impact.

For example, CI where there can be many developers contributing code in a single day and leveraging a shared cache (e.g. GitHub Actions Cache where there is a cache size limit and also time spent loading and saving the cache). In these cases, 5 days of cache accumulation can have meaningful impacts.

Thank you for your consideration!

@gopherbot gopherbot added this to the Proposal milestone Oct 14, 2024
@matthalp
Copy link
Author

Another option that just came to mind is to expose cache trimming on the tool chain to be able to explicitly call it with various some configurability as well.

@ianlancetaylor ianlancetaylor moved this to Incoming in Proposals Oct 14, 2024
@seankhliao seankhliao changed the title proposal: cmd/go: make cache trimming cutoff configurable proposal: cmd/go: make build cache trimming cutoff configurable Oct 14, 2024
@seankhliao
Copy link
Member

@gabyhelp
Copy link

Related Issues and Documentation

(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)

@matthalp
Copy link
Author

@ianlancetaylor @matloob I wanted to put this on your radar. I like the GODEBUG variable idea mentioned in #69565 (comment)

P.S. @ianlancetaylor you were my go code readability reviewer in another life 👋

@matloob
Copy link
Contributor

matloob commented Nov 12, 2024

@matthalp Have you looked at GOCACHEPROG (it is currently not enabled by default but there's an accepted proposal to do so in 1.24)? You could use it to implement a cache with a custom policy.

@matthalp
Copy link
Author

Thank you @matloob! I had not seen that before.

I can look into a custom cache implementation, but I feel like my use case -- as well as the one in #69565 -- would be enabled by making variables configurable in some way. Is that something you would be open to?

From https://cs.opensource.google/go/go/+/refs/tags/go1.23.3:src/cmd/go/internal/cache/cache.go;l=334-335

//
// We set the mtime on a cache file on each use, but at most one per mtimeInterval (1 hour),
// to avoid causing many unnecessary inode updates. The mtimes therefore
// roughly reflect "time of last use" but may in fact be older by at most an hour.
//
// We scan the cache for entries to delete at most once per trimInterval (1 day).
//
// When we do scan the cache, we delete entries that have not been used for
// at least trimLimit (5 days). Statistics gathered from a month of usage by
// Go developers found that essentially all reuse of cached entries happened
// within 5 days of the previous reuse. See golang.org/issue/22990.
const (
	mtimeInterval = 1 * time.Hour
	trimInterval  = 24 * time.Hour
	trimLimit     = 5 * 24 * time.Hour
)

@matloob
Copy link
Contributor

matloob commented Dec 9, 2024

@matthalp I don't think that's something we'd want to do unless we had a strong reason for it. Each additional bit of configuration adds complexity to the command and we'd need to have a strong reason to add it to balance out the complexity.

@rittneje
Copy link
Contributor

rittneje commented Feb 2, 2025

@matloob I don't think people should have to create their own GOCACHEPROG (which is fairly complicated) for something as simple as wanting to adjust the trimLimit.

@rsc rsc moved this from Incoming to Active in Proposals Feb 5, 2025
@rsc
Copy link
Contributor

rsc commented Feb 5, 2025

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@rsc
Copy link
Contributor

rsc commented Feb 13, 2025

I don't think we want to take on the complexity of making this configurable. Why not run 'go clean -cache' once a day?

@danp
Copy link
Contributor

danp commented Feb 13, 2025

For example, CI where there can be many developers contributing code in a single day and leveraging a shared cache

In this context, is something like find $(go env GOCACHE) -type f -mmin +90 -delete before saving the cache sufficient?

That should (I think) trim anything that wasn't used by whatever the CI/etc run had done on the cache it had.

@rittneje
Copy link
Contributor

Why not run 'go clean -cache' once a day?

@rsc That would not allow having the cache last longer than trimLimit (5 days), so it doesn't help with the kind of situation I described in #69565.

@seankhliao seankhliao added the GoCommand cmd/go label Feb 22, 2025
@aclements
Copy link
Member

Unfortunately, the go command already has too many knobs. Given that you can already trim the cache more frequently, we're not going to add this knob. We can consider #69565 (disabling the limit) separately.

@aclements
Copy link
Member

Based on the discussion above, this proposal seems like a likely decline.
— aclements for the proposal review group

@aclements aclements moved this from Active to Likely Decline in Proposals Feb 26, 2025
@aclements
Copy link
Member

No change in consensus, so declined.
— aclements for the proposal review group

@aclements aclements moved this from Likely Decline to Declined in Proposals Mar 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Declined
Development

No branches or pull requests

9 participants