#638070: Router loaders causing a lot of database hits for access checks and #636992: Entity loading needs protection from infinite recursion began to tackle the problem of excessive calls to user_load, but RDF module is still calling it for every node view, meaning a very serious performance hit by default.
If at all possible, we should use the account info already loaded in the node or comment object
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | rdf_load.patch | 5.44 KB | catch |
Comments
Comment #1
catchSubscribing.
Comment #2
catchTurned out to be exceptionally easy. #636992: Entity loading needs protection from infinite recursion still makes sense in principle, but this way is cleaner and much more performant for single nodes, as well as much better on memory with lots of comments for the specific case of RDF.
Comment #3
scor commentedThe reason why we had user_load was to give a chance to other module to alter the RDF mappings, but let's leave this option out for performance sake.
Comment #4
pwolanin commentedIn combination with #638070: Router loaders causing a lot of database hits for access checks this reduces to zero the calls to user_load on /node and /node/1 (including comments)
Comment #5
catchIf we consider that a regression, we could possibly add hook_rdf_mapping_alter() in a followup - would be a new hook, but no change to existing code.
Comment #6
scor commented@catch: since these mappings are cached in entity_info, we decided not to introduce hook_rdf_mapping_alter() because you can do it via hook_entity_info_alter(). The purpose of calling user_load was to be able to alter the mapping on a user basis (as opposed to entity type or bundle basis) so that modules can change the mapping based on some logic (user logged in, etc.).
Comment #7
catchAhh I see. In that case I think it's too much of a performance hit to allow for that flexibility. If someone desperately needed it, they could preprocess_username() themselves.
Comment #8
scor commented@catch: exactly. agreed.
Comment #9
webchickI definitely like this much better. Committed to HEAD.
I asked catch on IRC if it made sense to revert #636992: Entity loading needs protection from infinite recursion, but he said it's still valuable, since another module might genuinely need to load user objects into nodes (or vice versa, or similar). I think that makes sense, and likely CCK node reference/user reference module are going to need to make use of it. It also makes the entity and field APIs parallel, which is good since they're more than hard enough to learn already. ;)