Make WordPress Core

Opened 3 years ago

Last modified 2 days ago

#40351 assigned enhancement

Term post re-counts scale poorly, are common and difficult to avoid

Reported by: mattoperry Owned by: whyisjake
Milestone: 5.6 Priority: normal
Severity: normal Version: 4.8
Component: Taxonomy Keywords: has-patch needs-testing early needs-dev-note has-unit-tests
Focuses: performance Cc:


Under normal conditions whenever a post status transition occurs, _update_term_count_on_transition_post_status attempts to completely recalculate the post counts for each term related to the post by re-counting each term's total number of post relationships.. For sites with large term relationship tables, large numbers of total terms and high numbers of terms per post, this recalculation does not scale well and can lead to wp-admin lag (while saving a post for example) or failed queries.

A typical bad scenario looks like this:

Consider a site with a large term_relationship table and a post with (say) 30 terms assigned to it. When that post is updated, an eventual call to _update_post_term_count will cause that function to execute 60 total queries on some very large tables. 30 are SELECTs:

SELECT COUNT(*) FROM $wpdb->term_relationships, $wpdb->posts WHERE $wpdb->posts.ID = $wpdb->term_relationships.object_id AND post_status = 'publish' AND post_type IN ('" . implode("', '", $object_types ) . "') AND term_taxonomy_id = %d

(This is actually the hopeful case, since if we need to count attachments too, those are added to the above query via a subselect which probably makes things worse.)

Interspersed among the 30 SELECTs are 30 UPDATEs to the tt table, generated by:

$wpdb->update( $wpdb->term_taxonomy, compact( 'count' ), array( 'term_taxonomy_id' => $term ) );

One result of all of this can be failed queries -- typically the SELECTs -- and incorrect term post counts. Even more frequently the issue manifests as slow post updating behavior (the lag a user feels while waiting for a post to save or publish.)

We currently provide two mechanisms (besides just unhooking _update_term_count_on_transition_post_status entirely and not updating term post counts at all) to do something about this problem. The first is to defer post counts, but this just delays the badness until a later call of wp_update_term_count. The second is to define a per-taxonomy custom update callback using update_count_callback.

Neither of these mechanisms is intended to improve performance, and both are obscure. It would be better to by default increment or decrement post counts directly in the tt table in response to posts entering or leaving published (or other countable) stati, and reserve complete recalculations for special occasions (such as when a term is edited.) Using this strategy, the 60 total queries in the example above could -- on publish -- be replaced by a a single query that would look something like:

UPDATE {$wpdb->term_taxonomy} AS tt SET tt.count = tt.count + 1 WHERE tt.term_taxonomy_id IN ( .... )

(where the final list is a list of tt_ids.)

This solution is implemented now as a plugin here: https://github.com/Automattic/lightweight-term-count-update.

Patch based on this on the way soon.

Attachments (6)

cpu-usage.png (50.7 KB) - added by peterwilsoncc 6 months ago.
40351.diff (7.1 KB) - added by rebasaurus 3 months ago.
Porting of the LTCU plugin into core
40351-1.diff (7.2 KB) - added by rebasaurus 3 months ago.
Add $taxonomy arg to countable_status filter
40351.2.patch (7.2 KB) - added by davidbaumwald 5 weeks ago.
Updated patch for 5.6
40351.3.patch (9.7 KB) - added by lcyh78 4 weeks ago.
Add the filter to the original count code in _update_post_term_count (used in edit-{taxonomy} screen).
40351.4.patch (9.7 KB) - added by lcyh78 4 weeks ago.
Updated consolidated patch. $transition was not set on adding new post.

Download all attachments as: .zip

Change History (30)

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

3 years ago

#2 @desrosj
3 years ago

  • Keywords needs-patch needs-unit-tests added

#3 @peterwilsoncc
6 months ago

cpu-usage.png shows the room for improvement on large sites.

Work (a large news organisation) enabled light-weight term counting on March 7 due to CPU alerts firing daily during the wp_scheduled_auto_draft_delete and wp_scheduled_delete cron jobs. CPU use immediately dropped 30%+ during those tasks.

Note: for various reasons such as the number of users, we generate a lot of auto-drafts so these results aren't typical on a standard WordPress site, this is very specific to a high usage site.

At the very least, I think we can introduce some pre-flight hooks to the term counting functions to allow customisation of the database query. I'd suggest preflight filters for wp_update_term_count_now(), _update_post_term_count() and _update_generic_term_count().

#4 @johnbillion
6 months ago

I've also seen the Lightweight Term Count Update plugin reduce processing time from literally minutes when there's a large number of terms, to fractions of a second.

#5 @whyisjake
3 months ago

  • Milestone changed from Awaiting Review to 5.5
  • Owner set to whyisjake
  • Status changed from new to assigned

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

3 months ago

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

3 months ago

#8 @davidbaumwald
3 months ago

@whyisjake Is this one landable with 5.5 Beta 1 due to be released in just a few days?

3 months ago

Porting of the LTCU plugin into core

#9 @rebasaurus
3 months ago

  • Keywords has-patch needs-testing dev-feedback added; needs-patch removed

#10 @johnbillion
3 months ago

  • Keywords early added; dev-feedback removed
  • Milestone changed from 5.5 to 5.6

I think this should go in early in a cycle (unless Jake wants to run with it for 5.5). I'm willing to shepherd this for 5.6 as it has a huge performance impact on large sites.

#11 @johnbillion
3 months ago

cc @mboynes

#12 follow-up: @whyisjake
3 months ago

I'd love to see this land in 5.5, getting some more eyes on it.

#13 @lcyh78
3 months ago

Would it be possible to have the filter countable_status to include $taxonomy as a parameter as it is available with both usages.
Some custom post types plugins wish to count using a different set of statuses than the default and providing the taxonomy being counted will make it easy to implement this requirement with the filter.

Thanks in advance,
Neil James

#14 in reply to: ↑ 12 @peterwilsoncc
3 months ago

Replying to whyisjake:

I'd love to see this land in 5.5, getting some more eyes on it.

I'd rather put it in early in 5.6, it's a substantial change of approach.

In 40351.diff, the existing query is used on the edit terms screen. I can't remember if that is in the original plugin but it would be good to avoid that in core if possible.

3 months ago

Add $taxonomy arg to countable_status filter

5 weeks ago

Updated patch for 5.6

4 weeks ago

Add the filter to the original count code in _update_post_term_count (used in edit-{taxonomy} screen).

4 weeks ago

Updated consolidated patch. $transition was not set on adding new post.

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

2 weeks ago

#16 @hellofromTonya
2 weeks ago

  • Keywords needs-dev-note added

As discussed during <bug-scrub>, this one will get a Misc Dev Note for the new filter.

#18 @peterwilsoncc
2 weeks ago

  • Keywords needs-refresh added

I've started a pull request based on 40351.4.patch so the existing tests begin running on the screen.

It's still a WIP but so far I have made a few minor changes:

  • Documented countable_status filter, I suspect it will need a rename to the plural
  • Some tidy up for coding standards
  • Modified maybe_recount_posts_for_term() to allow for developers deferring post counts by switching to wp_update_term_count()
  • There were a few errors in the patch that I am slowly tidying up and the unit tests alert me to them, as best I can tell mostly type casting that needs fixing up

#19 @peterwilsoncc
2 weeks ago

Further notes on what needs to be added for this to be ready

  • Shortly after deleted_term_relationships and added_term_relationship fire, calls to wp_update_term_count() need to be removed/modified as the proposed patch doesn't defer term counting like the original plugin.
  • Including a term count on edit_term adds a new location in which terms are counted and doesn't seem like it's needed
  • countable_status is new, very useful but unrelated to this feature. It's probably best left for another ticket.

I'm leaving my PR as is at moment (in some need of change) so I can get some others' feedback on it before updating. As many eyes as possible would be dandy, thanks.

This ticket was mentioned in PR #519 on WordPress/wordpress-develop by peterwilsoncc.

2 weeks ago

  • Keywords has-unit-tests added; needs-unit-tests needs-refresh removed

Completely revised approach to scaling term counting.

  • respects deferring of term counting
  • replaces calls to the current functions with calls to new functions
  • does not introduce new full count points
  • terms added in wp_insert_post() can be double counted (this was happening with the earlier approach too)
  • has a single unit test to demonstrate the bug & prevent passing.

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

#21 @peterwilsoncc
2 weeks ago

I've created a new PR with a different slightly different approach to account for how term counting currently works in Core.

Both approaches are currently a little broken, off-by-one errors, but I think the revised approach is an improvement in terms of maintaining backward compatibility. The new PR is at https://github.com/WordPress/wordpress-develop/pull/519, comments on it would be dandy.

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

11 days ago

#23 @peterwilsoncc
2 days ago

In updated pull request #519:

  • unit tests for term counts, they pass on both trunk and the branch now :)
  • register_taxonomy() accepts a new argument update_count_by_callback for incrementing and decrementing term count. For backcompat it calls update_count_callback if it's defined by "count_by" is not
  • A new function _wp_prevent_term_counting() used to prevent double counting when adding terms within wp_insert_post() and wp_publish_post()
  • DB queries are deferred until the final numbers are known when transitioning post counts to reduce the number of UPDATE calls
  • wp_set_object_terms() and wp_remove_object_terms() now check the taxonomies object type to determine whether the counts should be updated immediately
  • All of Core's calls to the older functions have been removed


  • Has anything been missed?
  • Should I be checking trashing and deleting posts too in the unit tests?
  • Is _wp_prevent_term_counting() too much of a hack and using a reflection preferable?
  • Has anything been missed (yes, asking this question twice)?

As the definition of early 5.6 is beginning to be stretched some additional eyes on this would be superb.

#24 @prbot
2 days ago

peterwilsoncc commented on PR #519:

@NeilWJames Thanks for reviewing.

As update_count_callback() is expecting two arguments in current releases of WP, leaving $modify_by out of the call is intentional.

If a developer has defined a custom counting callback, calling that will ensure their counts are correct until they update how they are registering their taxonomy.

Note: See TracTickets for help on using tickets.