Skip to content

Convert wp_get_nav_menu_items to use tax query #2399

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
spacedmonkey wants to merge 8 commits into from
Closed

Convert wp_get_nav_menu_items to use tax query #2399

Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
84fd4ed
Convert wp_get_nav_menu_items to use tax query
spacedmonkey Mar 11, 2022
4d4603c
Use term_taxonomy_id instead of term_id.
spacedmonkey Mar 11, 2022
cefdd67
Fix lints.
spacedmonkey Mar 15, 2022
eaac0fe
Only query if count is not empty.
spacedmonkey Mar 21, 2022
5cc61d3
Add taxonomy exists check.
spacedmonkey Mar 21, 2022
ff9b3c8
Merge branch 'trunk' into fix/tax-query-menu
spacedmonkey Mar 22, 2022
d559364
Merge branch 'trunk' into fix/tax-query-menu
spacedmonkey Mar 24, 2022
e2a5add
Merge branch 'trunk' of github.com:WordPress/wordpress-develop into f…
spacedmonkey Mar 29, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 11 additions & 7 deletions src/wp-includes/nav-menu.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -693,24 +693,28 @@ function wp_get_nav_menu_items( $menu, $args = array() ) {

static $fetched = array();

$items = get_objects_in_term( $menu->term_id, 'nav_menu' );
if ( is_wp_error( $items ) ) {
if ( ! taxonomy_exists( 'nav_menu' ) ) {
return false;
}

$defaults = array(
$defaults = array(
'order' => 'ASC',
'orderby' => 'menu_order',
'post_type' => 'nav_menu_item',
'post_status' => 'publish',
'output' => ARRAY_A,
'output_key' => 'menu_order',
'nopaging' => true,
'tax_query' => array(
array(
'taxonomy' => 'nav_menu',
'field' => 'term_taxonomy_id',
'terms' => $menu->term_taxonomy_id,
),
Comment on lines +708 to +713

@peterwilsoncc peterwilsoncc Mar 22, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might have an adverse performance affect:

  • it adds a JOIN to the query
  • on a site with multiple menus (eg, header and footer) an additional uncached query will be made for each menu. Without the taxonomy portion of the query, the second and subsequent query will hit an existing cache.

A history lesson here might be helpful.

@spacedmonkey spacedmonkey Mar 22, 2022

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For reference this change remove one query and replaces it with a very simple JOIN. See the before and after.

Before

Screenshot 2022-03-22 at 12 17 58

After

Screenshot 2022-03-22 at 12 17 45

@spacedmonkey spacedmonkey Mar 22, 2022

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function was added in WP 3.0 - d357c80

Advanced tax queries were not added until 3.1 - https://core.trac.wordpress.org/changeset/16555

@peterwilsoncc peterwilsoncc Mar 23, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I set up a plugin to do some rudimentary testing. The numbers are only useful as relative indications.

Set up:

  • Vagrant local dev environment
  • MySQL on same host
  • Small menu: 5 posts, 5 tags
  • Med menu: 13 posts, 10 tags
  • Large menu: 100 posts, 100 tags

The database contained:

  • 1238 post objects
  • 15,000 term relationships

Anyway, I reckon this puppy does have some benefits and is probably worth putting in.

These are the results for 1000 loops of each test case.

Menu Size Cached Patched time Patched Queries Unpatched time Unpatched queries
Small Y 1.0665791034698 1001 4.2827336788177 1005
Small N 2.5907719135284 2000 10.757255315781 6000
Med Y 1.2250320911407 1001 7.2486755847931 1005
Med N 2.260603427887 2000 15.30788230896 6000
Large Y 0.75003957748413 1001 54.079955101013 1005
Large N 1.9239404201508 2000 89.751573801041 6000

),
);
$args = wp_parse_args( $args, $defaults );
$args['include'] = $items;

if ( ! empty( $items ) ) {
$args = wp_parse_args( $args, $defaults );
if ( $menu->count > 0 ) {
$items = get_posts( $args );
} else {
$items = array();
Expand Down