Performance: static caching of user objects in user_load(), respecting conditions

Quick visual review.

We don't normally use count - empty and !empty are faster. Also there's some code style issues - comments should be: // Sentence with a full stop at the end. - false should be FALSE (or maybe a constant following recent discussions in other issues) - general guidelines at http://drupal.org/coding-standards

A couple of things I missed,

This issue is actually a duplicate of http://drupal.org/node/91786 - which has a fairly recent patch on it. This should be merged back into that one ideally.

Also - running all tests, this appears to introduce no regressions. At first sight, pwolanin's patch at 91786 looks more concise, but I've not done any real comparison of the approaches at all - and the static cache facility needs to be considered in relation to both.

@catch: the difference between the two is that pwolanin's patch only cache simple uid user_load, while this one attempts to solve the general problem.

If it was me, I would have done:

ksort($array);
$key = md5(serialize($array));
if (isset($cache[$key])) {
 return $cache[$key];
}

But this is probably awful performance-wise.

Well, you can even drop the md5().

On a quick glance through, there's a good number of code style issues with the commenting (comments should end with periods; in-line comments should be set off the code with spaces, etc.)

See #91786: user_load() static caching.

Why #16 failed? The test said all was ok... I don't know much about the testing, what i had
overseen?

I converted for testing the last patch to D6.

I tried it on a demo site with a medium web 2.0 module setup.

It gives an instant 10% speed increase for a profile with full enabled caching include
block caching for logged users, i assume the speed increase for a production site will be around
10-15%. Thats pretty nice for such a small patch, even more because it takes effect on a page
with high execution time (for my demo site the page render time was 340-306ms without and down to
266ms with).

Combined with the path_cache module, you will see a pretty clean mysql fingerprint
without the redundant multi-calls.

Really sad this is not in D6. At last there should be a extensional patch for people building
a social site.

#347250: Multiple load users added this to Drupal 7 but won't be backported because it's an API change.

If someone wants to try to get a basic static cache into D6, re-opening #91786: user_load() static caching is probably the best bet. But for now closing these both out as duplicates.

Here is a D6 port of this patch.
Its running fine so far for us at www.daimonin.org.

Not sure it will have side effects for other sites and other non core module installations,
so count it for now as experimental.

Subscribing

Hi guys. Just a quick question. This is just a basic 3 line code to be added at the top of the function for user_load.

it is assuming that the $array variable is a string of uid, which occurs most if not all the time. I was wondering if there is anything wrong with the code below assuming the array parameter is a string variable of uid.

function user_load($array = array()) {
// Dynamically compose a SQL query:
$query = array();
$params = array();

//My Code
static $user = null;

if($array==$user->uid)
{
return $user;
}
//End of my code

if (is_numeric($array)) {
$array = array('uid' => $array);
}
elseif (!is_array($array)) {
return FALSE;
}

Thanks

Hi.
My first post.

Problem is that this assumption is definitely to strong. There's high probability to have this function get called with array as a parameter (i'm having that case on few of my installations).

I've made an alternative version of user_load patch that uses serialize() to compose function arguments into unique string, that can be use as a static cache key. Maybe more time consuming but bit clearer and "theoretically" more reliable -)

Previous patch was created from Subversive SVN, this one is from "diff" command.

#347250: Multiple load users

Reopening, as this is not closed for D6.

Patch in #24 seems close.

Moving to the correct version.

This is actually a significant API change, as some code might expect to get a fully refreshed object from a call to user_load(). At the minimum, we need to clone the returned object, and we might need to go as far as defaulting to non-cached.