Make WordPress Core

Opened 6 years ago

Last modified 3 years ago

#36723 new enhancement

Add caching to wp_old_slug_redirect

Reported by: spacedmonkey Owned by:
Milestone: Priority: normal
Severity: normal Version: 2.1
Component: Posts, Post Types Keywords: has-patch dev-feedback needs-unit-tests
Focuses: performance Cc:

Description

The wp_old_slug_redirect function is called on every page. It perform a raw sql query everytime it is run. The result of that query should be cached for performance reasons.

Attachments (1)

36723.patch (899 bytes) - added by spacedmonkey 6 years ago.

Download all attachments as: .zip

Change History (6)

@spacedmonkey
6 years ago

#1 @spacedmonkey
6 years ago

  • Keywords has-patch dev-feedback needs-unit-tests added

#2 @dd32
6 years ago

The wp_old_slug_redirect() function only runs when the current pageload is marked as a 404, based on the is_404() check at the start of the function.

I don't think caching is the answer here, rather the custom code you're using to make a "fake" page needs to flag the page as not-a-404 to WordPress.

#3 follow-up: @spacedmonkey
6 years ago

Sorry @dd32 I am not sure what you mean by "fake" page. The patch just caches the response from sql. That is then invalided when the clean_post_cache function is called and the last_changed value changes. I know WordPress VIP are doing something similar with caching the result of this function. Still not sure why this needs to call sql on every time. You could easily have a popular url that is 404ing as you remove the post. This just increase traffic to sql unnecessarily.

#4 in reply to: ↑ 3 @dd32
6 years ago

Replying to spacedmonkey:

Sorry @dd32 I am not sure what you mean by "fake" page.

Sorry, it was an assumption on my part - When someone says that a function that has a is_404() check in it is running every page, it generally means they're running a plugin which injects a "fake" page (one that doesn't actually result in a proper WP_Query object being located).

What I mean, is that the only time that query should ever be run, is in the case where a singular page (a non-hierarchical one at that) is requested and cannot be found.
Caching the function in that case doesn't seem to be the best location for it - it seems like the redirect should be cached instead to me.

#5 @spacedmonkey
6 years ago

I miss spoke when I said it was on every page load. I meant it hooked on template redirect on every page load.

So a little back story on this. I run a large high traffic multisite. Lots of editorial teams. I also use object caching to help with performance. While looking into sql log I found a high number of calls to this sql query. We don't cache 404s in our cdn, so this function is results in a high amount of sql traffic.

I am not sure it is possible to cache the redirect it's self. After getting the post ID it calls get permalink. The value of the permalink can be dynamic, as either it is filtered or the home_url changes, (ssl or domain mapping). I think that just save the result of sql is the best option, as it lets those filters run and doesn't break backwards compatibility.

Note: See TracTickets for help on using tickets.