id	summary	reporter	owner	description	type	status	priority	milestone	component	version	severity	resolution	keywords	cc
18631	wp_set_object_terms() should only wp_update_term_count() for affected terms	jeremyclarke	ryan	"{{{
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!
"	defect (bug)	closed	normal	3.3	Taxonomy		normal	fixed	has-patch dev-feedback	jeremyclarke