Make WordPress Core

Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#48838 closed enhancement (fixed)

Consider "caching" embedded REST API requests

Reported by: timothyblynjacobs's profile TimothyBlynJacobs Owned by: timothyblynjacobs's profile TimothyBlynJacobs
Milestone: 5.4 Priority: normal
Severity: normal Version: 4.4
Component: REST API Keywords: has-patch has-unit-tests commit
Focuses: performance Cc:

Description

Inspired by #46249. What if the REST API "cached" embeds processed by response_to_data. This would help when embedding on collection responses with posts that share property values.

We'd use the URL as the cache key. And the caching should be safe since it'd be isolated to response_to_data and we could clear it after response_to_data completes. The requests are also all GET requests.

Attachments (3)

48838.diff (3.7 KB) - added by TimothyBlynJacobs 5 years ago.
48838.2.diff (5.4 KB) - added by TimothyBlynJacobs 5 years ago.
48838.3.diff (5.4 KB) - added by kadamwhite 5 years ago.
Resolve PHPCS errors

Download all attachments as: .zip

Change History (8)

#1 @TimothyBlynJacobs
5 years ago

  • Keywords has-patch has-unit-tests 2nd-opinion added
  • Owner set to TimothyBlynJacobs
  • Status changed from new to assigned

I uploaded a first pass of what this might look like.

#2 @kadamwhite
5 years ago

This is really clever, and seems like a good idea. The potential savings feel large; going to test locally to validate, then consider landing this soon to try to shake out any potential impact before beta.

#3 @kadamwhite
5 years ago

  • Keywords commit added; 2nd-opinion removed

This didn't show much benefit on a site with little data, but I tested this by setting up a staging environment with 10k posts and it quicky began to yield significant benefits. Methodology:

  • wp post generate --count=10000 --post_author=admin
  • Run 20 REST requests for /wp-json/wp/v2/posts?per_page=100&_embed and record response time

This yielded an average response time of 8.738s on master, and 5.174s with this patch: a 40% improvement.

The likelihood of an enterprise-scale site only having posts by one user is low, so this should be seen as a hypothetical upper-boundary of performance gain this patch could yield. The reason for the variance is that on a vanilla WordPress site the only embeddable resource with a consistent-per-linked-resource link href is author; resources like terms will have a link specific to the post, such as /wp-json/wp/v2/categories?post=7240, which would miss our cache.

However, in discussion with @TimothyBlynJacobs he mentioned that he's seen plugins use links which would have consistent href's but could be potentially costly to re-compute, as plugins usually make less consistent use of internal object caching and other best practices we try to follow in core. This implies that this patch would both benefit the long-tail of WP sites with little content but many plugins, as well as the smaller set of enterprise-scale sites with thousands of posts or users.

Marking as commit as this appears to yield a real benefit in some situations, with no obvious downsides. Thanks Timothy!

@kadamwhite
5 years ago

Resolve PHPCS errors

#4 @kadamwhite
5 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 47138:

REST API: Reuse previously-generated embedded objects when building collection response.

Store each generated embedded object in a temporary cache when querying for linked resources so that repeated links to the same resource do not trigger repeated queries or processing.

Props TimothyBlynJacobs.
Fixes #48838.

#5 @TimothyBlynJacobs
5 years ago

  • Milestone changed from Awaiting Review to 5.4
Note: See TracTickets for help on using tickets.