Skip to content

[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

Merged
merged 1 commit into from
Jan 27, 2020

Conversation

nicolas-grekas
Copy link
Member

@nicolas-grekas nicolas-grekas commented Jan 16, 2020

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:

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.

@andrerom
Copy link
Contributor

andrerom commented Jan 21, 2020

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.

@nicolas-grekas nicolas-grekas force-pushed the cache-lru branch 2 times, most recently from a957b41 to 928b3f1 Compare January 24, 2020 15:05
nicolas-grekas added a commit that referenced this pull request Jan 27, 2020
…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
@nicolas-grekas nicolas-grekas merged commit 48a5d5e into symfony:master Jan 27, 2020
@nicolas-grekas nicolas-grekas deleted the cache-lru branch February 5, 2020 12:30
@nicolas-grekas nicolas-grekas modified the milestones: next, 5.1 May 4, 2020
@fabpot fabpot mentioned this pull request May 5, 2020
nicolas-grekas added a commit that referenced this pull request Apr 1, 2021
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
symfony-splitter pushed a commit to symfony/framework-bundle that referenced this pull request Apr 1, 2021
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants