-
Notifications
You must be signed in to change notification settings - Fork 29.3k
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
Investigate into the performance issues of the bracket pair colorizer extension #128465
Comments
Please note that there also is separate version Bracket Pair Colorizer 2 which should be faster, however lost some features around custom pairs. |
@alexdima and I did some brainstorming. The fundamental problem is to offer correct bracket highlighting while staying performant. Consider the following TypeScript snippet: function test() { [1]
const str = `
${(() => { if (true) { } })()}
} [2]`;
} [3] Does the bracket at [1] match with the bracket at [2] or at [3]? Thus, perfect bracket pair colorization requires syntax tree information. However, we think that the textmate tokens should be sufficient to identify matching bracket pairs, assuming the tokens are usually correct and that bracket pairs match if they have the same token type and the same level within all brackets of that token type. Simple matching is already implemented in the editor core. However, tokens are not always accurate, especially for long lines and for documents with many lines. The goal should be to avoid iterating over the entire document on each keystroke to compute bracket colors. We identified the following possible solutions for implementing bracket pair colorizing: 1. Do Bracket Pair Colorizing Inside An Extension
1.1. Expose Tokens To Extensions
API usage could look like this: const doc = vscode.workspace.textDocuments[0];
// If a non-undefined value is returned, these tokens must be in sync with `doc`.
// We intentionally don't return a promise here to avoid synchronization issues.
// This API allows to compute tokens in batches to avoid freezing the extension host.
const tokens: { isComputing: false, tokens: ILineToken[] } | { isComputing: true } = doc.getTokensForLine(myLine);
vscode.workspace.onDidChangeTextDocumentTokens((e: { changedRanges: Range[] }) => {
// Is fired immediately after the text document got changed or computation started/finished.
// Calls to `getTokensForLine` can still return `{ isComputing: true }`.
const tokens = doc.getTokensForLine(myLine);
});
// Tokens itself could look like this:
interface ILineToken {
kind: TokenKind,
languageId: string, // e.g. 'typescript' or 'html'
startOffset: number,
endOffset: number
}
// Only provide a very limited amount of token kinds so that
// we don't expose too much textmate flavor.
// We could easily provide the same token information with tree-sitter or monarch.
enum TokenKind {
default,
string,
comment,
regex
} There are various ways of how such an API could be implemented: 1.1.1. Send the Tokens Of The Renderer To The Extension Host
1.1.2. Compute the Tokens In The Extension Host
1.2. Compute the Tokens In The Extension
1.a) Fix either Bracket Pair Colorizer 1 or 2
1.b) Create a New Extension
1.c) Wait for a New Extension by the Community + Provide Guidance
We could also start with 1.b) and encourage authors to fork our extension. 2. Implement Bracket Pair Colorizing Inside The Editor Core
Personally, I would love to do 1.1.2 + 1.c) (with 1.b as fallback and encouraging authors to fork). |
Author of Bracket Pair Colorizer here. I follow this thread with interest, my extension is something that grew a bit out of control, and I grew tired of maintaining it. If there are some quick wins, I can still apply them, but I think my extension is so hacky it is easier to do 1.b or 1.c I coded this thing when I was still in college and it shows 👎 |
Author of Blockman here.
This was the first thing I was thinking when I started working on Blockman, and I think it's very natural idea, because why on earth would an extension re-parse/re-tokenize the file if the host already does it? Internal access to tokens would be super useful, if not all tokens, then at least the tokens which represent the opening and closing locations of each nested block.
Well, maybe, if it's possible, the extension API should have access to the parsing process of host tokens with some option to choose between strict (maximally correct) mode and fast mode. I think maximum correctness must be always ON at least for the tokens which represent the opening and closing locations of each nested block.
I think that's fine. Blockman gets brackets asynchronously and it seems to work fine for most of the users. |
Thanks so much for reaching out! Your extensions not only make VS Code an even more amazing editor, but are also a source for inspiration!
I had a quick look at your source, but I think a much more incremental data structures need to be adopted to get the desired performance - I doubt this is a low hanging fruit.
Did you experiment with notifications to inform users of BC2? When we have a more performant solution in place (however it may look), would you be open to help us getting users who struggle with BC2's performance to adopt that new solution?
The problem here is that at all costs we must prevent the renderer (who currently computes the tokens) from being slow. Synchronizing the tokens with the extension host could be slow, as the extension host is a separate process.
Unfortunately, the tokens produced by textmate grammars don't reflect blocks. |
Why not give the extension developers an option to access host tokens or not to access, so they can choose, they can test it for their specific case and maybe for some situations this approach will not be a deal breaker in terms of speed (even with async access).
What do you mean? Blockman uses the source code of Bracket Pair Colorizer 2 to find the locations of all brackets (which are by themselves the locations of block start and block end), and I believe BPC2 itself uses textmate, and so it provides all the locations (positions) of each bracket with this kind of array: {
char: string;
type: number; // opening or closing
inLineIndexZero: number;
lineZero: number;
}[] |
BC2 seems to postprocess the tokens.
As an extension author myself I can fully understand you. However, from VS Code's perspective, slow extensions do not only degrade the experience of the extension itself, but of the entire product. This is already the case for BC2 - not only is bracket coloring slow for huge documents, but also all the TypeScript features. |
I forked and made a minor edit about a month ago to debounce the tokenizations, was a major improvement for me with no noticeable change in responsiveness. https://github.com/jpcastberg/Performant-Bracket-Pair-Colorizer-2 |
With merging #129231, todays Insider build of VS Code now comes with native bracket pair colorization! We would love to hear your feedback and discuss potential migration paths! Setting As announced, we chose to go with option 2 and the outcome shows that this was the right thing to do. For checker.ts, this new colorizer is up to 10,000 times faster (it takes less than a millisecond to recolorize brackets when you prepend Key-Components of this implementation are:
|
@hediet Will VSCode also support colored line scopes? If it could that would be great, and my extension could be deprecated completely. |
See here: #131001
It would be awesome if you could show a one-time notification to users prompting them to enable the native bracket pair colorization! |
I un-archived https://github.com/CoenraadS/BracketPair & https://github.com/CoenraadS/Bracket-Pair-Colorizer-2/ if you want to make PR. |
Question: I know exposing tokens to extensions is a headache, but is it now possible and non-headache to expose only the positions of those brackets colored by native bracket pair colorization? No tree structure needed, just a simple array of positions. It will be super huge speed upgrade for Blockman (already 19k installs). |
Reopening, as still hundreds of thousands of users seem to have the extension installed and new uses are continuing to install both the old and the new version of the extension. @CoenraadS what do you think about our PRs here and here for your bracket pair colorizer 2 extension to more prominently offer a way to migrate to the native feature? |
I guess the main problem is that the native implementation does not cover all the features yet. At least for me, that's the reason why I continue using the extensions. |
What features do you miss? Our goal is not to have feature-parity, but to have just enough to make 80% of users of the old extension switch. |
|
I haven't been really active around this since for my work I'm using regular Visual Studio, and don't really touch VSCode anymore. But yesterday I finally got around to merging the PR's against my extension to make migration easier. However then I was trying to see how I could make the native functionality match the default settings of BPC2. I would like to see: Then people that migrate from my extension to native would not experience any different behavior. Assuming most people simply using the default settings. |
Thanks for looking at the PRs! I will prioritize #136475 for next release! |
This is the main blocker for me and my team to switch away from the extension: #143484 |
Thanks for mentioning the need for a token data query, @TheFanatr. I have written vscode-textmate-languageservice - a service that replicates Atom's ability to glean language provider data from token heuristics. These ideas are really powerful and in high demand, but the API to expose this data in a performant, platform-agnostic way isn't there. Admittedly I semi-regret the service as there has been complaints it causes painful slowdowns in the Matlab extension. 😦 |
Last week, only ~80k users used Bracket Pair Colorizer 1 (decreasing tendency). Thus I would say these extensions are no longer used and we can finally close this issue. |
Bracket Pair Colorizer is a very popular VS Code extension. However, users are reporting performance problems as the following tweet illustrates.
We should investigate into how the performance can be improved.
The text was updated successfully, but these errors were encountered: