Closed (won't fix)
Project:
Drupal core
Version:
8.0.x-dev
Component:
cache system
Priority:
Normal
Category:
Task
Assigned:
Reporter:
Created:
27 Sep 2010 at 22:20 UTC
Updated:
29 Jul 2014 at 19:03 UTC
It looks like we introduced a nightmare of cache_get()/cache_set() + drupal_static()/drupal_static_reset() combinations to D7 core.
So after working on all kind of related patches, I seriously wondered why we do not implement this cache->static-cache pattern at its root -- the database cache.
That way, other cache implementations (e.g., opcode variable store, memcache) don't have to use a giant static variable cache and can simply refetch the data from their storage.
In turn, we only have to care for what we always had to care for: Properly setting, getting, and clearing caches via cache_set(), cache_get(), and cache_clear_all().
| Comment | File | Size | Author |
|---|---|---|---|
| drupal.cache-db-static.0.patch | 4.02 KB | sun |
Comments
Comment #2
catchThis is won't fix for D7 - memcache requires network access, validating whether cache items are valid or not can be expensive - see #900436: Optimize memcache::valid(). Some items can be fetched from static cache many hundreds of times during a request (field info cache for example) - that would result in many, many hundreds or thousands of network requests. For those items where the item is fetched from cache hundreds of times during a request, even the difference between a static cache within the function, and instantiating the cache class could cause issues.
Having said that, it would be useful to consolidate this pattern somehow, so while I don't think this could be used generically, it might be an option sometimes. Moving to D8 so it can be discussed properly there.
Comment #3
David_Rothstein commentedSubscribing. Interesting idea, but we'd also have to be careful about the effect on memory usage. While it's probably true that most items which are cached in the database are also put in a static cache somewhere, it's not necessarily the case everywhere (e.g. expensive data that is only used once or twice per page request might not be statically cached), so forcing all that into a static cache could perhaps balloon the memory that Drupal uses if we're not careful.
Comment #4
sunI'm really struggling with the idea of having to use the current fragile and loosely coupled pattern of db/static caches for the entire D7 release cycle. Given the amount of cache invalidation problems I'm facing in contrib right now already, I don't really want to know what kind of nightmares I'm going to face when doing real sites based on D7.
I'd actually go as far and say that those db/static cache invalidation problems are going to ruin Drupal's reputation and adoption.
There's absolutely no pattern in how any cache can be reset. Some code uses a $reset flag, some other a separate _reset() function, and yet another doesn't provide anything at all, because stuff is reset upon another, entirely different reset. And of course, static cache ids and database cache ids may not even match. You need to read tons of code to understand what's going on behind the scenes.
Or you resort to calling drupal_flush_all_caches(), because, yes, it's the only thing that really gets the job done. If D7 stays that way, then we can be sure that contrib will turn D7 into a cache flushing mess.
@catch: If memcache is used over the network, then you might be right. However, there are still opcode cache variable stores (such as APC/XCache), which could happily serve cache items from their storage without having to bloat PHP memory. Either way, the idea is to leave that decision up to the cache controller, which you can happily replace with whatever makes sense.
@David: That's as simple as adding an optional
$static = TRUEargument to cache_get(), which then passes on the flag/option to the cache controller.Comment #5
catchThis can be Drupal 8, or it can be a duplicate of #636454: Cache tag support, or for that matter #581626: Use a consistent/clean pattern for using $reset or drupal_static(). Both of these were moved to D8 long ago, for good reason.
Benchmarks, then.
http://drupal.org/project/apc and http://drupal.org/project/memcache both have D7 versions, introducing an API change here has real world effects on those modules, we are past the point of doing this just because it might be a nice idea.
Comment #6
damien tournoud commentedStatic caching is intra-request, caching is inter-request. We already discussed this at length while introducing the drupal_static() pattern, and concluded that mixing the two is not a good idea and can have some deeply-rooted consequences.
This is not a new idea, and it's definitely not something we should do so late in this cycle.
Comment #7
sunNot sure whether this was mainly pushed back by the point in time of the release cycle, but it might not have been a super idea in the first place, although I'm really not sure.