-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Cache] Add LRU + max-lifetime capabilities to ArrayCache #35362
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
Conversation
e22800d
to
f4e7876
Compare
f4e7876
to
5df76ef
Compare
Added some suggestions, looks good 👍 Suggestion of LFU over LRU was because LFU in theory can have higher cache hits as the most frequently loaded items are kept in memory until they expire. However I'm unaware of a way to do that without involving array functions in some way so I think this is good approach for now and might be faster in some cases as there is less computation going on. So a CPU vs IO trade-off basically. |
a957b41
to
928b3f1
Compare
928b3f1
to
db2cf05
Compare
db2cf05
to
48a5d5e
Compare
…che (nicolas-grekas) This PR was merged into the 5.1-dev branch. Discussion ---------- [Cache] Add LRU + max-lifetime capabilities to ArrayCache | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | Deprecations? | no | Tickets | Fix https://github.com/orgs/symfony/projects/1#card-30686676 | License | MIT | Doc PR | - In #32294 (comment), @andrerom writes: > if you plan to expose use of ArrayAdapter to a wider audience you should probably also add the following features to it: > - max item limit to avoid reaching memory limits > - own (very low, like default 100-500ms) TTL for in-memory caching, as it's in practice stale data when used in concurrent scenarios > > If you want to be advance you can also: > > - keep track of use, and evict cache items based on that using LFU when reaching limit > - in-memory cache is domain & project specific in terms of how long it's somewhat "safe" to keep items in memory, so either describe when to use and not use on a per pool term, or allow use of pool to pass in flags to opt out of in-memory cache for cases developer knows it should be ignored This PR implements these suggestions, via two new constructor arguments: `$maxLifetime` and `$maxItems`. In Yaml: ```yaml services: app.lru150_cache: parent: cache.adapter.array arguments: $maxItems: 150 $maxLifetime: 0.150 framework: cache: pools: my_chained_pool: adapters: - app.lru150_cache - cache.adapter.filesystem ``` This configuration adds a local memory cache that keeps max 150 elements for 150ms on top of a filesystem cache. /cc @lyrixx since you were also interested in it. Commits ------- 48a5d5e [Cache] Add LRU + max-lifetime capabilities to ArrayCache
This PR was squashed before being merged into the 4.4 branch. Discussion ---------- [FrameworkBundle] Dont store cache misses on warmup | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix #38694 | License | MIT | Doc PR | symfony/symfony-docs#15172 When we are warming the annotation cache, we are reading all annotation into an `ArrayAdapter`. When we are done we move the values to a `PhpArrayAdapter` to store them in a file. @Seldaek [found out](#38694 (comment)) that when you are using a custom constraint with a `Symfony\Component\Validator\Constraints\Callback`, there is a strange error like: > Can use "yield from" only with arrays and Traversables That is because the `Closure` in the `Symfony\Component\Validator\Constraints\Callback` cannot be serialised and saved to cache. But since the `ArrayAdapter` is also [storing misses as null](#35362), the null values are understood as real values. When all values are moved to the `PhpArrayAdapter` and we ask the cache for a value (via Doctrine's `CacheProvider`), it will return `null` as a value instead of `false` as a cache miss. And `null` is not something one could "yield from". Commits ------- 27a22b3 [FrameworkBundle] Dont store cache misses on warmup
This PR was squashed before being merged into the 4.4 branch. Discussion ---------- [FrameworkBundle] Dont store cache misses on warmup | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix #38694 | License | MIT | Doc PR | symfony/symfony-docs#15172 When we are warming the annotation cache, we are reading all annotation into an `ArrayAdapter`. When we are done we move the values to a `PhpArrayAdapter` to store them in a file. @Seldaek [found out](symfony/symfony#38694 (comment)) that when you are using a custom constraint with a `Symfony\Component\Validator\Constraints\Callback`, there is a strange error like: > Can use "yield from" only with arrays and Traversables That is because the `Closure` in the `Symfony\Component\Validator\Constraints\Callback` cannot be serialised and saved to cache. But since the `ArrayAdapter` is also [storing misses as null](symfony/symfony#35362), the null values are understood as real values. When all values are moved to the `PhpArrayAdapter` and we ask the cache for a value (via Doctrine's `CacheProvider`), it will return `null` as a value instead of `false` as a cache miss. And `null` is not something one could "yield from". Commits ------- 27a22b34af [FrameworkBundle] Dont store cache misses on warmup
In #32294 (comment), @andrerom writes:
This PR implements these suggestions, via two new constructor arguments:
$maxLifetime
and$maxItems
.In Yaml:
This configuration adds a local memory cache that keeps max 150 elements for 150ms on top of a filesystem cache.
/cc @lyrixx since you were also interested in it.