Call cache_users() when 'fields'=>'all' in WP_User_Query

[danielbachhuber]

Because roles and caps are populated (r32001), which is a meta call, we should proactively call cache_users() and pre-empt dozens of unnecessary meta SELECTs.

cc @boonebgorges

[boonebgorges]

Yeah, we should do something here. Not totally sure what. A couple thoughts:

• This is not a new issue. [32001] pointed the role query at the correct blog, but the query has always taken place.
• fields=all doesn't interact with any cache - either for usermeta or for user objects themselves

If we call all of cache_users(), we have essentially eliminated the distinction between fields=all and fields=all_with_meta. See 36508.diff​. On many sites this would be an improvement. But usermeta also tends to be abused, so that fetching usermeta for all users in a query can significantly affect memory footprint. This is especially worrisome given that all is the default value of fields.

A couple of other possibilities:

1. We introduce a standalone function for priming the wp_x_capabilities cache, and call that - either instead of cache_users() or as an option within cache_users() - when fields=all.
2. We lazyload usermeta in WP_User_Query loops. Usermeta, including capabilities, wouldn't be loaded until it's requested. This would mean some refactoring of WP_User, so that capabilities aren't loaded into user objects until they're requested (so WP_User::allcaps etc would be handled via magic method or something like that). This is obviously much more extensive, but has the potential to improve performance across the board when using either WP_User_Query or WP_User.

@danielbachhuber Do you have thoughts about the best strategy?

[danielbachhuber]

I think this option would be the most precise:

We introduce a standalone function for priming the wp_x_capabilities cache, and call that - either instead of cache_users() or as an option within cache_users() - when fields=all.

Solely priming the capabilities cache would continue to respect the distinction between fields=>all and fields=>all_with_meta, while also addressing the unnecessary queries issue.

I agree with this sentiment too:

But usermeta also tends to be abused, so that fetching usermeta for all users in a query can significantly affect memory footprint.

[boonebgorges]

This is more complicated than I'd originally thought. (Gee, I end up saying that a lot.) You can't prime WP's metadata cache for a single key. Cached data is keyed by object ID (such as user_id), and if get_metadata() finds that $cache[ $user_id ] is populated, it assumes that it's *completely* populated (ie, with all of $user_id's usermeta). Invalidation, cache priming, and a bunch of other stuff is tied up with this schema.

Instead, I opted in 36508.diff​ to run a very specific query within WP_User_Query, and then pass an array of $raw_caps to the WP_User object, which WP_User uses instead of get_user_meta( $cap_key ). The structure of WP_User makes it impossible to do this in a non-clunky way, but the patch works.

[ocean90]

@boonebgorges After three months, what are you plans with 36508.diff​?

[boonebgorges]

Replying to ocean90:

@boonebgorges After three months, what are you plans with 36508.diff​?

Sorry this got lost in the shuffle. I continue to think the approach is clunky, but can't think of a better way to do it. I don't have the stomach for this kind of change at this point in the cycle, so let's pick it up for 4.7.

[spacedmonkey]

This was fixed as part of #55594.