Cost aware LFU - #595
Cost aware LFU#595rs-sac wants to merge 4 commits into
Conversation
No functional change:
- Move `EntrySizeAndFrequency` from `sync::base_cache` and
`future::base_cache` (duplicated) to `common::concurrent`
- Add `EvictionPolicyConfig::uses_frequency_sketch` instead of comparing
against `EvictionPolicyConfig::TinyLfu` directly
- Fix doc wording in `sync::builder` ("to the cache" -> "of the cache")
Add a `cost` closure to the `sync` and `future` cache builders, analogous to `weigher`, and store the resulting `policy_cost` in `EntryInfo`. The cost represents the relative cost of recomputing an entry's value and, unlike the policy weight, does not affect capacity accounting.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (6)
📝 WalkthroughWalkthroughThis PR introduces a cost-aware TinyLFU eviction policy variant that extends cache entries with a separate cost value for fine-grained admission decisions. Entry metadata, policy configuration, both async and sync cache implementations, builder APIs, and comprehensive tests are updated to support cost-weighted frequency aggregation during admission. ChangesCost-aware TinyLFU eviction policy
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #595 +/- ##
==========================================
+ Coverage 93.38% 93.45% +0.06%
==========================================
Files 44 44
Lines 17075 17283 +208
==========================================
+ Hits 15946 16151 +205
- Misses 1129 1132 +3 🚀 New features to boost your workflow:
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| /// `policy_cost` is the relative cost of recomputing the entry's value (for | ||
| /// example, the time it takes to load it). Unlike `policy_weight`, it does not | ||
| /// affect the cache's capacity accounting. It is only consulted by the | ||
| /// cost-aware eviction policy. Defaults to `1`. | ||
| policy_cost: AtomicU32, |
There was a problem hiding this comment.
🚩 Per-entry memory overhead: new AtomicU32 for policy_cost on every entry
Every EntryInfo now contains an additional policy_cost: AtomicU32 field (src/common/concurrent/entry_info.rs:42), adding 4 bytes per cache entry regardless of whether the cost-aware policy is used. For caches with millions of entries, this is a non-trivial memory increase. This is a trade-off that could be mitigated by conditionally including the field (e.g., via a separate entry type or an Option), but the current approach keeps the code simpler.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
fwiw, there was some research in this area that might be interesting. Sometimes the simple and obvious approach works, sometimes it doesn't. Most of the time, caches are configured and not monitored for tuning so I am always hesitant to give eviction algorithm knobs that may be worse if set. I think its also worthwhile for a cache to emphasize that users would need to monitor for the new goal (from hit rate to request latency) since its less intuitive about what good performance results are. |
Add `EvictionPolicy::cost_aware_lfu()`, a TinyLFU variant that weights each entry's sketch frequency by its policy cost (from the builder's `cost` closure), so that an entry that is accessed less often but is expensive to recompute can potentially be kept in favor of one that is accessed more often but is cheap to recompute. Admission compares the candidate's and victims' `frequency * cost` instead of frequency alone. With no `cost` closure configured, every entry has a cost of 1 and the policy behaves like plain TinyLFU. Over-capacity eviction is unchanged (LRU order). (Since the LFU-based policies rely primarily on admission-time size enforcement, I did not bother complicating the eviction code.)
2 participants
This series of patches contains a relatively simple variation on the existing LFU algorithm that compares a potential admission to existing cache items based on a weighted cost rather than straight frequency of use. The idea is that if the purpose of the cache is to reduce time spent computing, it may be the case that an item less frequently used is more worth caching if the time required to recompute its value is disproportionately greater.
(Concretely, if item A is used 1x/sec and item B is used 2x/sec, LFU would prefer B. But if A costs 10 to recompute and B costs 1 to recompute, then preferring A would save more computation.)
Thus, this new algorithm compares weighted costs rather than frequencies, where the weighted cost is the frequency multiplied by a cost function (similar to the existing weighing function).
Summary by CodeRabbit
New Features
cost()method to the cache builder to supply per-entry recomputation cost for the cost-aware policy.Tests
Documentation