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

Add caching of post processor output #959

Closed
westonruter opened this Issue Feb 14, 2018 · 2 comments

Comments

3 participants
@westonruter
Copy link
Member

westonruter commented Feb 14, 2018

The preprocessor/sanitizer can be an expensive series of operations. If we take a hash of the pre-sanitized HTML and use this as the cache key for caching the sanitized output, it would be possible to short-circuit the entire sanitization process by serving the cache output directly. This would not preclude full page caches since PHP would still be generating the buffered output, but it would be a good way to augment full page caches. Props @ThierryA for the idea.

@westonruter

This comment has been minimized.

Copy link
Member Author

westonruter commented Apr 3, 2018

This can be done after #1048 since it includes the Server-Timing response headers which are key for verifying that it is working.

The cache key used for should consist of a hash of the output buffer with the plugin version and perhaps the list of sanitizers and embed handlers being used. If a plugin or theme is installed which adds another sanitizer or embed handler, this will impact the sanitizer/post-processor output, so it should be part of the cache key.

Reference the code used in the CSS processor caching: https://github.com/Automattic/amp-wp/blob/14c22c2f25490df3d1899fa1f71e969574372d90/includes/sanitizers/class-amp-style-sanitizer.php#L496-L515

I don't think that a transient should necessarily be used for caching the output since if an external object cache is not installed this can result in the options table exploding with transients. That being said, transients are being used now to store the result of image dimension lookup requests, but that is not ideal either. See also #960 where we'll warn users that they should have an external object cache present.

@westonruter westonruter added this to the v1.0 milestone Apr 3, 2018

@postphotos postphotos added this to Definition in v1.0 Apr 19, 2018

@postphotos postphotos moved this from Definition to To do in v1.0 May 2, 2018

@ThierryA ThierryA self-assigned this May 16, 2018

@westonruter

This comment has been minimized.

Copy link
Member Author

westonruter commented May 16, 2018

Caching should be disabled if WP_DEBUG is enabled.

@ThierryA ThierryA moved this from To do to In progress in v1.0 May 17, 2018

@ThierryA ThierryA changed the title Add caching of preprocessor output Add caching of post processor output May 17, 2018

@ThierryA ThierryA moved this from In progress to Ready for review in v1.0 May 22, 2018

@westonruter westonruter moved this from Ready for review to Ready for QA in v1.0 May 31, 2018

@kienstra kienstra moved this from Ready for QA to Ready for Merging in v1.0 Jun 6, 2018

@kienstra kienstra moved this from Ready for Merging to In Production in v1.0 Dec 11, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.