WordPress.org

Make WordPress Core

Opened 15 months ago

Closed 9 months ago

#45105 closed defect (bug) (fixed)

wp_list_authors() performance issue. Queries for user data on authors with no posts

Reported by: billerickson Owned by: ianbelanger
Milestone: 5.2 Priority: normal
Severity: normal Version:
Component: Users Keywords: has-patch commit
Focuses: performance Cc:
PR Number:

Description

On a client site with a single author, 4 posts, and thousands of subscribers (a membership site), we were getting over 17,000 queries on a 404 page that included the wp_list_authors() function: https://cl.ly/aa7ed29b1815

To reproduce this issue:

  1. Install Query Monitor + Debug Bar
  2. Add wp_list_authors() to a page and note the number of queries
  3. Generate new users (wp user generate) and notice the increase in queries

The key issue is the check for empty posts runs after get_userdata() is called, so a query for that user's metadata happens regardless of their post count.

Moving the post count check above get_userdata() fixes the issue.

Attachments (2)

45105.diff (896 bytes) - added by billerickson 15 months ago.
45105.1.diff (869 bytes) - added by ianbelanger 10 months ago.

Download all attachments as: .zip

Change History (12)

@billerickson
15 months ago

#1 @SergeyBiryukov
15 months ago

  • Component changed from General to Users
  • Milestone changed from Awaiting Review to 5.1
  • Owner set to SergeyBiryukov
  • Status changed from new to reviewing

#2 @pento
12 months ago

  • Keywords has-patch added
  • Milestone changed from 5.1 to 5.2
  • Version trunk deleted

Moving to 5.2, as this isn't a trunk regression.

#3 @billerickson
11 months ago

This issue affects almost every Genesis-based website. The default Genesis 404 page builds a sitemap listing all posts, pages and authors: https://gist.github.com/billerickson/858ad34efdea9c75417b4217e33f8c3b#file-archive-php-L37

Any Genesis site with a sufficiently large registered user list (ie: membership sites) can be taken down by hitting random 404s since they’re uncached in most instances and trigger thousands of queries.

This patch is an easy win for performance.

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


10 months ago

#5 @ianbelanger
10 months ago

  • Owner changed from SergeyBiryukov to ianbelanger
  • Status changed from reviewing to accepted

#6 @ianbelanger
10 months ago

  • Keywords needs-testing added

I tested the patch and it does limit the number queries as intended, however it was throwing 2 PHP notices for each user due to the fact that $author = get_userdata( $author_id ); was moved after $posts = isset( $author_count[ $author->ID ] ) ? $author_count[ $author->ID ] : 0; in the loop, so $author was not yet defined.

45105.1.diff changes $posts = isset( $author_count[ $author->ID ] ) ? $author_count[ $author->ID ] : 0; to $posts = isset( $author_count[ $author_id ] ) ? $author_count[ $author_id ] : 0; to avoid throwing the PHP notices, while still limiting the number of queries.

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


9 months ago

#8 @dswebsme
9 months ago

  • Keywords needs-testing removed

Patch 45105.1.diff tested and confirmed. This patch correctly addresses the reported issue without generating any PHP notices (as mentioned as a side effect of the original patch).

Ready for commit.

#9 @ianbelanger
9 months ago

  • Keywords commit added

#10 @SergeyBiryukov
9 months ago

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

In 45235:

Users: In wp_list_authors(), check for author's post count before getting author's metadata.

This significantly reduces the number of SQL queries when wp_list_authors() is called on a site where the majority of users don't have any posts, e.g. a membership site.

Props billerickson, ianbelanger, dswebsme.
Fixes #45105.

Note: See TracTickets for help on using tickets.