wp_set_object_terms() should only wp_update_term_count() for affected terms

[jeremyclarke]

wp_set_object_terms($object_id, $terms, $taxonomy, $append = false)

The problem

Currently, after creating/inserting any new terms passed in via the $terms argument, wp_set_object_terms() will run wp_update_term_count() on all of the $terms passed in, regardless of whether they were actually affected in any way. This seems really wasteful to me, and as far as I can tell causes huge problems when trying to delete large terms or remove terms from many posts at once.

If you look at the delete section of the function you can see that it notes the terms that are being deleted, and after deletion runs wp_update_term_count() only on those terms. This is the way it should be.

The main section that adds terms, on the other hand, ignores whether a term was affected and updates the counts of all terms passed in. Inside the foreach loop it carefully checks if each term is already attached to the object, and if it is not, it adds it. Unfortunately, the call to wp_update_term_count() that comes next fails to use this information, updating all categories both new and old.

IMHO only posts that weren't already on the object should have their counts re-generated and re-saved, as there is no reason to believe that the other terms have changed.

Am I missing something? I can't see a reason to reset counts on the other terms that weren't added to the object.

Importance

This may seem trivial, but when adding/removing a single category to/from hundreds of posts the re-counting time adds up quickly (deleting one term took 18 minutes on my site!) In such a scenario only the one term should have it's count redone, but every single term on every single post ends up being recounted over and over because they are all being passed in. Sure, wp_defer_term_counting() is a partial solution to this issue, but even there it should only be deferring that one term that is being added/deleted, not all the terms that are merely being confirmed as already on the post.

I noticed this because the $append version of wp_set_object_terms() is HUGELY faster on the site I'm working on, because when you use it you don't pass in already-assigned $terms, and thus you don't encounter the bug I'm describing.

Unlike for adding a single category, where you can just pass in one $term and $append=true, there is currently no matching system for removing a single category, you are locked into passing in all terms that were previously on the object but with your deleted term removed (see wp_delete_term() for an example). IMHO this is why wp_delete_term() has such atrocious performance on large sites with lots of terms (see #4365).

FWIW the issue of simple term-removal will be solved by #15475 , which adds a wp_remove_object_terms() function that works as the opposite of wp_set_object_terms() with $append=true. If you look at the patch on that ticket you'll see that it has the same property as the deletion section of wp_set_object_terms: it only resets the term count on terms that were actually deleted.

Solution

My solution (patch attached) is straightforward: Create a new array in wp_set_object_terms() called $new_tt_ids and add tt_ids of terms into it as they are added to the object, but not if they were already attached to it. Then when running wp_update_term_count() use that array instead of the full array of all $tt_ids set on the object.

After some testing I've noticed huge improvements in performance of deleting categories :

Without patch (# of posts in category/seconds for AJAX to finish deleting it)

• 12 / 21s
• 86 / 156s

With patch

• 12 / .76s
• 112 / 2.72s
• 408 / 10.2s

I also tested my own term migration system which uses wp_defer_term_counting() and the difference was significant, though not as pronounced for predictable reasons (~7 seconds for 50 posts before patch, ~1 second for 50 posts after patch).

P.S. Is there a reason wp_delete_term() doesn't use wp_defer_term_counting() while deleting posts? Seems like it would save a lot of trouble!

[jeremyclarke]

Fix wp_set_object_terms to only reset count on affected terms

[ryan]

In [18999]:

Call wp_update_term_count() only for those terms that have been added to or removed from the object. Props jeremyclarke. fixes #18631