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

Allow tokenizer to pause and resume in the middle of a line #1444

Merged
merged 1 commit into from Apr 1, 2023

Conversation

Guldoman
Copy link
Member

Fixes #1212 and #1369.

For now the pause policy is that every 200 tokenized chars, if half a frametime has passed, it "yields".
We might consider allowing the customization of the pause policy.

The yielding behavior is not implemented with coroutines because of performance concerns around creating them. I might test if this is a valid concern.

Only tokenization coroutines are allowed to tokenize whole lines.
Highlighter:get_line only tokenizes the first 200 characters, plus anything else it can in the allowed time frame.
This is done because that function can be called multiple times per frame, which could tank the performance. We might consider adding a parameter to allow resuming the tokenization.

When the tokenizer pauses, the remaining text uses the incomplete token type. Suggestions for a different name are welcome.

We sadly can't resume the tokenization of a line from where it changed, and need to restart the tokenization from scratch.
This is because the change itself could match with a rule that applies before it.

Testing is very welcome.

@Guldoman Guldoman linked an issue Mar 27, 2023 that may be closed by this pull request
@Guldoman Guldoman force-pushed the PR_yieldable_tokenizer branch 3 times, most recently from d54c3f5 to a1035c8 Compare March 27, 2023 23:20
@adamharrison
Copy link
Member

For now the pause policy is that every 200 tokenized chars, if half a frametime has passed, it "yields".
We might consider allowing the customization of the pause policy.

I think it's fine; very few people are going to customize this. If anyone wants to, they can complain, and we can put in a config variable, but I really don't think it's necessary.

The yielding behavior is not implemented with coroutines because of performance concerns around creating them. I might test if this is a valid concern.

I honestly think it's fine; it's clear enough, and not a huge amount of code.

Only tokenization coroutines are allowed to tokenize whole lines.
Highlighter:get_line only tokenizes the first 200 characters, plus anything else it can in the allowed time frame.
This is done because that function can be called multiple times per frame, which could tank the performance. We might consider adding a parameter to allow resuming the tokenization.

Seems reasonable.

When the tokenizer pauses, the remaining text uses the incomplete token type. Suggestions for a different name are welcome.

I'm good with this.

We sadly can't resume the tokenization of a line from where it changed, and need to restart the tokenization from scratch. This is because the change itself could match with a rule that applies before it.

Honestly, that's fine, and reasonable for now. If it becomes an issue with people who really want to edit really long lines, while tokenizing them, we can always revisit.

I'm going to do a bit more testing, and if it looks good, I'll merge.

@adamharrison
Copy link
Member

Yeah, tested; looks good. Merging.

@adamharrison adamharrison merged commit 86fa90a into lite-xl:master Apr 1, 2023
8 checks passed
takase1121 pushed a commit to takase1121/lite-xl that referenced this pull request Aug 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Lite XL really hates really long lines Editor freezes when opening a json file
2 participants