WordPress.org

Make WordPress Core

Opened 16 months ago

Last modified 3 months ago

#38741 accepted enhancement

Introduce the concept of a large site in order to speed up the Users screen when there are many users

Reported by: johnbillion Owned by: johnbillion
Milestone: 5.0 Priority: normal
Severity: normal Version:
Component: Users Keywords: early needs-patch
Focuses: administration, multisite, performance Cc:

Description

Given a single site install with a high number of users, the Users screen can become very slow due to the nasty query performed by count_users(). The result of this query is used to populate the count of users with each role in the role filtering links across the top.

SELECT
	COUNT(NULLIF(`meta_value` LIKE '%\"administrator\"%', false)),
	COUNT(NULLIF(`meta_value` LIKE '%\"editor\"%', false)),
	COUNT(NULLIF(`meta_value` LIKE '%\"author\"%', false)),
	COUNT(NULLIF(`meta_value` LIKE '%\"contributor\"%', false)),
	COUNT(NULLIF(`meta_value` LIKE '%\"subscriber\"%', false)),
	COUNT(NULLIF(`meta_value` = 'a:0:{}', false)), COUNT(*)
FROM
	wp_usermeta
WHERE
	meta_key = 'wp_capabilities'

On a site with tens of thousands of users, this SQL query can easily take tens of seconds.

Multisite has the concept of a large network (by default, one with > 10,000 users or > 10,000 sites), but single site doesn't have this concept.

The query performed by count_users() can be skipped entirely. If it is skipped, the list of roles will simply display all available roles and won't display a count next to each role. I think this is a completely acceptable trade-off for such a performance gain on sites with a high number of users.

Attachments (7)

38741.diff (3.9 KB) - added by johnbillion 16 months ago.
38741.2.diff (5.5 KB) - added by tharsheblows 15 months ago.
with wp_is_large_site()
38741.3.diff (6.3 KB) - added by tharsheblows 10 months ago.
with tests and cleaned up a bit
38741.4.diff (8.5 KB) - added by tharsheblows 10 months ago.
renamed functions and apply filter before ms one
38741.5.diff (8.9 KB) - added by johnbillion 6 months ago.
38741.6.diff (8.4 KB) - added by johnbillion 6 months ago.
38741.7.diff (9.1 KB) - added by johnbillion 6 months ago.

Download all attachments as: .zip

Change History (36)

@johnbillion
16 months ago

#1 @johnbillion
16 months ago

  • Keywords has-patch needs-testing added

38741.diff is a partial patch which implements the performance improvement on the Users screen, but only implements a no-oped version of a wp_is_large_site() function.

To test the patch, change the return value of wp_is_large_site() to true.

TODO: Implement logic in wp_is_large_site() which determines if the site has a large number of users or not. This will probably need to be converted to a cached value in the same way that Multisite uses a cached value for wp_is_large_network().

#2 @Mista-Flo
16 months ago

+1 on this, we should focus on this performance issue.

#4 @tharsheblows
15 months ago

There's also an instance in wp-includes/update.php so I've updated that one too. This makes the count a transient with a max expiration of 12 hours and takes the query down from about 2.5s to 50.3ms with user table of 184k.

I'm not sure if:

  1. the value should be a transient
  2. the expiration time should be filterable

Still needs testing and tests.

@tharsheblows
15 months ago

with wp_is_large_site()

@tharsheblows
10 months ago

with tests and cleaned up a bit

#5 @tharsheblows
10 months ago

With tests and cleaned up a bit

#6 @johnbillion
10 months ago

  • Keywords has-unit-tests added
  • Milestone changed from Awaiting Review to 4.8

#7 @tharsheblows
10 months ago

Updated function names and applies before the 'wp_is_large_network' too.

@tharsheblows
10 months ago

renamed functions and apply filter before ms one

This ticket was mentioned in Slack in #core by jeffpaul. View the logs.


10 months ago

This ticket was mentioned in Slack in #core by jeffpaul. View the logs.


10 months ago

This ticket was mentioned in Slack in #core by obenland. View the logs.


9 months ago

#11 @obenland
9 months ago

  • Milestone changed from 4.8 to Future Release

#12 @johnbillion
9 months ago

Needs some more testing and review.

#13 @johnbillion
6 months ago

@tharsheblows I just remembered that we already have get_user_count() which stores a count of all the users on the network. This can be used in place of the new user count functions that you've added in your patches.

Do you want to update your patch? I'd like to see about getting this into 4.9.

#14 @tharsheblows
6 months ago

@johnbillion that's multisite only, isn't it? In ms-functions.php.

I'd love to get this in 4.9! :)

@johnbillion
6 months ago

@johnbillion
6 months ago

@johnbillion
6 months ago

#15 @miss_jwo
6 months ago

Looks good to me. Thansk for changing the function name to wp_is_large_user_count() as the original suggestion would have been misleading in many ways.

#16 @johnbillion
5 months ago

  • Keywords needs-testing removed
  • Milestone changed from Future Release to 4.9
  • Owner set to johnbillion
  • Status changed from new to reviewing

#17 @johnbillion
5 months ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 41613:

Users: Introduce the concept of a large site in order to speed up the Users screen when there are many users.

Calling the count_users() function is expensive, regardless of the counting strategy that's used, and it gets
slower the more users there are on a site. In order to speed up the Users screen in the admin area, calling
count_users() can be avoided entirely while still displaying the total count for users.

This introduces some new functions:

  • wp_is_large_user_count()
  • wp_get_active_user_count()
  • wp_update_active_user_count()

A corresponding wp_is_large_user_count filter is also introduced.

Props tharsheblows, johnbillion

Fixes #38741

#18 @johnbillion
5 months ago

In 41614:

Users: There is not, in fact, 12345 users on every WordPress installation.

Props spacedmonkey

See #38741

#19 @johnbillion
5 months ago

  • Keywords needs-patch needs-unit-tests added; has-patch has-unit-tests removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

Some issues have arisen:

  • The function names need improving as nothing here relates to whether a user is "active" or not.
  • COUNT(ID) should be used to better utilise the table index.
  • The tests are failing ¯\_(ツ)_/¯

#20 @johnbillion
5 months ago

In 41623:

Users: Remove the failing tests added in [41613] while they're investigated.

See #38741

#21 @jrchamp
5 months ago

If you don't want to worry about the right field to use to match the available indexes, COUNT(1) might be a simpler, consistent option that the database can optimize easily.

This ticket was mentioned in Slack in #core by jeffpaul. View the logs.


5 months ago

#23 @johnbillion
5 months ago

  • Focuses multisite added
  • Status changed from reopened to accepted

I'm going back to the idea of moving get_user_count() out of the Multisite files and making it available for single site installations too. Otherwise we have separate functions (get_user_count() and the soon-to-be-renamed wp_get_active_user_count()) for essentially doing the same thing.

Patch coming up.

Adding the multisite focus even though this functionality doesn't actually affect Multisite.

#24 @johnbillion
5 months ago

  • Keywords needs-revert added; needs-patch needs-unit-tests removed

Regrettably I think this needs some more work and I don't want to rush it in today. I'm going to revert this and get it into 5.0 early.

#25 @johnbillion
5 months ago

In 41753:

Users: Revert [41613], [41614], and [41623] as this feature needs some more work.

See #38741

#26 @johnbillion
5 months ago

  • Keywords early needs-patch added; needs-revert removed
  • Milestone changed from 4.9 to Future Release

#27 @johnbillion
5 months ago

  • Milestone changed from Future Release to 5.0

This ticket was mentioned in Slack in #core-multisite by flixos90. View the logs.


3 months ago

#29 @tharsheblows
3 months ago

I'm currently using 38741.7.diff(ish — with 4.8.3) on a production site and will let you know if it all goes horribly wrong. (It hasn't yet.)

Note: See TracTickets for help on using tickets.