#22189 closed defect (bug) (fixed)
Saving navigation menus is slow
Reported by: | nacin | Owned by: | |
---|---|---|---|
Milestone: | 3.5 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Menus | Keywords: | |
Focuses: | Cc: |
Description (last modified by )
This is a follow-up to a few tickets, such as #17031. I could not find a duplicate. It is also tangentially related to #14134, which would be menus are affected to max post var settings.
Bottom line: saving navigation menus are slow. I was hoping to see that removing hierarchy ancestor cleaning in #11399 made a big improvement, but a 90-item flat menu still took 24 seconds and a few thousand queries to save. No me gusta.
I started chipping away at this giant list of queries, and came up with a number of changes across a number of areas of core to reduce them. The page now saves (and renders — it doesn't redirect) with only a few hundred queries — it's a lot, but considering a few queries per post being saved, not bad — and just a few short seconds on my little MacBook Air that could.
Since these improvements are largely outside the menus API, I will be spawning individual tickets for most things.
Attachments (5)
Change History (27)
#1
@
12 years ago
- Summary changed from Saving navigation menus are slow to Saving navigation menus is slow
@
12 years ago
When printing nav checklists, we avoid meta and term caches. WP_Post's magic get/set will trigger a metadata_exists() check. Using property_exists() and some cleaner code can prevent the meta cache from firing.
#3
@
12 years ago
22189.diff — Avoid useless queries in wp_unique_post_slug() for nav menu items. We may want to introduce a pre_* filter here in the future, so plugins could similarly sidestep these queries.
22189.2.diff — When printing nav checklists, we avoid meta and term caches. WP_Post's magic get/set will trigger a metadata_exists() check. Using property_exists() and some cleaner code can prevent the meta cache from firing.
#22190 — Delete oEmbed caches on pre_post_update so cached meta is used. Patch: attachment:22190.diff.
#22191 — In update_metadata(), compare old/new values before deciding whether to call add_metadata(). Patch attachment:22191.diff.
#22192 — update_metadata() and update_option() strict checks can cause false negatives. No patch, but this is solved for nav menus only in 22189.3.diff by casting integers to strings.
Also in 22189.3.diff:
- Don't call wp_get_nav_menu_items() in wp_update_nav_menu_item() unless we need to. (Huge.)
- When saving a menu item, only set the menu term against it if it isn't already set. This prevents a wp_get_object_terms() call that can trigger a meta cache we haven't loaded.
- When creating a new menu item, we need to create it then add meta (as we need the post ID to do so). But when updating a menu item, update the meta first. This allows for update_metadata() to use the existing meta cache for old/new comparison, rather than forcing a new meta cache fetch.
#4
@
12 years ago
Some of these things, you may notice, are legitimate bugs or less-than-ideal API behavior. But a decent amount of this is also only a problem when you are saving a ton of posts at once. The only similar situation for core is bulk_edit_posts(), which A) could probably be made faster (I haven't checked) and B) isn't unexpected by the user to be super fast. Try running an operation on a few hundred or thousand Gmail messages. It understandably takes time.
We can probably make some adjustments to make some of this work better at scale. The biggest ones are probably to improve the order in which metadata sets, term sets, and term counting is performed from wp_insert_post(). Some of this would mean new API such as a 'meta' array — #20451 — so things can be done all in one go, either before or after the cache clear, depending. I'm also looking very curiously at the actual need to flush an object's metadata when we clear its cache too (aside from object deletions).
With regards to term counting, what I was seeing is that term counting (triggered by post status changes) was getting deferred, but in order to defer them, term IDs were still fetched for each object (to be entered into the defer queue), which triggered a new term cache fetch for each object. It would be cooler if objects themselves were deferred, that way an enterprising individual could defer term counting, update a ton of posts, fetch the term cache for all of those posts at once, then run the count queue.
Of course, neither term counts (especially on post status changes) are even needed for menu items, which is another story.
Overall, while there is continued room for improvement, this is a quick bite at the apple to speed up menus now.
#7
@
12 years ago
In 22189.2.diff, the logic in line 255 looks reversed: $title
is only used if it's not set.
#14
in reply to:
↑ 11
@
12 years ago
- Resolution fixed deleted
- Status changed from closed to reopened
Replying to nacin:
In [22235]:
When $menu_item_db_id
is zero is_object_in_term()
will return a WP_Error
and the newly created menu item will not be associated with the specified menu. attachment:22189.tax_input-on-create.diff skips is_object_in_term()
if the call is not updating an existing menu item.
#17
@
12 years ago
Was this supposed to lower the number of PHP variables as well as the number of queries? I've upgraded the site to 3.5, but because of a limit of 1000 php variables the menu is still limited to 90 items. Were these improvements not actually integrated into 3.5?
#18
@
12 years ago
More info on max_input_vars
http://anothersysadmin.wordpress.com/2012/02/16/php-5-3-max_input_vars-and-big-forms/
#19
@
12 years ago
- Resolution fixed deleted
- Status changed from closed to reopened
Because of bureaucracy and the fact that I work for a large organization that gets attacked every so often, I can't easily get the limit changed. Would it be feasible for me to go through the .diff files from beginning to end and implement the changes manually? or are they already implemented in 3.5 and I need to find a different solution?
I'm going to re-open this ticket without asking hoping that someone who knows how WordPress works can give a quick answer. Sorry that I'm a noob and I don't know how this ticketing system works at all.
Avoid querying for slugs in wp_unique_post_slug(). We may want to introduce a pre_* filter here in the future.