Skip to content
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

Store block template object in object cache. #5467

Draft
wants to merge 2 commits into
base: trunk
Choose a base branch
from

Conversation

spacedmonkey
Copy link
Member

@spacedmonkey spacedmonkey commented Oct 12, 2023

Trac ticket: https://core.trac.wordpress.org/ticket/59600


This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

}

$theme = ! isset( $template_file['theme'] ) ? $template_file['theme'] : get_stylesheet();
$template->content = file_get_contents( $template_file['path'] );
Copy link
Member Author

Choose a reason for hiding this comment

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

@felixarntz The massive performance benefits you saw in your PR are from this line. At the moment, EVERY time a block template is loaded, it is read into memory. In the case of TT4, this means loading, post meta template for every post on the page. In my test site, around 10 times. This is wasteful.

$template_content = file_get_contents( $template_file['path'] );
$theme = get_stylesheet();
$template = new WP_Block_Template();
$cache_key = md5( serialize( $template_file ) );
Copy link
Member Author

Choose a reason for hiding this comment

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

$template_file is array, with contains theme, slug and path. Making it a good cache key.

@@ -881,7 +881,7 @@ function wp_start_object_cache() {
)
);

wp_cache_add_non_persistent_groups( array( 'counts', 'plugins', 'theme_json' ) );
wp_cache_add_non_persistent_groups( array( 'counts', 'plugins', 'theme_json', 'block_template' ) );
Copy link
Member Author

Choose a reason for hiding this comment

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

@felixarntz This is the most important part of this PR. This caches can never be a be a transient cache. Files can be edited local or by uploading via FTP. This will trigger no cache invalidation.

Co-authored-by: Mukesh Panchal <mukeshpanchal27@users.noreply.github.com>
@felixarntz
Copy link
Member

@spacedmonkey This approach caches more data than my PR #5463, but the concerns that you shared in #5463 (review) apply to this PR as well, as far as I can tell.

Can you please clarify what the benefits of this PR are over #5463?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants