Support weight-based eviction #24
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
- Add `total_weight` field and optional `max_weight` and `weighter` closure fields to `InnerCache`. - Rename `max_capacity` to `max_entries` and make it optional. - Update `sync` and child methods of `InnerCache` to check and update the `total_weight` value during processing inserts, updates, invalidations and evictions.
It is currently failing because bitflags v1.3.1 is incompatible with the MSRV. This issue will be addressed by another pull request.
future::Cache::get_or_try_insert_with_hash_and_fun
- Implement aggregated victims strategy for cost-based eviction. - Rename max_entry back to max_capacity, and remove an optional max_weight field from InnerCache.
- Rename an internal structure TotalWeight to TotalCost. - Lightly refactored.
- Rename many internal items, for example:
- TotalCost struct -> WeightedSize struct
- cost() method -> weigh() method
- cost variable -> policy_weight variable
- NOTE: Cost usually means the miss penalty in cache.
- Add EntrySizeAndFrequency struct for admit() method.
In order to reduce chances to pollute the cache main space, restore the old admission behavior, which requires the candidate frequency must be higher than the aggregated frequency of potential victims.
- Rename KeyValueEntry struct to KVEntry. - Updated WriteOp::Remove enum variant to store KVEntry.
- Fix a bug in admit method to go into an infinite loop when the LRU entry has been concurrently invalidated. - Also add a retry limit to admit method.
- Add `weighter` method to the `CacheBuilder`s. - Add unit tests for `sync::Cache` with weighter closure registered.
Replace Vec in AdmissionResult with SmallVec.
Copy a unit test size_aware_admission from sync::Cache to future::Cache.
ben-manes
reviewed
Aug 24, 2021
- Write docs. - Change the weigher type in the CacheBuilder::wegher method to take an impl trait rather than a trait object.
- Implement size-aware cache management to the unsync cache. - Rename `KVEntry` to `KvEntry`.
Disable Clippy until we figure out what to do on the following lint: https://rust-lang.github.io/rust-clippy/master/index.html#non_send_fields_in_send_ty Relates to #54 (cherry picked from commit 646beb0)
This will not compile. Conflicts: .github/workflows/CI.yml .vscode/settings.json CHANGELOG.md README.md src/sync/base_cache.rs src/unsync/cache.rs
Fix after merging the master branch. Note that added crossbeam-utils crate to the dependency for `AtomicCell` (using for tracking weighted size).
- Change `weigher`'s return type from `u64` to `u32`. - Refactor data types `ValueEntry`, `KeyDate` and `KeyHashDate` to reduce the number of `Arc` pointers used by adding `EntryInfo` struct.
Update the `weigher` examples to avoid `u32` overflows.
Implement weighted size handling in `sync::BaseCache` for when updating an existing cache entry.
Change `WriteOp` back to having `Upsert`.
Add `evict_lru_entries` to `sync::BaseCache`, which will evict LRU entries when cache's capacity is exceeded by updating existing entries with bigger weighted sizes.
Add `evict_lru_entries` to `unsync::Cache`, which will evict LRU entries when cache's capacity is exceeded by updating existing entries with bigger weighted sizes.
Rename `weighted_size` in `ValueEntry` to `policy_weight`.
Add `sync::ValueEntryBuilder` for `sync::BaseCache`.
Some clean up
Change `sync::ValueEntryBuilder` and `sync::entry_info::EntryInfo` from dynamic dispatch (by trait objects) to enum dispatch. A simple benchmark showed enum dispatch is slightly (a few %) faster than dynamic dispatch in our use case.
Finish implementing size-aware cache management on `unsync::Cache`.
Update `sync::SegmentedCache` to support weigher function.
Update the docs.
Adding finishing touch.
|
Finally, I have got some time to work on this PR. Now it is ready to merge. |
tatsuya6502
commented
Dec 31, 2021
There was a problem hiding this comment.
Merging.
Although all unit tests are passing and I have reviewed the diff several times, I am a bit worried for regressions because this PR is huge (+2,009 -539). I will not release this immediately, but will spend next few days to run additional tests to shake out some bugs.
This was referenced Jan 1, 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.
Edit on Dec 31, 2021
Finished implementing above features on the all caches:
future::Cachesync::Cachesync::SegmentedCacheunsync::CacheChanges
API:
CacheandSegmentedCachemax_capacity()fromusizetoOption<usize>.builder()method.CacheBuilderweighermethod to set the weigher closure.Internal Changes (Highlighting major changes):
sync::BaseCachedo_insert_with_hashmethod to calculate the policy weight of the entry using the weigher closure.sync::InnerCacheadmitandfind_cache_victimmethods to do the weight-based admission.evict_lru_entriesmethod to evict LRU entries when the weighed size exceeds the max capacity.sync::ValueEntryEntryInfoenum.EntryInfoenum to reduce the number ofArcusages:Notes