Performance regression due to exe-based extension recommendations

[jrieken]

We have seen a slowdown in startup performance of 1.16 stable/insiders and could trace this to the fact that product.json now contains exe-based extension recommendation that are probed using cp.exec.

Do the following:

• checkout origin/release/1.16 and build vscode yourself
• due a few runs and write down perf numbers (F1 > Startup Performance)
• now download 1.16 stable from our website and use its product.json-file for the self-build verson
• measure again and see a severe performance hit between 500-1000ms
• remove the exeBasedExtensionTips and performance is back to normal

The following things should be changed

• Don't compute extension recommendation on startup as this isn't needed for a blinking cursor. Re-schedule yourself to run a few seconds after your service is getting instantiated
• Don't use cp.exec but read the path-variable to check for the existence of a executable. @dbaeumer is the expert and has some code that does exactly that

My hand-written notes of the performance ticks: App Ready, start window, window ready, code loaded, .... You see that the middle column (using the 1.16 product.json) delays things

[jrieken]

We have worked-around this by clearing the list of exe's from product.json

[jrieken]

@ramya-rao-a Please create an followup item that fixes the actually issue here

[ramya-rao-a]

@jrieken Instead of removing the recommendations altogether for 1.16, I can move the checks inside a setImmediate just like it is done for the fileBased recommendations here: https://github.com/Microsoft/vscode/blob/release/1.16/src/vs/workbench/parts/extensions/electron-browser/extensionTipsService.ts#L184

And then look into your suggestion of not using cp.exec for 1.17.
Thoughts?

[jrieken]

Unsure, we took the safe bet. Even a setImmediate will make us block the event loop because calling cp.exec 30 times easily takes 300ms. We should de-dupe the executables we are looking for first and then use a single setImmediate for each of them. Or have a better solution that doesn't rely on which/where but on us checking the PATH manually. My suggestion is that we keep 1.16 with this disabled and try stuff out for Insiders.

[ramya-rao-a]

Made the below changes for 1.17

• Check PATH manually instead of using cp.exec for which/where
• Find the recommendations based on executables only when recommendations are asked for in the Extensions viewlet. Checking PATH manually is fast enough (about 50 ms) that this will not block anything

[bpasero]

@ramya-rao-a please avoid sync fs APIs in the workbench, I filed #34423 which hopefully is easy to solve for you by switching to async calls.