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

Proposal: Replace the token queue with an event-handler system #403

Closed
fb55 opened this issue Feb 11, 2022 · 0 comments · Fixed by #419
Closed

Proposal: Replace the token queue with an event-handler system #403

fb55 opened this issue Feb 11, 2022 · 0 comments · Fixed by #419

Comments

@fb55
Copy link
Collaborator

fb55 commented Feb 11, 2022

Why

The token queue adds a level of indirection that makes it harder to fix some issues. Eg. #292 is easy to fix once the token queue is gone. Also, debugging is currently complicated, as stack traces end at the token queue.

With the queue gone, stack traces will point at the corresponding line in the tokenizer. V8 will be able to optimise more aggressively; in my branch combining all of the changes, I see a ~15% performance increase using htmlparser-benchmark.

Game plan

  1. Update the tokenizer to produce events. There will be a QueuedTokenizer class that wraps around the tokenizer, which provides an interface for the parser. Opened as refactor(tokenizer): Introduce events #404
  2. Invert event processing in the parser. The parser currently first checks the insertion mode, and then the token type. By inverting this (checking first the token type, then the insertion mode), we prepare the parser to accept the events from (1). Opened as refactor(parser): Invert event processing #405
  3. Tie everything together. Have the updated parser from (2) consume the tokenizer events from (1). Opened as refactor(parser): Consume tokenizer events #419

(1) and (2) do not depend on one-another and can be merged independently.

cc @wooorm @43081j

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

Successfully merging a pull request may close this issue.

1 participant