WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 2 years ago

Last modified 2 years ago

#18369 closed defect (bug) (invalid)

Pre-cache post_meta for nav_menu_items rather than querying one at a time

Reported by: mitchoyoshitaka Owned by:
Milestone: Priority: normal
Severity: minor Version:
Component: Performance Keywords: has-patch
Focuses: Cc:

Description

We know that wp_get_nav_menu_items is going to have to get_post_meta on each individual nav menu item, and unless it's already cached, each of these get_post_meta will spawn their own database query to update_meta_cache.

Instead, we could update_meta_cache once on all of the objects which we know we're going to hit at the beginning of wp_get_nav_menu_items.

Here's the effect of this patch on DB queries on a site I'm working on, with two decently sized nav menus: (averaged over 10 hits)

3.3-trunk: 69 ms, 198 queries

with patch: 55 ms, 142 queries

Attachments (2)

18369.diff (805 bytes) - added by mitchoyoshitaka 3 years ago.
18369.2.diff (1.0 KB) - added by nacin 3 years ago.

Download all attachments as: .zip

Change History (10)

mitchoyoshitaka3 years ago

comment:1 SergeyBiryukov3 years ago

  • Milestone changed from Awaiting Review to 3.3

nacin3 years ago

comment:2 follow-up: nacin3 years ago

Are these front-end queries? Cause this doesn't look right at all. The 198/142 look more like the admin.

18369.2.diff significantly reduces the number of queries that are run on the admin -- rough estimate, by about two-thirds. More optimizations are likely possible.

comment:3 in reply to: ↑ 2 ; follow-up: mitchoyoshitaka3 years ago

Replying to nacin:

Are these front-end queries? Cause this doesn't look right at all. The 198/142 look more like the admin.

The ridiculous number of queries was a combination of the admin bar loading, a couple complex nav menus (which I'm now caching), and a bug with W3 Total Cache which I'm reporting. It's a site still early in development but yes, client side. Now down to 32 queries.

18369.2.diff significantly reduces the number of queries that are run on the admin -- rough estimate, by about two-thirds. More optimizations are likely possible.

Why are you keeping update_post_term_cache false on the client side?

comment:4 in reply to: ↑ 3 nacin3 years ago

Replying to mitchoyoshitaka:

Now down to 32 queries.

Can you post them? Also the code you're using for wp_nav_menu (and any associated callbacks or walkers).

Why are you keeping update_post_term_cache false on the client side?

There's no way core does this all of these queries on its own. I suspect something else is causing quite a number of queries. We would have fetched postmeta initially if we had needed them.

comment:5 mitchoyoshitaka2 years ago

Nevermind, this was a weird interaction between W3 Total Cache and some code we had running. The site no longer is configured as it was when this issue occurred, so I can't really reproduce it. Nacin, should we go ahead and close this? Or do the tickets here still merit committing?

comment:6 ryan2 years ago

  • Resolution set to invalid
  • Status changed from new to closed

comment:7 nacin2 years ago

In that case, you might be running into the fact that under a persistent cache, WP won't bother caching metadata or term data via WP_Query.

comment:8 ocean902 years ago

  • Milestone 3.3 deleted
Note: See TracTickets for help on using tickets.