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

Perform parsing off the main thread when Tree-sitter is enabled #17339

Merged
merged 7 commits into from May 23, 2018

Conversation

Projects
None yet
1 participant
@maxbrunsfeld
Contributor

maxbrunsfeld commented May 16, 2018

Refs #16590

Overview

Currently, all parsing in Atom takes place on the main thread, whether you're using the new Tree-sitter system or the default TextMate system. In some circumstances, parsing can take long enough that it reduces the app's responsiveness. This PR makes it so that when you've enabled Tree-sitter, parsing will take place on a background thread so that it can never delay UI responses.

This will also improve Atom's overall parsing speed, because the native parser is now reading directly from the native superstring TextBuffer instance without any intermediate C++ -> JS -> C++ calls which add overhead and generate garbage.

Tasks

  • Update Atom to use the new Tree-sitter API that allows for multi-threaded parsing
  • When the buffer changes, parse asynchronously
  • When the buffer changes during a parse, enqueue another parse and record the changes
  • Parse buffers asynchronously when initially opening them

Related PRs

tree-sitter/tree-sitter#165
tree-sitter/node-tree-sitter#11
atom/superstring#65
tree-sitter/tree-sitter#170

@maxbrunsfeld

This comment has been minimized.

Contributor

maxbrunsfeld commented May 23, 2018

When editing, parsing no longer impacts the app's frame rate at all:

high-frame-rate

@maxbrunsfeld

This comment has been minimized.

Contributor

maxbrunsfeld commented May 23, 2018

Another interesting twist: parsing asynchronously is great for Atom's frame rate and responsiveness, but can cause Atom to do more work overall (bad for battery life) because for many edits, it has to render twice: once immediately after the edit, and then again once the parsing completes in order to update the syntax highlighting.

Here's a flame graph of this behavior:

sync-operations-before

I've just added a refinement to the algorithm to address this. Now, we perform a limited amount of parsing work immediately on the main thread. If this completes, we can re-render with the correct highlighting from the start. Only when parsing takes a long time do we need to do it in the background. The underlying Tree-sitter API is described in this PR.

Here's the new flame graph for the same editing action as above (the scale is different):

sync-operations-after

Parsing and resolving the parse promise only took half of a millisecond, so it's better for the user's energy usage to finish the parse up front and render only once.

@maxbrunsfeld

This comment has been minimized.

Contributor

maxbrunsfeld commented May 23, 2018

The amount of synchronous work that we allow the parser to do is specified as a number of abstract 'parsing operations'. These don't correspond directly to any unit of time, but they just serve as a rough way to limit the the work we do that has very low overhead to keep track of.

I've currently hard-coded it to 1000 parsing operations. If we want, we could do something more sophisticated like measuring how long parsing operations take on average on the user's machine and tuning the limit appropriately. I don't think the exact number is too important though. We just want some limit on the amount of synchronous work we'll do.

@maxbrunsfeld maxbrunsfeld merged commit a78d682 into master May 23, 2018

3 checks passed

ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@maxbrunsfeld maxbrunsfeld deleted the mb-async-parsing branch May 23, 2018

@daviwil daviwil referenced this pull request May 29, 2018

Closed

Iteration Plan: May 29, 2018 #17426

8 of 17 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment