Skip to content

[syntax-coloring] Rework the syntax map to use offset + length and simplify the delta logic #10289

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

Conversation

nathawes
Copy link
Contributor

This patch changes the syntax map data structure to be offset based rather than line/col based in order to avoid calling getLineAndColumn for the start and end offset of every token. This removes the 30% of time spent in getLineAndColumn during this request for large files (rdar://problem/28965123).

The logic for returning the affected range and the token ranges to highlight following an edit also made several assumptions that no longer hold. This patch changes it to compare the syntax maps from before and after the edit, find the first mismtaching tokens from the start and end of the syntax maps
and return the tokens in that range (adjusted to line boundaries). This fixes syntax highlighting issues with interpolated multi-line strings (rdar://problem/32148117) and block comments.

With the above changes the per-keystroke time spent for syntax highlighting (with sematic info disabled) drops from ~80ms to just under 50ms for a 12KLOC file.

This patch also changes unterminated/invalid regular and multi-line strings to now be highlighted as strings.

Resolves rdar://problem/28965123 and rdar://problem/32148117.

Nathan Hawes added 4 commits June 15, 2017 13:15
…han start/end line+column and simplify the delta logic

This patch changes the syntax map data structure it uses to be offset based
rather than line/col based in order to avoid calling getLineAndColumn for the
start and end offset of every token. This removes the 30% of time spent in
getLineAndColumn for this request in large files (rdar://problem/28965123).

The logic for returning the affected range and the token ranges to highlight
following an edit also made several assumptions that no longer hold. This
patch changes it to compare the syntax maps from before and after the edit,
find the first mismtaching tokens from the start and end of the syntax maps
and return the tokens in that range (adjusted to line boundaries). This fixes
syntax highlighting issues with interpolated multi-line strings
(rdar://problem/32148117) and block comments.

With the above changes the per-keystroke time spent for syntax highlighting
(with sematic info disabled) dropped from ~80ms to just under 50ms for a
12KLOC file.
…gs as strings

Also add tests for syntax map deltas when editing multi-line strings
@nathawes nathawes requested a review from akyrtzi June 15, 2017 20:28
@nathawes
Copy link
Contributor Author

@swift-ci Please smoke test

@nathawes nathawes removed the request for review from akyrtzi June 15, 2017 23:21
Copy link
Contributor

@bitjammer bitjammer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good. I would suggest you add doc comments for the functions you've touched and also add some developer comments for some of the more opaque stuff like comparison of ranges and offsets, why you're checking them, etc.

@nathawes
Copy link
Contributor Author

@swift-ci Please smoke test

@nathawes nathawes force-pushed the rdar32148117-multiline-syntax-coloring-issues branch from 09824ef to 992b6b8 Compare June 16, 2017 20:56
…o new highlighted tokens but are removed ones, and take account of the range of the mismatching token in the previous syntax map

We still need to adjust the affected range to the line boundaries and return all
tokens on the line when there are no new tokens, as the client will clear all
tokens on that line in its copy of the syntax map leaving the other tokens
unhighlighted. We also need to extend the affected range to include the ranges
of the mismatched tokens from the previous syntaxmap, so their highlighting will
be cleared.

Also add more comments to better document the new syntax map structure and
behaviour.
@nathawes nathawes force-pushed the rdar32148117-multiline-syntax-coloring-issues branch from 992b6b8 to 3791e12 Compare June 19, 2017 19:41
@nathawes
Copy link
Contributor Author

@swift-ci Please smoke test

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.

2 participants