WordPress.org

Make WordPress Core

Opened 11 months ago

Last modified 11 months ago

#45471 new enhancement

Allow caching of parse_blocks results

Reported by: joostdevalk Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: Cache API Keywords: has-patch dev-feedback
Focuses: performance Cc:
PR Number:

Description

A *lot* of Gutenberg implementations are going to have to parse the blocks in a post. Core itself already parses the blocks on output, but also when trimming an excerpt in excerpt_remove_blocks. All this parsing is done with parse_blocks. Unfortunately, the only thing passed to parse_blocks right now is a string, with no way of caching it.

My suggestion would be to add a cache key to the parse_blocks function, which defaults to false. I've attached a proposed patch to the function, if we agree on this we could then look at how to implement this in core itself.

Attachments (1)

cache-key.patch (1.1 KB) - added by joostdevalk 11 months ago.
Cache key patch

Download all attachments as: .zip

Change History (6)

@joostdevalk
11 months ago

Cache key patch

This ticket was mentioned in Slack in #core-editor by joostdevalk. View the logs.


11 months ago

This ticket was mentioned in Slack in #core-editor by desrosj. View the logs.


11 months ago

#3 @dmsnell
11 months ago

This is a good idea, definitely valid for re-parses.

The cache key however appears to be based purely on the content of the document though and not on the parser, which is filterable. I think that in almost all cases this will be fine as the parser won't likely change within a cache session, but perhaps we could rewrite the patch to account for that.

For instance, we could probably just add the name from the parser class given in the filter to the cache key. On this point, why pass in the cache key and default to false at all? I'm probably missing something but what key would we use other than one we could autogenerate based on content and parser?

function parse_blocks( $content ) {
        $parser_class = apply_filters( 'block_parser_class', 'WP_Block_Parser' );
        $cache_key    = sprintf( 'block_%s_%s', md5( $content ), $parser_class );

        $cached = wp_cache_get( $cache_key, 'parsed_blocks' );
        if ( $cached ) {
                return $cached;
        }

        $parser = new $parser_class();
        $parsed = $parser->parse( $content );

        wp_cache_add( $cache_key, $parsed, 'parsed_blocks' );

        return $parsed;
}

Are we trying to only turn on caching when we think or know we will need it?

Also, at this point is there a reason not to create a PR in the Github repo?

Thanks! This is great having more help on this area.

#4 @desrosj
11 months ago

In the current state of the patch, a cached empty string would not be returned and would always trigger parsing. if ( false !== $cached ) should be used if an empty string should be returned when cached.

This ticket was mentioned in Slack in #hosting-community by joostdevalk. View the logs.


11 months ago

Note: See TracTickets for help on using tickets.