Skip to content

Add in-memory caching to QueryRunner #475

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 22, 2025

Conversation

chriszarate
Copy link
Member

This is a performance and developer experience improvement to remove redundant query executions during a single request. It implements a simple in-memory cache in QueryRunner so that repeated identical queries are quickly resolved from cache.

Why is in-memory caching beneficial?

Mainly, because block bindings with identical queries are often repeated within a remote data block. For example, this remote data block has 7 children core blocks with block bindings, each of which needs the same data from the exact same query:

Screenshot 2025-04-21 at 15 24 30

Currently, for this example, our block binding resolver function executes the exact same query 7 times. Via our Guzzle client, the redundant query executions resolve from object cache. On top of this, WP Core implements its own in-memory cache for object cache, which means that there is only one network request to object cache.

So why do we need this additional caching? Two main reasons:

  1. There is additional post-processing that happens after the query is executed. The response is parsed as JSON and traversed recursively by JsonPath in order to extract the values defined by the query's output schema.
    • In aggregate across many blocks (or across many posts on an archive page), this can be expensive.
    • Additionally, the plugin user has the opportunity to define their own callback functions to generate or transform this output, which might contain expensive code. They might have the (very reasonable) expectation that these callback functions would only be called once.
  2. While we will soon cache error responses in object cache (Cache error responses for a short time #474), some errors unavoidably manifest as exceptions that must be caught, and therefore cannot be cached by Guzzle middleware. We do not want to repeat these requests many times over within the same request.

It has a small side benefit of removing the appearance of multiple calls in our logging and debugging tools:

Before After
Screenshot 2025-04-21 at 15 30 07 Screenshot 2025-04-21 at 15 29 46

Why implement in-memory caching here?

The QueryRunner implements query execution and response parsing, so it feels like a natural location. We could implement caching at the block binding level, but use cases that executed queries directly (e.g., custom code, REST API, interactivity API) would not benefit.

Another option is to inject the post-processing logic into our Guzzle middleware and cache the result, but that could result in breakage when processing code is updated. It's much safer to cache the raw API response.

Copy link
Contributor

Test this PR in WordPress Playground.

Copy link
Contributor

@ingeniumed ingeniumed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one item that needs to be tweaked, otherwise this is good with me.

@@ -152,7 +153,6 @@ protected function get_raw_response_data( HttpQueryInterface $query, array $inpu

return [
'metadata' => [
'age' => intval( $response->getHeaderLine( 'Age' ) ),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed that this is still being referenced in QueryRunner#get_response_metadata, just need to remove that.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opted to restore it for now in 64549e9

@chriszarate chriszarate merged commit a260454 into trunk Apr 22, 2025
13 checks passed
@chriszarate chriszarate deleted the improve/query-runner-in-memory-cache branch April 22, 2025 12:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants