[volumecache 06/12] Index and evict disk cache entries - #376
Conversation
fe14d86 to
545360a
Compare
545360a to
d56be02
Compare
d56be02 to
61bc5ac
Compare
| if (m_currentDiskBytes >= it->second.bytes) { | ||
| m_currentDiskBytes -= it->second.bytes; | ||
| } else { | ||
| m_currentDiskBytes = 0; |
There was a problem hiding this comment.
Maybe would also want to flag for a disk index rebuild here
| // Tracks the root the in-memory index was last built against (used to | ||
| // decide when a rebuild is needed). | ||
| // Set to m_cacheDir when the in-memory disk index is built, such as on startup. | ||
| // Clear this value to signal that the in-memory disk index is no longer valid. | ||
| std::string m_diskIndexRoot; |
There was a problem hiding this comment.
I feel slightly weird about m_diskIndexRoot being both the root of the disk index and a flag for when it needs to be rebuilt... If it's only ever set to m_cacheDir when active, could it just be a boolean flag for needsRebuild? Or is the concern that m_cacheDir might change from under it?
| std::scoped_lock lock(m_mutex); | ||
| evictIfNeededLocked(0); |
There was a problem hiding this comment.
Nit: Should this be wrapped in a scope, in case this function is ever extended?
| if (!std::filesystem::exists(root)) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
Should this throw an error or log a warning?
| // On Windows this can fail if another thread (e.g. loadFromDisk) | ||
| // still holds tensorstore file handles into the same path. The | ||
| // bookkeeping has already been updated to reflect eviction; the | ||
| // stale files will be picked up by the next clearDiskCache. |
There was a problem hiding this comment.
When is clearDiskCache called? If it has to be manually triggered by the user, is it possible for the files to persist through multiple sessions with AGAVE and continually expand the cache?
Stacked draft PR 6 of 12 for the refreshed volume cache work.
This PR adds cold-start disk index loading, disk byte accounting, and LRU-style eviction when writes would exceed the configured disk cap.
The key thing here is that when we start up agave, we read the disk cache so that we have an in-memory index of it.