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

Performance: Async Syntax Highlighting #922

Merged
merged 72 commits into from Dec 14, 2019
Merged

Conversation

bryphe
Copy link
Member

@bryphe bryphe commented Nov 11, 2019

This is a follow-up to #866 - the issue being that, even though our textmate syntax highlighting (reason-textmate is faster than the JS syntax highlighting strategy when run in isolation: https://github.com/onivim/reason-textmate#performance ) - it's actually slower in wall-clock time when run in the editor.

The problem is that we are 'time-slicing' our syntax highlighting - giving it a budget of 2ms per frame. This means we are running it at 1/8 the potential speed (assuming a frame time of 16ms) - essentially 8x slower.

#866 was an attempt to move this to an OCaml thread to run in parallel, but unfortunately this is where the lack of multicore is problematic - most the of operations are CPU bound (the processing of the regex), so it causes delays in the rendering / responsiveness. When multicore is in place - threading will be a viable option, and we can revisit!

However, without OCaml multicore, we have to go back to running the syntax highlighting in a separate process - this will allow us to still get parallelism. The implementation can be simpler and more performant than our previous node-textmate strategy, which used JSON-RPC, because we can use OCaml's efficient Marshal module as the bridge, and we can strongly-type both ends of the protocol in Reason, and leverage the native reason-textmate parsing (and reason-tree-sitter.

TODO:

  • Send language info to client
  • Send theme to client
  • Create type for managing the state in Oni_Syntax_Server
    • Track buffers / buffer updates in state
    • Move SyntaxHighlighting.t there
  • Update theme when it changes
  • Run syntax highlight job continuously in Oni_Syntax_Server
  • Create simpler client-side state for managing syntax highlighting - just the tokens
  • Wire up 'textmate' strategy again
    • Hook up grammar discovery
    • Add getUpdatedLines to job / interface
    • Add clearUpdatedLines to job / interface
  • Wire up 'treesitter' strategy again
    • Add TreeSitterConverter repository (like GrammarRepository but for tree-sitter)
    • Hook up tree-sitter json discovery
    • Add getUpdatedLines to job / interface
    • Add clearUpdatedLines to job / interface
  • Hook up configuration / configuration changes (to decide if we need to pick up tree-sitter)
  • Hook up logic to determine which highlight strategy to use
  • Add rei interfaces + review and consolidate
  • Implement 'shift' logic when adding / removing lines
  • Debug issue where new characters are not highlighted with previous token (this was an pre-existing issue, will test further to explore a fix in separate PR)
  • Add integration test for syntax highlighting
    • Windows: Fix crash when spinning up syntax server
  • Turn on 'C' tree-sitter? (will defer to a separate change)
  • Keep track of ranges that were updated, send those up, and clear them
    • Store updated ranges as part of job
    • Add 'getPendingRanges'
    • Call 'clearPendingRanges'
  • Add health check for syntax server
  • Test release builds
    • OSX
    • Windows
    • Linux

... later - keyword extraction would be nice to have!

src/Syntax_Server/State.re Outdated Show resolved Hide resolved
src/Syntax_Server/State.re Outdated Show resolved Hide resolved
@bryphe bryphe force-pushed the experiment/syntax-service-v2 branch from 89d2831 to 218eede Compare December 13, 2019 02:05
@bryphe bryphe changed the title [WIP] Performance: Async Syntax Highlighting Performance: Async Syntax Highlighting Dec 13, 2019
@bryphe
Copy link
Member Author

bryphe commented Dec 14, 2019

/azp run

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 this pull request may close these issues.

None yet

2 participants