search

microsoft / vscode-cpptools

Official repository for the Microsoft C/C++ extension for VS Code.
Other
5.54k stars 1.57k forks source link

Re-lexing (`lex_document_to_line`) can be replaced with use of cached lexer line states. #3126

Open sean-mcmanus opened 5 years ago

sean-mcmanus commented 5 years ago

We are re-lexing (a certain percentage of) the document for every hover or go to definition (instead of caching/resuing the results). For hover, it's a regression from adding document comments. Hover is usually expected to be faster than Go to Def, and with hover it just shows "Loading...", but Go to Def shows a progress bar indicator. Maybe the large file scenario isn't important enough (i.e. sqlite3.c).

bobbrow commented 1 month ago

In the case of sqlite.c, what I'm seeing appears to be delays caused by reference highlighting. Hover and GTD are fast when the reference highlighting is already done. Unless you right click on the symbol, GTD requires cursor positioning which queues up the reference highlights first,

Colengms commented 1 month ago

@bobbrow . In Sean's original issue he referred to re-lexing as part of hover and gotodef. Re-lexing can now be avoided (and I'm in the process of doing that work), as we now have cached lexer line-end states associated with buffers. (Though, there is still overhead somewhere to generate that cache, just not redundantly/repeatedly if there were no edits).

What you are seeing with reference highlighting is likely the TU needing to complete a preparse, which it completes within the context of the reference highlighting operation, allowing subsequent Hover and GTD operations to complete quickly. If the reference highlighting operation were skipped, for example, a similar impact would just occur on the next IntelliSense operation.

The only lexing work that had been part of reference highlighting would occur only when getting no results from IntelliSense, at which point it tried to match preprocessor directives (#if/#else/#end). That also avoided lexing work unless the currently selected text looks like a preprocessor directive. I'm not sure what version of the Extension your comment is based on, but as of 1.22.8, that operation now uses the cached lexer states. If the cursor is not on a preprocessor directive, it only lexes a single line. (It still needs to lex backwards and forwards, however, if the cursor is on a preprocessor directive, to find matches. But I suspect your testing was not specific to preprocessor matching.)

I'm going to re-update the title here to reflect the original issue, and move it into 1.23, as 're-lexing' can now be completely removed from the codebase. If there is another issue you are seeing that is not 'by design' (TU preparse), could you open a new issue for it?

bobbrow commented 1 month ago

Thanks for the additional information.