Don't call setTheme if _theme hasn't changed
#151
Conversation
`setTheme` can be an expensive operation because it parses the raw theme every time it's called: https://github.com/microsoft/vscode-textmate/blob/9e3c5941668cbfcee5095eaec0e58090fda8f316/src/main.ts#L93-L95 In https://github.com/banga/git-split-diffs/, I invoke `codeToThemedTokens` many times, one line at a time, so the cost of `setTheme` adds up. In my local profiling, this was taking about 25% of the time spent in `codeToThemedTokens`. I realize this is probably not the intended way to use this function, but there isn't an easy alternative for syntax highlighting individual lines in a diff.
|
cc @octref in case you haven’t seen this PR. Thanks! |
|
Makes sense, thank you! |
|
Hey @banga, I just saw your repo on HN frontpage, congrats! |
|
Thanks and thank you for making this project!
Yes of course!
Yeah that sounds useful. The initial implementation was actually really easy. I used chalk which already supports hex colors and converts them to appropriate ANSI sequences for the current terminal. I eventually had to do more work for supporting alpha in colors (so I could e.g. highlight inline changes without overwriting the underlying syntax highlighting), so I implemented https://github.com/banga/git-split-diffs/blob/main/src/SpannedString.ts. It felt like I was sort of reinventing html, but I needed something tailored to my use-case of highlighting spans in a line, one line at a time, and then flattening it into styled segments. Slightly unrelated, but one thing I'm invested in for my project is speeding up startup time – we currently spend a bunch of time in parsing the grammars and themes to in-memory structures. I know very little about the internals of vscode-textmate, but it seems that there might be a way to cache this parsed version on disk and quickly load that in instead? That would be helpful in enabling more CLI apps to use this project, since developers tend to be really sensitive to perf issues in those. |
@banga I just came across this issue again. I currently don't have time to look into this yet. Would be great if that could happen. |
I tried this today, but it was actually slightly (~6%) slower in my tests. This is what I tried: microsoft/vscode-textmate@main...banga:main. Separately, I noticed that after updating to |
I just opened #200.
To avoid losing contexts, do you mind opening a new issue so we can track this? Plan to fix this, but needs more digging into it. |
setThemecan be an expensive operation because it parses the raw themeevery time it's called:
https://github.com/microsoft/vscode-textmate/blob/9e3c5941668cbfcee5095eaec0e58090fda8f316/src/main.ts#L93-L95
In https://github.com/banga/git-split-diffs/, I invoke
codeToThemedTokensmany times, one line at a time, so the cost ofsetThemeadds up. In my local profiling, this was taking about 25% ofthe time spent in
codeToThemedTokens. I realize this is probably notthe intended way to use this function, but there isn't an easy
alternative for syntax highlighting individual lines in a diff.
Before
After
