Repository navigation
Make eviction strategy in dir cache link-aware #3641
There are no files selected for viewing
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,9 +15,9 @@ import ( | |
| "slices" | ||
| "strings" | ||
| "sync" | ||
| "syscall" | ||
| "time" | ||
|
|
||
| "github.com/dustin/go-humanize" | ||
|
|
||
| "github.com/thought-machine/please/src/clean" | ||
|
|
@@ -182,8 +182,7 @@ func (cache *dirCache) storeFile(target *core.BuildTarget, out, cacheDir string) | |
| } | ||
| // TODO(peterebden): This is a little inefficient, it would be better to track the size in | ||
| // RecursiveCopy rather than walking again. | ||
| return unsharedSize(cachedFile) | ||
| } | ||
|
|
||
| func (cache *dirCache) Retrieve(target *core.BuildTarget, key []byte, outs []string) bool { | ||
|
|
@@ -210,11 +209,15 @@ func (cache *dirCache) retrieve(target *core.BuildTarget, key []byte, suffix str | |
| } | ||
|
|
||
| func (cache *dirCache) retrieveFiles(target *core.BuildTarget, cacheDir string, outs []string) (bool, error) { | ||
| info, err := os.Stat(cacheDir) | ||
| if os.IsNotExist(err) { | ||
| log.Debug("%s: %s doesn't exist in dir cache", target.Label, cacheDir) | ||
| return false, nil | ||
| } else if err != nil { | ||
| return false, err | ||
| } | ||
| cache.markDir(cacheDir, 0) | ||
| cache.touch(cacheDir, info) | ||
| if len(outs) == 0 { | ||
| return true, nil | ||
| } | ||
|
|
@@ -379,98 +382,175 @@ func newDirCache(config *core.Configuration) *dirCache { | |
| return cache | ||
| } | ||
|
|
||
| // Period of time in seconds between which two artifacts are considered to have been used at the same time. | ||
| const accessTimeGracePeriod = 600 // Ten minutes | ||
|
|
||
| // touchInterval is the minimum time between updates of an entry's modification time when it's retrieved. | ||
| // Updating it on every retrieval would cause a lot of unnecessary writes. | ||
| const touchInterval = time.Hour | ||
|
|
||
| // A cacheEntry represents a single entry in the cache (i.e. one stored target). | ||
| type cacheEntry struct { | ||
| Path string | ||
| Size uint64 // Total size of the files in this entry that are not linked from outside the cache | ||
| Mtime int64 | ||
| Files []fileID | ||
| } | ||
|
|
||
| // A fileID identifies a file on disk independently of the path(s) it's linked at. | ||
| type fileID struct { | ||
| Dev, Ino uint64 | ||
| } | ||
|
|
||
| // A cacheFile is a single file in the cache, which may be linked into multiple entries | ||
| // and also from outside the cache (typically into plz-out, when it's stored or retrieved). | ||
| type cacheFile struct { | ||
| Size uint64 | ||
| Links uint64 // Total number of hard links to this file | ||
| Refs uint64 // Number of those links that are inside the cache | ||
| Unshared bool // True if this file is only linked from inside the cache. | ||
| } | ||
|
|
||
| // fileInfo returns the identifier and number of hard links for a file. | ||
| func fileInfo(info os.FileInfo) (fileID, uint64) { | ||
| st := info.Sys().(*syscall.Stat_t) | ||
| // The types of these fields vary between platforms, so the conversions are needed on some of them. | ||
| return fileID{Dev: uint64(st.Dev), Ino: uint64(st.Ino)}, uint64(st.Nlink) //nolint:unconvert | ||
| } | ||
|
|
||
| // unsharedSize returns the total size of the files under the given path that are not hard linked from anywhere else. | ||
| func unsharedSize(path string) uint64 { | ||
| var size uint64 | ||
| filepath.Walk(path, func(path string, info os.FileInfo, err error) error { | ||
| if err == nil && !info.IsDir() { | ||
| if _, links := fileInfo(info); links <= 1 { | ||
| size += uint64(info.Size()) | ||
| } | ||
| } | ||
| return nil | ||
| }) | ||
| return size | ||
| } | ||
|
|
||
| // readEntry reads a single entry in the cache, recording all the files in it. | ||
| func readEntry(path string, files map[fileID]*cacheFile) (*cacheEntry, error) { | ||
| info, err := os.Lstat(path) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| entry := &cacheEntry{Path: path, Mtime: info.ModTime().Unix()} | ||
| if err := filepath.Walk(path, func(path string, info os.FileInfo, err error) error { | ||
| if err != nil { | ||
| return err | ||
| } else if info.IsDir() { | ||
| return nil | ||
| } | ||
| id, links := fileInfo(info) | ||
| if _, present := files[id]; !present { | ||
| files[id] = &cacheFile{Size: uint64(info.Size()), Links: links} | ||
| } | ||
| files[id].Refs++ | ||
| entry.Files = append(entry.Files, id) | ||
| return nil | ||
| }); err != nil { | ||
| return nil, err | ||
| } | ||
| return entry, nil | ||
| } | ||
|
|
||
| // clean runs background cleaning of this cache until the process exits. | ||
| // Returns the total size of the cache after it's finished. | ||
| // Sizes only count files that are not hard linked from outside the cache; those are typically also in someone's | ||
| // plz-out, and so removing them would not free any space (and would make it less likely for that file to be shared | ||
| // with the next repo that builds it). | ||
| func (cache *dirCache) clean(highWaterMark, lowWaterMark uint64) uint64 { | ||
| entries := []*cacheEntry{} | ||
| files := map[fileID]*cacheFile{} | ||
| var totalSize uint64 | ||
| if err := fs.Walk(cache.Dir, func(path string, isDir bool) error { | ||
| if !cache.shouldClean(filepath.Base(path), isDir) { | ||
| return nil // nothing particularly to do for other entries | ||
| } | ||
| if size, marked := cache.isMarked(path); marked { | ||
| totalSize += size | ||
| } else if entry, err := readEntry(path, files); err != nil { | ||
| // This can happen if another process is cleaning concurrently; it's not fatal to us. | ||
| log.Warning("Failed to read cache entry %s: %s", path, err) | ||
| } else { | ||
| entries = append(entries, entry) | ||
| } | ||
| if !cache.Compress { | ||
| return filepath.SkipDir // Already handled | ||
| } | ||
| return nil // Need to keep walking if we are dealing with compressed files | ||
| }); err != nil { | ||
| log.Error("error walking cache directory: %s\n", err) | ||
| return totalSize | ||
| } | ||
| for _, f := range files { | ||
| if f.Unshared = f.Links <= f.Refs; f.Unshared { | ||
| totalSize += f.Size | ||
| } | ||
| } | ||
| // Entries with no unshared files would free nothing if removed, so are never candidates for cleaning. | ||
| candidates := entries[:0] | ||
| for _, entry := range entries { | ||
| for _, id := range entry.Files { | ||
| if f := files[id]; f.Unshared { | ||
| entry.Size += f.Size | ||
| } | ||
| } | ||
| if entry.Size > 0 { | ||
| candidates = append(candidates, entry) | ||
| } | ||
| } | ||
| log.Info("Total cache size: %s", humanize.Bytes(totalSize)) | ||
| if totalSize < highWaterMark { | ||
| return totalSize // Nothing to do, cache is small enough. | ||
| } | ||
| // OK, we need to slim it down a bit. We implement a simple LRU algorithm. | ||
| slices.SortFunc(candidates, func(a, b *cacheEntry) int { | ||
| if diff := a.Mtime - b.Mtime; diff > -accessTimeGracePeriod && diff < accessTimeGracePeriod { | ||
| return cmp.Compare(b.Size, a.Size) | ||
| } | ||
| return cmp.Compare(a.Mtime, b.Mtime) | ||
| }) | ||
| for _, entry := range candidates { | ||
| if _, marked := cache.isMarked(entry.Path); marked { | ||
| continue | ||
| } | ||
|
|
||
| log.Debug("Cleaning %s, used %s, saves up to %s", entry.Path, humanize.Time(time.Unix(entry.Mtime, 0)), humanize.Bytes(entry.Size)) | ||
| if err := cache.cleanPath(entry.Path); err != nil { | ||
| log.Warning("Error while cleaning cache: %s", err) | ||
| continue | ||
| } | ||
| // A file is only freed once the last entry referring to it is removed. | ||
|
Contributor
There was a problem hiding this comment. Reference counting requires a global lock. Does this mean we can't merge this until #3644 is merged?
Collaborator
Author
There was a problem hiding this comment. Nope that's about concurrency across processes. The reference counts here are all in memory, we've collected them all into this map. |
||
| for _, id := range entry.Files { | ||
| f := files[id] | ||
| f.Refs-- | ||
| if f.Refs == 0 && f.Unshared { | ||
| totalSize -= f.Size | ||
| } | ||
| } | ||
| if totalSize < lowWaterMark { | ||
| break | ||
| } | ||
| } | ||
| return totalSize | ||
| } | ||
|
|
||
| // touch updates the modification time of a cache entry to mark it as recently used. | ||
| func (cache *dirCache) touch(path string, info os.FileInfo) { | ||
| if time.Since(info.ModTime()) < touchInterval { | ||
| return // Updated recently enough already | ||
| } | ||
| now := time.Now() | ||
| if err := os.Chtimes(path, now, now); err != nil { | ||
| log.Debug("Failed to update modification time of %s: %s", path, err) | ||
| } | ||
| } | ||
|
|
||
| func (cache *dirCache) cleanPath(path string) error { | ||
| lockFile, err := core.AcquireExclusiveFileLock(path + ".lock") | ||
| if err != nil { | ||
|
|
||
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel a slight unease with a function called
readEntrywriting to one of its input, but maybe this is just a functional programming pet peeves of mine.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah it's just not very easy to figure out a better way of handling it. Because things can error partway an iterator doesn't work super well either.