Make WordPress Core

Opened 9 years ago

Closed 9 years ago

#35454 closed defect (bug) (duplicate)

`lazyload_term_meta` is called many time when calling `get_term_metadata`

Reported by: sfai05's profile sfai05 Owned by:
Milestone: Priority: normal
Severity: normal Version: 4.4
Component: Query Keywords:
Focuses: performance Cc:

Description

Step to reproduce :
Call WP_Query more then one time in a same request, then action lazyload_term_meta will hook to filter get_term_metadata more then one time.
Line 3633 in wp-includes/query.php

Hugely affect performance if you have many custom taxonomy.

Change History (6)

#1 @boonebgorges
9 years ago

Hi @sfai05 - Welcome to WordPress Trac!

Hugely affect performance if you have many custom taxonomy.

Can you say more about this? Have you done any detailed profiling to see exactly what's affecting performance? Is lazyload_term_meta() triggering lots of database queries? Or is it the cache causing excessive cache reads? Are you running a persistent object cache (Memcached, Redis, etc)?

#2 @sfai05
9 years ago

Sure. I am running Wordpress 4.4 with Memcached.

I found this problem coz my admin dashboard take super long time to response. Then I find that the memcache get request on the dashboard is much higher then before ( > 4000 for loading one page ).

I run three WP_Query on different dashboard widget. Some of the widget will do get_term_meta

The problem is the function lazyload_term_meta is added to hook get_term_metadata duplicated if you called WP_Query for more then one time. So when you run get_term_meta afterward, the action lazyload_term_meta will be triggered multiple times. And in the function lazyload_term_meta you will do multiple Memcache get depends on how many taxonomy you have created.

I only do ~40 get_term_meta it already crashed my memcache server.

Now I set the flag update_post_term_cache to false in WP_Query to keep my server running.

Last edited 9 years ago by sfai05 (previous) (diff)

#3 @boonebgorges
9 years ago

@sfai05 - Thanks for the details.

I'm struggling to see whether this is a fundamental architectural problem with the lazyloading technique, or whether there's a race condition that's causing many more cache checks than is required. Maybe you can help me figure this out.

WP_Query::lazyload_term_meta() is designed to bail when $this->updated_term_meta_cache is true. However, this flag doesn't get set until the end of the method. I wonder if this fact, combined with lots of calls to get_term_meta(), is causing get_object_term_cache() to be called more often than it should. Two things to look at:

  • How many times is lazyload_term_meta() being called? (It should be once for every call to get_term_meta().) And how many times is it getting past the $this->update_term_meta_cache flag? (It should be once for each WP_Query loop previously loaded on the page.)
  • If you move the $this->updated_term_meta_cache = true check to the top of the method - just before the // We can only lazyload... comment - does it help?

#4 @swissspidy
9 years ago

  • Component changed from General to Query

#5 @boonebgorges
9 years ago

In 36524:

Improve WP_Query lazyloading logic, for better performance.

Lazyloading for comment meta and term meta, introduced into WP_Query in
4.4, used flags - updated_term_meta_cache and updated_comment_meta_cache -
in an attempt to prevent cache priming from happening more than once per query
object. This technique was mostly effective, but not entirely efficient, since
the flag didn't prevent the lazyload_*_meta callbacks from running. The
obvious solution - removing the filter callback after it'd be run once - was
dismissed for 4.4 because of concerns that remove_filter() could disable
lazyloading too generally in the context of nested queries, due to the way
_wp_filter_build_unique_id() doesn't always build sufficiently unique IDs for
similar objects. However, further testing shows that this concern is only valid
in a very small subset of cases, while the cost of keeping the query objects in
memory, via the $wp_filter global, is quite significant. As such, this
changeset removes the flags in favor of the remove_filter() technique.

See #35454, #35816.

#6 @boonebgorges
9 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to duplicate
  • Status changed from new to closed

I'm going to close this ticket in favor of #35816. [36424] should resolve your problem, since the cache will only be primed the first time get_term_meta() is called within the loop.

Thanks again for the report!

Note: See TracTickets for help on using tickets.