Skip checking install requests for known extensions #2288
Conversation
not sure what will go in the release so some change log information is out of date
We check the package.json file for each extension which is an expensive wholistic file read that happens async during startup. This is slow and we can skip that for many extensions that we know will never need to ask us to install or find dotnet. What I did to generate this list: - Look at the extensions.all output on an empty (no custom extensions) code - Look at the marketplace and add all first party / business partner related extensions I saw on the page that had the highest number of downloads This is not an endorsement of these extensions or their codebases. There is not a way to filter these or get anything besides .all of the extensions at this time.
|
Can we use an O(1) structure? Is that worth the memory cost and upkeep of a const size, small size container? Probably not |
|
Set is indeed implemented typically using an o(1) data structure such as a hashtable in JS based on my research. So, it may be slightly memory intensive, but considering we do the operation a great number of times, it's likely worth this cost. Can run through with the perf review team. |
| @@ -13,6 +13,131 @@ import { IJsonInstaller } from "./IJsonInstaller"; | |||
|
|
|||
| export class JsonInstaller extends IJsonInstaller | |||
| { | |||
| private readonly knownCommonExtensionIdsNotUsingDotnet: Set<string> = new Set<string>([ | |||
There was a problem hiding this comment.
Would it be better to do this the other way around?
That may need to have a way to specify additional extensions locally.
There was a problem hiding this comment.
Hm, do you mean inclusive instead of exclusive? I think that's a good idea, but I worry it would harm the adoption/usefulness of our API by only allowlisting certain extensions to use it. Worst case, I could see that coming up in an anti-competitive practice lawsuit, which concerns me.
Now, maybe we could add some other way for extensions to specify they want us to do something. However, this needs to be done at activation before other extensions can run. We could ask them to ship with an additional manifest file, perhaps... though I'm not sure if we would be privy to that logic, and that risks things like protected files / bad read access, so I think I prefer this approach. Thanks for taking a look 👀
Co-authored-by: Michael Yanni <[email protected]>
We check the package.json file for each extension which is an expensive wholistic file read that happens async during startup. This is slow and we can skip that for many extensions that we know will never need to ask us to install or find dotnet.
What I did to generate this list:
This is not an endorsement of these extensions or their codebases.
There is not a way to filter these or get anything besides .all of the extensions at this time.
cc @adrianvmsft @JakeRadMSFT @lifengl