menu load objects are not cached

seems like a good idea. eventually, this should be against HEAD in order to cvs commit team to review.

New feature, so marking 6.x-dev.

The new menu system [see _menu_translate()] is calling user_load(1) 9 times on a view profile page. same for edit profile. we should either cache user load or make menu smarter. load can be expensive since we do hook_user(load) every time.

Easy...

Installed patch, created story, edit my account and profile, still works.

I'm not sure this is a good idea, since the menu system doesn't know the details of the objects being loaded - especially in combination with a load function that may be looking at additional parts of the path.

I'd suggest solving the user load on profile pages problem specifically - I already have considered (but not yet implemented) it as part of this patch: http://drupal.org/node/111481 since it requires a separate load function in any case.

Of course, that patch is still waiting on: http://drupal.org/node/177488

I was away for the weekend, I'll try to get back to that one tonight.

that never should happen, as the load functions can be called from any page if there is a menu item pointing at them that's why we have that linked issue. I think we could agree on that a load function should be a simple black box: in comes the object id, out comes the object.

huh?

Let's try again, I suspect i garbled the html.

that may be looking at additional parts of the path.

that never should happen, as the load functions can be called from any page if there is a menu item pointing at them that's why we have that linked issue.

@chx - well, it seems that the various forms of the patch here: http://drupal.org/node/177488

allow a load function to use more than just the single object ID to determine what to load. That's why I liked the form of the patch that passed in directly the map and index - it makes this possibility more clear/explicit. I think this is a useful tool - think, for example, about node revisions - node load wants you to pass in both the nid and the vid. So, putting in this patch would potentially block the functionality being introduced by http://drupal.org/node/177488.

This solves a problem here and now. We can easily add caching to 177488 too. But that issue does not belong here.

I'm very much in favor of this!
Still I'm not sure this change could make it into D6, though it would improve performance.
Would be great, if someone like chx or pwolanin could follow up...

Didn't apply anymore, after #184022 went in. For now, I rerolled this, but only caching functions with non-array arguments though. Wasn't sure if and how we'd cache the other ones. With this incomplete caching, about 10ms are saved on my machine for profile pages.

code looks reasonable

other than non-unix line endings, patch applies cleanly. I don't see any problems testing with a user profile, etc.

@Pancho - thanks for taking this up again.

However, after debating with chx, the only major performance gain (for core at least) will be on user profile pages, as far as I know. So, lets just make it clear that responsibility for caching to the load function somewhere in the docs?

While it seems unlikely that the cached data would get stale, one can imagine some scenarios where the page callback alters the object and hence affects which tabs/links should be rendered.

I just checked the api for the user_load() function in D6 and noticed that user_load() still not caches users. Any reason in particular why it was decided not to do this?

user_load caching is the most needed spot. see #91786: user_load() static caching for new patch.