Blocks: Try parse caching #66373
Draft
Blocks: Try parse caching #66373
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tyxla
added
[Type] Code Quality
|
Size Change: +46 B (0%) Total Size: 1.81 MB
ℹ️ View Unchanged
|
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
tyxla
requested review from
mcsf,
Mamaduka,
youknowriad,
ntsekouras,
a team and
ellatrix
|
Quick impressions: I don't think this is wise. In no particular order of importance:
Do I think it's worth looking at instances of superfluous block parsing in Gutenberg? Yes. Some thoughts:
|
|
Share the same ideas/concerns as @mcsf In addition to that, I'll say that in my experience, parsing has been an issue in two places:
Feels like there's potentially a universal solution in adding "virtual/transient" lazy and cached properties to entities. |
tyxla
added
the
[Status] Blocked
|
Thank you for the feedback, folks! It was quite clear that it won't be that simple. I will consider some alternatives based on what you suggested. I'll keep the PR open for now, since it helps with evaluating the impact |
Lately I’ve been trying hard to unify efforts to improve parsing performance for these kind of “inspection” related operations around moving towards a low-overhead streaming parser. WordPress/wordpress-develop#6760 This is in PHP but a JavaScript port is trivial. The new streaming parser is lazy and can stop parsing whenever the desired question is answered, such as “is there a So while I also applaud the efforts to improve the full block tree parser, I encourage folks working on parsing related problems to consider if the greater wins are available outside of the current profiling metrics: can we go faster by doing less (streaming and avoiding the construction of a tree) rather than by adding more (caching and complexity)? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
This PR attempts to cache the block parser function.
Why?
Initially, the experimentation with React Compiler in #66361 brought me here. I was getting a bunch of errors, which seemed to be occurring while parsing blocks.
Then I realized, if we
parse()10 times a simple string like this:we'll always get 10 different arrays, with 10 different paragraph
WPBlockobjects. That's probably fine at first glance, but given that those blocks later influence the entire editor and cause re-rendering virtually everything (different client IDs in practice means different blocks, even if all block properties and attributes are the same). At a large scale, it might actually influence the editor performance. So, wouldn't it make sense if the same input always produces the same output (at least in a single user session)?Disclaimer: this may be a very naive conclusion: I'm not that familiar with the block parser, and I'm only trying out things. Ideally, assume that I'm missing something.
How?
To get the
parse()function to be more predictable, I'm introducing some naive caching toparse(). It's quite straightforward, because the input is a string, and an object of options. I'm building a predictable string key out of these 2 arguments and using that key for caching theparse()output.It was interesting that once I introduced the caching, I saw the errors reported from React Compiler drop from 65 to 7!
This appears to be because we're calling
parse()in some functions that are memoized, and with React Compiler,useMemo()doesn't seem to like functions that return different output on different calls.Testing Instructions
Testing Instructions for Keyboard
Same
Screenshots or screencast