Add query cache to WP_User_Query class

[johnjamesjacoby]

When calling get_users() the query itself is not cached. If multiple calls to get_users() with the same $args array occur, each will result in a database hit.

Note that each individual user object is cached, and that query caches will need to be invalidated when users are added/removed/edited.

Theoretically, on busy sites with many users who are constantly changing passwords and things, caching user queries may not yield a significant performance improvement. For the majority of installations (where users are not shifted around nearly as much) this could result in several additional cache hits per page load (especially with persistent caches in place.)

Functions like wp_dropdown_users() are called multiple times when viewing the Posts list-table, and many plugins also call get_users() knowing that user objects are cached (but not noticing the additional database hits for each call itself.)

Membership plugins like BuddyPress would have inherent benefits, and bbPress could operate more efficiently when querying for the intersection of many topics with many users who are engaged in them.

Patch imminent.

[spacedmonkey]

#41847 was marked as a duplicate.

[spacedmonkey]

My first patch adds basic caching. It add cache invalidation for user meta, adding users to blogs and CRUD functions for users.

This changes all queries to be ids, get the whole user object. Then for fields queries, just pick off the fields out of the WP_User object that you want.

Other big change is to the cache_users function. This function is very similar to _prime_<type>_cache function. I have added new param to cache_users of $update_meta_cache to optionally prime user meta caches.

[spacedmonkey]

Related: #40669

[westi]

Some thoughts on the latest patch:

• It uses two different last_changes values - users and comments - I guess some copy paste bug.
• As mentioned in the original ticket adding caching here is probably not going to help at all on busy multi-site installs because the cache will continually be invalidated - potentially just adding useless cache set traffic both for individual query results and updates to last_changed.
• I wonder if a better solution to the described problems would be to introduce in-memory non-persistent caching for query results so that re-running queries in the same page generation will be neutered.
• Ideally we should add caching only when we can demostrate a functional performance improvement not just because we can.

[dd32]

I have some serious concerns about adding caching here - it seems like it's something that for the majority of sites will fall into one of the following buckets

• It's a small site, and get_users() is called with the same set of simple arguments, resulting in cache hits
• It's a large site, where get_users() is seldom called but when it is it's a few simple arguments, resulting in cache hits
• It's a large site, where get_users() is called often, with varied arguments (often complex) resulting in many cache misses due to varying keys

For the first two cases, a cache here would be useful, however, I don't think they would really need the cache at all.
The 3rd case is where caching would be thought to be needed, but if the query arguments change often then caching wouldn't actually be of benefit.
If the reasoning for this change is simply to reduce queries, that seems like the incorrect way to go, and I'd suggest that looking at what is calling get_users() often is a better case of a location for the caching.

While we could make the caching for get_users() in-memory non-persistent (like we do with say comments IIRC), I'm not convinced that it'll actually help site performance and instead just make this part of the code more complex.

To tack on to @westi's comment though - in-memory non-persistent query caching sounds like an interesting approach, and one that i believe some DB caching dropins may already do. I'd like to see that built up in a plugin myself to see exactly what impact it has on a regular site (and then a site which would actually benefit from it) past memory usage - but i suspect it wouldn't be of great benefit to the majority of installs (and probably a detriment to a lot of them).

[strategio]

We are running a big site with more than 100k users and this is causing an important performance hit for us. I think an in-memory caching would already help. For instance, a user drop-down is displayed twice in "Posts -> All Posts" and for each call, get_users is called with the same arguments. One unique query takes ~3s, so this is an important place of improvement for us.

If you still prefer not to add caching in get_users, let me know and I will open a ticket to fix it at least in WP_Posts_List_Table::inline_edit where wp_dropdown_users is called twice.

[spacedmonkey]

As caching requirements will be different for different projects, it may be best to keep this caching as a plugin. To that end, I have created a plugin that will do the caching. You can find the code ​here. This plugin requires a number of new filters to added in core.

I would love feedback on this plugin.

[srikanthmeenakshi]

#55223 was marked as a duplicate.

[OllieJones]

A similar issue crops up in the Gutenberg editor's REST request to populate the author dropdown.

The REST request looks like this

​https://giantsite.example.com/wp-json/wp/v2/users?context=view&who=authors&per_page=50&_fields=id,name&_locale=user

Enhancing its performance requires judicious filtering at the 'rest_user_query' hook.

(Just like handling the Posts and Pages panels requires judicious filtering on 'wp_dropdown_users_args'.)

Why are these slow? They need to examine each user's wp_capabilities metadata, which holds a serialized php object. It's expensive for MariaDB / MySQL to filter on this sort of predicate.

WHERE ... meta_key = 'wp_capabilities' AND meta_value LIKE '%"author"%'

for two reasons. One is the notorious LIKE '%something' dbms performance killer. The other is the fact that meta_value columns are CLOBs, which take extra work for the dbms to retrieve. And retrieve them it must, one for every user. In a megauser installation, that is *very* slow. It also hammers the dbms internal cache, potentially slowing down everything else.

[khoipro]

When running wp_dropdown_users with 90k users (from my project), even if we set 'role' is only administrator, the user query still slow. The alternative we must fix is providing a input text and a custom REST API to find a single user by user_login and replace input value on save.

This is a first draft of PR for adding query caching to WP_User_Query cache. Tests need to be added and lots of manual testing.

Trac ticket: https://core.trac.wordpress.org/ticket/40613

[spacedmonkey]

Updated the patch to be on github.

I think core is in a place where we could introduce this without breaking anything.

[OllieJones]

For what it's worth, I have published a plugin to address many of these many-registered users issues, here. ​https://wordpress.org/plugins/index-wp-users-for-speed/advanced/

The core fix is still a good idea, of course.

[rjasdfiii]

Please add samples of the SQL statements necessary for this user cache. We should optimize the queries and/or improve the indexes at the same time.

[spacedmonkey]

@rjasdfiii Change databases is out of scope of this change. Let's do that in a follow up ticket.

• if users have modified the select clause

There is already a unit test for this, see test_users_pre_query_filter_should_bypass_database_query.

• random in orderby if possible

There is no random orderby that can see.

@peterwilsoncc I pushed a lot of changes, I think this is ready for review now.

[flixos90]

This is not a bug, but an enhancement. Since there have been no updates in 2 months, I'll move this to the 6.3 milestone, since the 6.2 Beta 1 deadline is tomorrow. If the PR can be finalized and approved tomorrow in time, we can move it back, but at this point it seems unrealistic given there are a few other query related enhancements already that need to be finalized by then.

[spacedmonkey]

@flixos90 For context, no update is because of a lack of feedback / code review.

Pinging @johnbillion @dd32 @peterwilsoncc for code review.

[rjasdfiii]

Replying to khoipro:

When running wp_dropdown_users with 90k users (from my project), even if we set 'role' is only administrator, the user query still slow. The alternative we must fix is providing a input text and a custom REST API to find a single user by user_login and replace input value on save.

Does that mean fetching 90K entire user objects? Or names 90K Names? And is it for displaying? Or for auto-complete?

I can think of better ways to handle auto-complete than starting with 90K strings.

[mukesh27]

This ticket was discussed in the recent performance bug scrub.

The ​PR 3544 need manual testing so can anyone from testing team review this. @robinwpdeveloper

Props to @spacedmonkey

[mukesh27]

Hi @spacedmonkey, I reviewed PR and left some feedback on it.

[spacedmonkey]

In 55657:

Users: Cache database queries within WP_User_Query class.

Cache the results of database queries within WP_User_Query class. Only cache queries that are requesting 3 or less fields so that caches are not storing full user objects. Cache results are stored in a new global cache group named users-queries. Add a new parameter to WP_User_Query called cache_results to allow developers to opt out of a receiving cached results. cache_results parameter defaults to true. Also add a new helper function called wp_cache_set_users_last_changed, similar to wp_cache_set_posts_last_changed that incroments last changed value in cache group users. Ensure that wp_cache_set_users_last_changed is called whenever user / user meta is modified for proper cache invalidation.

Props johnjamesjacoby, spacedmonkey, westi, dd32, strategio, srikanthmeenakshi, OllieJones, khoipro, rjasdfiii, flixos90, mukesh27, peterwilsoncc.
Fixes #40613.

Committed. ​https://github.com/WordPress/wordpress-develop/commit/19bd759db57b4f1cc72157a446915ffbafdd838e

[spacedmonkey]

I think this change need it's own dev note.

[SergeyBiryukov]

Thanks for the commit!

Minor nitpick, could the users-queries group be renamed to user-queries, for consistency with other groups:

• post-queries
• comment-queries
• term-queries
• network-queries
• site-queries

Trac ticket: https://core.trac.wordpress.org/ticket/40613

[spacedmonkey]

Replying to SergeyBiryukov:

Thanks for the commit!

Minor nitpick, could the users-queries group be renamed to user-queries

Good call. I have created a PR to change this group. ​https://github.com/WordPress/wordpress-develop/pull/4359

[spacedmonkey]

In 55680:

Users: Change cache group from users-queries to user-queries.

The cache group users-queries was added in [55657]. This global cache group, was named to be inline with cache groups added in [55526]. However, the naming of the group does not match, as other cache groups, do not end with s at the end. This change fix this naming.

Props spacedmonkey, SergeyBiryukov, peterwilsoncc.
See #40613.

Committed.

[spacedmonkey]

Draft dev note ready for reivew - ​https://docs.google.com/document/d/167MFCnGHllI8EhT2asQCuBAznHn5X9Xt0-E1eR5w0mw/edit?usp=sharing

[flixos90]

Dev note was posted last week: ​https://make.wordpress.org/core/2023/07/14/improved-caching-for-database-queries-in-wp_user_query/