update_menu_item_cache appears to load postmeta for all linked posts

[huemordave]

Discovered this debugging a memory limit error that was causing a staging site to WSoD on every page. The root cause was that loading the site header menu triggered loads of all postmeta.

For context, this site has 35 posts in its menu. If I run the query that killed the site, it returns ~240MB of postmeta (which trips our server's ~512MB memory limit). Yes, 7MB of meta_values per post sounds high; this is because we use a page builder (Beaver Builder) that stores serialized post design configuration in postmeta.

While this sounds excessive, it *would* be fine. Except that when WordPress needs to load menu items (via wp_get_nav_menu_items), it updates a number of related caches. First we update the menu item cache, then the post cache for any posts that are linked from a menu item, then the postmeta cache for all the postmeta of those linked posts, then we run out of memory and crash.

_prime_post_caches, the function that updates the post cache, does provide parameters to enable or disable term and postmeta cache priming. update_menu_item_cache disables term priming but not postmeta priming. This appears to be an oversight: normal nav menu processing doesn't use any of the postmeta we're priming the cache with, so there's no point in loading any of it into the object cache.

The website I diagnosed this problem in is running WordPress 6.4.2 on PHP 8.2. It's hosted on WP Engine which does automatic core and PHP updates. As part of an earlier diagnosis phase, I disabled every plugin that wasn't absolutely necessary for rendering this header. I still got the memory limit error. I was *not* able to disable Beaver Builder or the custom theme we developed for this client as those are responsible for loading the template that calls wp_nav_menu. I don't think there's any plugin hooks in Beaver Builder doing anything funny either; I made PHP print out a stacktrace right before the memory error and the trace goes straight from my header code to WPDB:

#0 /wp-includes/class-wpdb.php(2263): wpdb->_do_query('SELECT post_id,...')
#1 /wp-includes/class-wpdb.php(3152): wpdb->query('SELECT post_id,...')
#2 /wp-includes/meta.php(1177): wpdb->get_results('SELECT post_id,...', 'ARRAY_A')
#3 /wp-includes/post.php(7421): update_meta_cache('post', Array)
#4 /wp-includes/post.php(7808): update_postmeta_cache(Array)
#5 /wp-includes/nav-menu.php(804): _prime_post_caches(Array, false)
#6 /wp-includes/class-wp-query.php(3560): update_menu_item_cache(Array)
#7 /wp-includes/class-wp-query.php(3824): WP_Query->get_posts()
#8 /wp-includes/post.php(2460): WP_Query->query(Array)
#9 /wp-includes/nav-menu.php(729): get_posts(Array)
#10 /wp-includes/nav-menu-template.php(165): wp_get_nav_menu_items(Object(WP_Term), Array)
#11 /wp-content/themes/beaverwarrior/components/CustomHeader/custom-header/includes/frontend.php(19): wp_nav_menu(Object(stdClass))

The way I'm currently fixing this problem is modifying update_menu_item_cache. Yes, this is corehacking, and I understand that it will be overwritten and break with the next core update.

I specifically changed this line:

        if ( ! empty( $post_ids ) ) {
                _prime_post_caches( $post_ids, false );
        }

to instead be:

        if ( ! empty( $post_ids ) ) {
                _prime_post_caches( $post_ids, false, false );
        }

This disables postmeta cache priming for linked posts in nav menus and fixes the memory limit I was running into. I would like to know if there's any further implications from doing this that I'm not aware of. If not, I can submit a pull request to have this merged in.

[spacedmonkey]

@huemordave Thanks for your ticket.

I am not sure what is being ask here from a code standpoint. The issue here is the plugin you using it missing using post meta and storing large bobs of data in post meta. Normally is only recommended to store around 1MB in post meta.

Within your code you can fix this by disabling update_menu_item_cache for all query.

Using a filter like this should work.

add_action( 'parse_query', function ( &$wp_query ) {
      $wp_query->query_vars['update_menu_item_cache'] = false;
}, 1 );

That would you could maintain this in your own codebase without the need to hack / change core files.

Does this solve your problem?

[joemcgill]

@spacedmonkey is this possibly related to the changes from [52975] or would all of the post meta from posts included in a nav already be primed prior to this change?

[spacedmonkey]

@joemcgill As you can see from [52975], we replace calls to get_posts with _prime_post_caches. get_posts set the update_post_term_cache to false. We replicate this behaviour by setting the $update_term_cache to false when calling _prime_post_caches.

This would have been an issues pre WP 6.0.

[flixos90]

As far as I understand @huemordave's report, the problem is that update_menu_item_cache() updates post meta for all posts in the menu. If the meta data is truly not used, then it is indeed wasteful to do that and should be reconsidered. We need to double check whether updating post meta is an oversight or intentional.

Additionally, something else to point out is that update_menu_item_cache() also updates term meta for all terms affected. We should look at whether that data is used as well - if not, it could be removed too.

@spacedmonkey My follow up question is: In [52975], why do the _prime_post_caches() and _prime_term_caches() calls update the meta cache? Is that necessary?

I know this was already happening before, so [52975] is definitely not the problem. But I still wonder: Why is this function loading all the post and term metadata?

[spacedmonkey]

@flixos90 In [52975] we replace calls to _prime_post_caches() and _prime_term_caches(), which replaces calls to get_posts and get_terms, which primed meta caches. Is this data is used? Likely not, but someone might be using it in custom development. If we change this, it would be seem as a breaking, IMO.

I think a solution would be consider, lazily loading post meta. There is already a ticket for lazily loading post meta #57496. In this case, we would have change how lazily loading post meta works. Currrently lazily loading post meta, calls all queue term ids term meta, on the first call to term_meta. But this would not work for post meta, as that is called in lots of places, like feature media. Instead, to make post meta lazily loading work effectively, we would have to only process the queued lazily load meta, it a id is requested that is in the queue.