-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Cache] Improve the performance of tag invalidation #44233
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you run some real world benchs to confirm the issue is not only theoretical?
Because I have one concern, see comment :)
foreach ($tags as $tag) { | ||
\assert('' !== CacheItem::validateKey($tag)); | ||
unset($this->knownTagVersions[$tag]); | ||
$ids[] = $tag.static::TAGS_PREFIX; | ||
} | ||
|
||
return !$tags || $this->tags->deleteItems($ids); | ||
foreach ($this->tags->getItems($ids) as $tag => $version) { | ||
$tagsToUpdate[$tag] = $version->set($newVersion ?? $newVersion = random_int(\PHP_INT_MIN, \PHP_INT_MAX)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's an issue with this, which was also existing with the previous strategy but was fixed by always deleting items: unused tags are never removed and will last forever in the DB.
I experimented on my local machine with different configurations for TagAwareAdapter with RedisAdapter and FilesystemAdapter as a backend. I was focused on extra Surprisingly, the charts show that with non-zero TTL for known tag versions the rate of reads is also increased by ~1%. |
I have to admit I jumped to conclusions. My scenario is much more theoretical than realistic. The fact is that if many processes simultaneously access items with the same tags attached, then on every invalidation you can get overwrites of tag versions which cause recomputation of item values. Theoretically, you can get up to +100% to the number of computations of item values. In real use cases, it is a few percent more write operations and a bit fewer reads, but if tag invalidations happen not often, the difference is negligible. PS Please do not reference my charts, they represent a very specific scenario that bears no relation to reality. |
This PR is a follow-up to #44015.
The change of tag invalidation algorithm has impact on performance because of possible mutual overwrites of tags and calculated values. The degree of impact mainly depends on the number of concurrent processes which access the same keys so the issue is greatly mitigated by
LockRegistry
inget()
and the cache of known tag versions but in case of plain PSR-6 operations, the rate of writes may increase many times.The main purpose of changeing the algorithm was prevention of tag invalidation losses, and it do its job flawlessly, but for the sake of performance I think we should consider the hybrid algorithm: update tag versions if possible or delete them if the pool rejects updates. It looks like a good compromise to me.