-
Notifications
You must be signed in to change notification settings - Fork 73
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
Incremental parsing is ineffective when a new tag is opened #23
Comments
This is a great point @marijnh. I don't think I'd ever considered it. I'd love to avoid the performance pitfall, but it does seem like a bit of work. To recap Tree-sitter's current behavior - We discard a subtree if its preceding external scanner state is different from the preceding external scanner state on our parse stack. I think we could generalize this in the following way. When deciding whether or not to reuse a subtree (assuming that it is reusable in all other respects):
When pushing this node onto the parse stack, we would need to add some logic that ensures that we don't allow this node to affect the stored "preceding external scanner state" value associated with our current parse stack. I think the compatibility check could have a signature like this: bool tree_sitter_html_external_scanner_compare(
const void *scanner,
const char *current_state,
const char *state_before_this_subtree
); |
In the case of HTML, the Do you think this would be enough to get good incremental performance, or is the reuse logic too restrictive? Or would we need to provide more information about the subtree to the |
Thanks for the thoughtful answer. In Lezer (where I play decidedly more loose and fast with incremental parsing safety in an attempt to keep tree size and code complexity small) I am now using a thing where a tokenizer can turn off these state checks for node reuse entirely, and I haven't been able to find a counterexample where incremental parsing of HTML actually fails with the check turned off. But that may just mean I haven't looked hard enough. The rules you propose sound more robust. |
@maxbrunsfeld any new developments on this? Seems like an urgent issue specially in html parsing. We also need a quicker, notification about the really changed html nodes |
Due to the way changes in the external scanner state prevent reuse of nodes created with another state, if you open a new tag (which is a common editing operation) somewhere in a big document, everything coming after that tag will be re-parsed. (Same with removing or renaming an opening tag.) This is somewhat similar to the situation when you open a block comment marker, but there the structure of the content actually changes—here you will in most situations just end up creating the exact same nodes.
But not in all situations, I guess, due to the affront to context-freedom that is implicitly closed elements. So I guess this behavior is defensible. But it seems unfortunate. Is it a conscious design decision? Have there been any attempts at workarounds? (Like maybe a mechanism to make dependence on scanner state more fine-grained and disabling the state compare for nodes that have no scanner-state-dependent tokens in them—I think that could be made to work for the HTML case, but it may not be worthwhile in any other situation. I noticed the scanner state approach fits the Python scanner very well—there it encodes exactly the thing you need, without wasting any useful opportunities for reuse.)
As always, feel free to close as 'out of scope'.
Crude benchmark
The text was updated successfully, but these errors were encountered: