user_load() static caching
| Project: | Drupal |
| Version: | 7.x-dev |
| Component: | user system |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | patch (code needs work) |
The $user object is created in one of two different ways in Drupal:
1. It is created by session.inc during bootstrap. The user 'load' op is not called here.
2. It is created by a call to user_load(). The user 'load' op is called here. This happens semi-randomly throughout the codebase, e.g. when a node author is loaded, when a user page is viewed or edited, when a user logs in or out.
The reason for this is optimization killes and I agree that it's not necessary to do a full user_load on every request.
The problem is that module writers can't be guaranteed that the $user object they are working with is a "fully loaded" $user object, as Chris Johnson notes.
The current solution is to call user_load() yourself if you want to be sure. This is expensive because user_load has no caching.
It would be a great advantage to be able to test whether user_load has already run or not. One approach would be to implement the user hook for your module and set a flag there, then test it later. But that's expensive too. So here's my proposal: a one-line patch that simply records that user_load() has run:
<?php
$user->user_loaded = TRUE;
?>Now modules can simply check for the presence of $user->user_loaded.
| Attachment | Size |
|---|---|
| userloadjv.patch | 515 bytes |
#1
Would it be easier if modules called user_load as normal, but the user_load function had a static variable that was set if it had already been run and just returned $user?
I agree that this is a problem.
#2
shoudl get fixed in HEAD first.
#3
yes, a static var cache like node_load() seems best. any eaosn not to go that route?
#4
How will you determine whether the calling function intends the 'load' hook to be fired or not? To be consistent with node_load, the load hook would only be fired the first time, and will require a change to the user_load function signature to reset the cache.
#5
yes, thats what i had in mind.
the only place where we load user info without firing hook_user(load) is during session creation and i agree that that doesn't need to change.
#6
This isn't really needed. By convention, the global $user object is the object loaded by the session handling.
If you need the full user object, do a user_load() and add caching to user_load(). It might cost you 1ms.
#7
update: no progress. user_load() still has no static cache like node_load(). these two issues have worked on it.
#8
right, we should really have added a static cache to user_load in D6 since we now often take a simple int parameter. It was an unfortunate oversight (bug?).
simple (but untested) patch attached that adds such a cache.
#9
typo at the end: if ($cachable)
#10
since db_fetch_object() is defined in the API as returning FALSE for no result, we can also trim a couple lines. fixed typo.
#11
#12
Looks good. We need to reset this cache after a user_save(), no?
#13
yea- good point. Also, we should probably remove the caching code in user_category_load()? Otherwise we are caching the same data twice.
#14
Attached is the patch that I've been using with Drupal 5 for a year or so - it's slightly different because it uses memcache, but should illustrate what needs to be changed.
The main thing to watch out for is modules (buddylist, for example) that write directly to the {users} table without going through user_save - they either need to be rewritten to use user_save for all changes to user objects, or to have a line added that invalidates the user cache when a change is made.
#15
Q: Why do we want to clone a user object before passing it along? If a user object is changed somewhere, we would not need to reload it (again)?
Also, possibly related: #254491: Standardize static caching
#16
Hi,
I hope this progresses as fast as possible, and the fix gets backported to D6.
Right now, not having user_load cache the result causes a lot of trouble.
I am writing a module that uses hook_user() and was wondering why so _many_ queries were being bounced off my DB server. Now I know...
Bye,
Merc.