Replace hard coded static cache of entities with cache backends

The last submitted patch, entitycache.patch, failed testing.

The last submitted patch, entitycache.patch, failed testing.

If you don't react before I'm getting back home, I'll try to fix your patch.

We can chain backends if we want both static and a real cache, sounds like an easy win: we could get rid of this awful loading logic that do both, and rely on a unique object with a clean interface that would do both at the same time. It's a pattern we could also re-use at various places in Drupal too.

Instanciating a single object, you won't notice it on your runtime, it's not significant, even with 10 or 20 different entity controllers, this won't hurt a bit your execution time, it's really nothing compared to what does a single Views, yet most people continue to put up to 10 Views on the same page.

In Drupal 7 the static cache is quite dumb, all you have is a cacheGet() and cacheSet() internal methods, which acts like a basic key/value store (if we omit the deprecated conditions array). If it is smarter than this, please enlighten me because I probably missed it.

Some other ideas arround the idea:
- We could, in the future, share cache chains to entity controllers that needs the same chain (we should then prepare some kind of mass deregistration mecanism in the array cache for static entries to act upon controller destructor)
- This pattern, with a clean cache chain definition mecanism, could be used in a lot of other places where the code actually emulate this behavior, for example a lot of info hooks

Using it massively would remove a lot of duplicated code everywhere, it might even be useful in conjunction with the actual DrupalCacheArray implementation that could evolve to use this base.

Using the chain of command (for saving), chain of responsability (for loading) and proxy patterns (basically using a chain towards the objects behind) for this seems really clean in term of architecture because the final object only rely on the cache interface, without worrying about what's behind, it's actually maintainer's even somehow.

Then, I'm not so sure about loosing the possibility to fetch from cache when loading by $conditions. Yep, $condition is deprecated, but we have not yet built the easy to use replacement aka entity_load_by_properties() or so. I could see having the helper using the same pattern as entity_load(), what would make it possible to specifically add at least static caching for frequently used queries.

I think that the conditions (or properties later) loading wouldn't be affected that much by replacing the internal cacheGet() with a backend directly: you still control what happens in your load() function and you are not forced to cache entities by id, you can do something else instead. You can still add a layer for frequently used queries in specific controllers.
But, to be honest, I think that trying to implement such layer would be more than difficult, the load() method may still be optimized later for very specific cases if needed. I even have a hunch that if the controller has direct access to its own cache backend, it would be easier to implement such frequently used queries caching: the controller knows its own schema and fields.

I'd prefer the approach taken by #1302378: Use multiple specialized entity controllers: Have a dedicated entity cache controller class, which is invoked by entity_load() and handles everything related to caches - see #1302378-4: Use multiple specialized entity controllers (comment #4).

The approach in #1302378: Use multiple specialized entity controllers suppose that there is an upper layer that needs to combine itself the cache and "main" controller, which means that the entry point of the API is in another piece of code (in the exemple in procedural code), I think this would be prone to errors: some developers will continue to hit the controllers directly (which makes a lot of sense) instead of calling entity_load(). I don't think this a good idea until there is a front controller for all entity operations.
I think that in the end, both provide the same feature and generalize caching method, but I prefer catch's method: it allows the entity "main" controller to implement its own load/save etc.. methods and have a finer control over its own cache if needed, while most of the others would probably continue to use the generic one.

We'll probably want some entity-type specific cache controller configuration, e.g. the cache bin to use. The entity cache controller would be the obvious place to take care of that.

Regarding specific entity type cache configuration, weither it's a cache controller or a chain set inside the main controller, I think both solutions are equivalent in term of configuration, especially if we can create the cache chain outside and inject it when building the controller instance: instead of creating a cache controller from configuration, we create a cache chain from configuration, doesn't seem too different in the end, it just moves some responsabilities.

And, latest note, I think that even if the cache controller is the way to go, this isntance can still benefit from using the cache chain catch is proposing, it's just a matter of moving it into another objet.

Having a permanent cache renders hook_entity_load() useless. If possible I'd prefer to drop it in the long road anyways, an entity really shouldn't be altered by page request. If it needs to be changed, re-save it with new property values.

There's a point I agree with is that we shouldn't alter this kind of stuff on every load, it can get quite CPU intensive if you have a lot of modules playing with this. But, in the other end, having the hook_entity_load() can be useful for some modules to set business data which are not properties, but that needs to be on the entity and cached along with it. Concrete use case: I could load data from business tables and set them on a node, but I don't want to store this data in actual node properties because I don't want to duplicate it and handle the full synchronization between my source business data and node properties. If I can still attach arbitrary data on the object I can then just handle nice synchronisation by clearing a few cache on my business table updates. Right now, I actually achieve this using the virtual_field module, that provides a null field storage backend, but defining some virtual fields allows me to actually store precomputed data in field cache along with my commerce products, line items and stuff, without worrying about a complete field storage layer.

EDIT: Sorry, a bit off topic, and that's an akward and quite ugly way of doing this, but in my case, it's mainly for denormalizing some data for performances. I don't know where the property system is going to right and if it's going to help me with those kind of use cases.

What matters in here, is that by caching entities, we cache the hook_entity_load() result with it, and that's why I really like the idea, because if we do this I can then remove my need for hacks such as virtual field etc.

Yes, I honestly think those kind of non queryable stuff are really ugly, but sometime you just don't really have other straightforward choices. I don't know what will be final API for properties, but properties could solve that somehow.

@fubhy that sounds just like http://drupal.org/sandbox/catch/1153584 which I've never got around to implementing. I agree that'd be a good idea but I think we should add a separate issue for it. I also want to break the ArrayCache backend into a separate patch probably since that's useful for unit testing (and we also need to look at re-organising the cache tests so they're more re-usable with alternate cache backends).

Not that off-topic, the cache chain interface is the idea behing this issue, and IMHO it should be done before we can actually continue this issue. It is a good reusable pattern that could be used at many places.

Opened #1637478: Add a PHP array cache backend.

A small changes here could have significant ramifications (for better or worse), so I am adding a tag as a reminder to check performance.

I nearly committed a very basic EntityCache controller for the Countries module before doing some simple tests to discover that this caused a 20 fold decrease in speed. On the plus side of this testing, reading in an array of Country entities from the database was 3 times faster than including iso.inc and doing a single drupal_alter() on the countries list. So performance gains could be significant here :)

Awesome. Now we can get this rolling again. I worked on this a bit during DCon Munich already but was not really satisfied with the outcome because it was still hard-wired with the storage controller. My wish would be to make the caching layer independent of the storage system. I am not entirely sure how we would approach that yet, though. I do have a crazy suggestion though which I have been playing around with in my head lately but it would be a bit drastic to say the least. Will post more about that soon.

#3 woops

a fast look to all the changes already in repository says this implementation very different against repo one so not possible for a fast reroll

Weird, I could swear the the tags list was empty when I added those tags...