[wp-trac] [WordPress Trac] #29894: get_terms() isn't caching - duplicate queries generated

[WordPress Trac]

#29894: get_terms() isn't caching - duplicate queries generated
----------------------------------------+-----------------------------
 Reporter:  webgeekconsulting           |       Owner:  boonebgorges
     Type:  defect (bug)                |      Status:  accepted
 Priority:  normal                      |   Milestone:  Future Release
Component:  Taxonomy                    |     Version:  4.0
 Severity:  normal                      |  Resolution:
 Keywords:  needs-unit-tests has-patch  |     Focuses:  performance
----------------------------------------+-----------------------------

Comment (by boonebgorges):

 webgeekconsulting - Thanks for the patch. This is the kind of thing I had
 in mind, too. A couple thoughts and concerns:

 - Doing `SELECT *` is going to increase the overhead of calling
 `get_terms()` with 'fields' values of 'count', and possibly some of the
 others that currently query just two or three columns. The numbers may be
 small, but I'd like to see some benchmarks before moving forward with
 these kinds of changes. Maybe seed a database with a certain number of tax
 items, then run a script that runs `get_terms()` a bunch of times,
 clearing the cache between each call. Over a couple thousand runs, this'll
 give a reasonable average of the overhead introduced by the broader
 `SELECT` statement.
 - I'd feel much more comfortable making these changes if we had better
 unit test coverage for the various values of 'fields' with respect to the
 object cache.
 http://core.trac.wordpress.org/browser/trunk/tests/phpunit/tests/term/getTerms.php
 has some tests for 'fields' and some for cache, but none that combine the
 two. What's needed is tests that (a) call `get_terms()` to prime the
 cache, then (b) call `get_terms()` again to fetch items from the cache,
 and then (c) verify that the values returned are correct and that the
 number of database queries has not been incremented.
 - The changes you've proposed here, and the ones I proposed here
 https://core.trac.wordpress.org/ticket/29894#comment:10 (stricter
 typecasting for arguments) are not mutually exclusive. It might be
 possible to do my suggestions for now (which would address your original
 issue) and then rework the cache internals as you've suggested for a later
 release, if it seems like the items I've suggested above are too much work
 to do right now.

 If you're able to work on some of these tests and benchmarks, it'll make
 it much more likely that this stuff'll get in sooner rather than later.
 :-)

--
Ticket URL: <https://core.trac.wordpress.org/ticket/29894#comment:14>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform