Vscode-cpptools: [Regression] "Go To Definition" performance downgrade in v0.20.0

[yaweiw]

Relevant issue #3480

We've managed to identify a performance regression in v0.20.0. We tested "Go To Definition" performance between v0.19.0 and v0.20.0 using the same repo. The performance downgrade is distinct. As there were a few bug fixes wrt GTD in v0.20.0, it doesn't surprise me if regression bugs were introduced. We've roll back to 19 and everything works just fine.

[yaweiw]

@sean-mcmanus @Lennon925

[sean-mcmanus]

Is the performance regression you are seeing dependent on the size of the file and is the main process using the CPU? We started re-running our lexer on Go to Def in that build. We plan to eventually cache the lexing. If the IntelliSense process is using CPU then that is a different issue.

[yaweiw]

We noticed the slowness when the definition of functions is in another subfolder. It seemed irrelevant to the size of the source code file. The size of the repo however is quite big and browse.db is around 1.6GB. We are not seeing high CPU utilization on the machine at that moment. Not sure how cached lexing has anything to do with it. My gut feeling is we should look into the performance of how symbol is fetched out of the browse db.

[sean-mcmanus]

Yeah, looking at the linked issue, it sounds like a database querying issue. You can verify this if Go to Definition is fast for local variables and slow when definitions are in a different TU, which requires a database look up. Does 0.19.0 actually work correctly? It's possible 0.19.0 was buggy and failing to query the database for the definition. I'm not aware of any change that would have made the database queying itself slower.

We released 3 insiders 0.19.1-insiders/2/3 -- do you know which one introduced the regression? That would help narrow down the change that caused it.

[sean-mcmanus]

I see one fix where we were not querying the database if the definition wasn't found in the TU and #include headers were missing and intelliSenseEngineFallback is Disabled, but in that case it would give "No results" with 0.19.0.

[yaweiw]

locals or symbols within the same TU is fast. Slowness only happens across different TUs.
Test results:
0.19.1 insiders1 is as good as 0.19.0. insiders2 is slower than insiders1 but better than insiders3. insiders3 is as slow as 0.20.0. We also noticed massive Current Disk Read Queue Length from PerfMon, which indicates burst of Disk Reads that initiated by cross TU GTD.

[sean-mcmanus]

Yeah, the change made in 0.19.1-insiders3 could cause up to 3 times as many database operations as it retries with less strict type checking. It might be possible to avoid the 2 extra iterations if we build 3 results simultaneously in 1 pass (not sure yet). That could reduce the time by 1/3 or so.

[yaweiw]

cool looking forward to that. thanks mate @sean-mcmanus

[Lennon925]

@yaweiw @sean-mcmanus ,
In addition, I found it's very slow if you use "Ctrl" + mouse left button to go to definition.

[yaweiw]

@Lennon925 Ctrl + click works the same as F12. What's happening under the hood is cross TU query against the browse.db file that was generated by Tag Parser. Correct me if wrong @sean-mcmanus

[Lennon925]

@Lennon925 Ctrl + click works the same as F12. What's happening under the hood is cross TU query against the browse.db file that was generated by Tag Parser. Correct me if wrong @sean-mcmanus

There is a little difference between them. When you use ctrl+click, you can go to definition until underline appears. Sometimes underline can't appear for long time.

[sean-mcmanus]

Ctrl+Mouse click can potentially be slower, because it could cause a hover to be queued beforehand. We had an unrelated hover perf issue that could cause 2X database accesses that I fixed while investigating https://github.com/microsoft/vscode-cpptools/issues/3596 .

We plan to release an insiders with these Go to Def (hover) perf fixes as soon as we have the semantic colorization finished.

Yeah, if no definition/declaration is found in the current TU by IntelliSense it will need to query the browse.db.

[sean-mcmanus]

The slowness that got fixed should have only been happening for functions/methods. If you're seeing slowness in another scenario that would be a different issue.

[Lennon925]

Ctrl+Mouse click can potentially be slower, because it could cause a hover to be queued beforehand. We had an unrelated hover perf issue that could cause 2X database accesses that I fixed while investigating #3596 .

We plan to release an insiders with these Go to Def (hover) perf fixes as soon as we have the semantic colorization finished.

Yeah, if no definition/declaration is found in the current TU by IntelliSense it will need to query the browse.db.

Thanks for your reply and I am very looking forward to the semantic colorization.

[yaweiw]

@sean-mcmanus any updates?

[sean-mcmanus]

@yaweiw We're still finishing up the colorization work...we haven't decided on a release date for the 0.24.0-insiders build yet. I was hoping at least by some day early next week.

UPDATE: Maybe around June 11th?

[sean-mcmanus]

Fixed with 0.24.0-insiders (available by setting "C_Cpp.updateChannel" to "Insiders").

[yaweiw]

@sean-mcmanus we tried and noticed the performance is better than 0.19.0 but still worse than 0.17.7 which we are using atm. What's the difference?

[sean-mcmanus]

0.19.0 and after does an additional lexing pass for the source file (always happens, even in Tag Parser mode), which slow down Go to Def if your source file is large (we plan to improve the performance on this later). It also will run IntelliSense on the file and potentially O(N) additional work (only in Default intelliSenseEngine mode), so could try setting the intelliSenseEngine to TagParser to see if Go to Def is faster. If you could run some profiler and or break into the debugger while it's slower to get a call stack that might help provide more info.

Or, do the following:

1. Go to Def.
2. Look at the main process CPU usage before the IntelliSense process starts using CPU. If that is the slow section, it's probably lexing that is the bottleneck.
3. If you see the IntelliSense process taking too long, that would be bottleneck.
4. If you see the IntelliSense process finish, and then the main process taking too long, then it would be the additional processing we do afterwards. We do additional type checking between the database and IntelliSense during this time. The fix I made reduced the database iterations required.

If you have some way to reduce the size of your database and see if that "fixes" it that would indicate that the database size/querying is still the bottleneck. Or check to see if there's a large database size difference between 0.17.7 and later versions, i.e. we might have had a bug that caused the database to be too small.

[yaweiw]

@sean-mcmanus ok. we will test and get back. thanks.

[yaweiw]

@sean-mcmanus change to "Tag Parser" fixes the code navigation performance issue but intellicode seems missing/unsupported with "Tag Parser"?

[sean-mcmanus]

Yeah, "Tag Parser" is not recommended to use -- it just does an incomplete parse to attempt to get global symbols.

Why are you using the Tag Parser mode? I was just suggesting that to compare the performance. Performance with the IntelliSense is expected to be slower though.

[yaweiw]

If IntelliSense is expected to be slower how come GTD in 0.17.7 is faster than 0.24.0?

[sean-mcmanus]

0.17.7 didn't have IntelliSense based Go to Def...the Go to Def just used the global symbols from the Tag Parser.

We could potentially add a "gotoDefMode" setting that can disable the IntelliSense-based stuff, but we'd need to get more info on what the problem is with the IntelliSense-based go to def, i.e. if we need to disable the type checking with definitions across translation units or if there is a problem with go to def for local variables (i.e. the creation of the IntelliSense client itself).

We also added stuff like hover doc comments which could be causing additional slowdown (i.e. don't hover to see if that helps).

[yaweiw]

Not sure if it's reported, apart from the performance issue, GTM in 0.24.0 sometimes jumps to the wrong location e.g. couple of lines below the right location. IntelliSense/Code completion also seems problematic e.g. gives the totally irrelevant recommendations.

[sean-mcmanus]

If the file hasn't been parsed by IntelliSense at least once, it will fall back to the irrelevant tag parser results for completion and hover. We have reproed some bugs where go to definition or hover results are off by a few lines after edits.