WordPress.org

Make WordPress Core

Opened 7 months ago

Last modified 3 days ago

#51469 reviewing enhancement

Use `_prime_post_caches` for speeding up cached `get_pages` call

Reported by: david.binda Owned by: SergeyBiryukov
Milestone: 5.8 Priority: normal
Severity: normal Version:
Component: Posts, Post Types Keywords: has-patch needs-testing has-testing-info
Focuses: performance Cc:

Description

The get_pages function uses a cache containing ID of pages matching params of a previous call.

The IDs are, on a subsequent call with the same params, inflated using the get_post function call. This works well in terms of the same request, as all the pages were previously added to the in-memory cache.

However, on a subsequent request, when the cache is hit, there are likely no pages already in the in-memory cache, and those need to be fetched from the backend cache server, one by one.

With the new wp_cache_get_multiple, sites with persistent cache backend could improve the speed in such cases, in case the code used the _prime_post_caches (which internally uses the _get_non_cached_ids function, which is taking advantage of the mentioned wp_cache_get_multiple).

But also sites without a persistent cache backend could benefit from the bulk SQL query constructed by the _prime_post_caches function, instead of fetching each page via individual SQL query.

The performance gains should be most noticeable in case a site has a lot of pages which are being requested via get_pages function.

IMHO, a _prime_post_caches( $cache, false, false ); should be used to match the behaviour of the get_pages function when the pages are obtained from the database (thus, no post meta, nor taxonomies priming).

Attachments (1)

51469.diff (527 bytes) - added by david.binda 7 months ago.

Download all attachments as: .zip

Change History (10)

@david.binda
7 months ago

#1 @david.binda
7 months ago

But also sites without a persistent cache backend could benefit from the bulk SQL query constructed by the _prime_post_caches function, instead of fetching each page via individual SQL query.

This does not apply, as the initial wp_cache_get on a subsequent request on a site without persistent object cache backend would not get anything.

But, IHMO, the rest of the report still applies for sites with persistent object cache backend :)

#2 @SergeyBiryukov
5 months ago

  • Milestone changed from Awaiting Review to 5.7
  • Owner set to SergeyBiryukov
  • Status changed from new to reviewing

This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.


4 months ago

#4 @hellofromTonya
4 months ago

  • Keywords has-patch added

#5 @hellofromTonya
4 months ago

  • Keywords needs-testing added

Next steps for this ticket (from core scrub):

Sergey will "confirm the initial issue and verify that the patch does fix it as expected."

Adding needs-testing to test for the performance gains:

From Sergey:

Would be great to confirm the performance improvements, looks good to go otherwise

Steps to test the performance improvements:

  1. Use a persistent object cache (Redis or Memcached).
  2. Have a site with a lot of pages.
  3. Go to the the Pages screen (since it uses the get_pages() function being patched here) and measure the response time (TTFB, time to first byte).
  4. Apply the patch and see if there's any improvement.

This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.


3 months ago

This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.


3 months ago

#8 @hellofromTonya
3 months ago

  • Milestone changed from 5.7 to 5.8

Tomorrow is 5.7 Beta 1. We've run out of time for this ticket in this cycle. Punting to 5.8.

If any maintainer or committer feels this can be resolved in time, or wishes to assume ownership during a specific cycle, feel free to update the milestone accordingly.

#9 @hellofromTonya
3 days ago

  • Keywords has-testing-info added

This ticket is flagged for performance manual testing. Comment 5 has the step-by-step process to do this testing. Marking the ticket ready for testing, i.e. via the combination of keywords: has-patch needs-testing has-testing-info

Note: See TracTickets for help on using tickets.