Skip to content
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

removed on language activation events and switched to a more specific event #271

Merged
merged 2 commits into from
Aug 24, 2022

Conversation

ashishchoudhary001
Copy link
Contributor

@ashishchoudhary001 ashishchoudhary001 commented Aug 18, 2022

Reason for this change is captured in the GitHub Issue: 228

Though at the moment the extension gets activated on a variety of events...but if we remove other activation events the extension will be activated when:
Workspace contains a file/folder ending with .portalconfig at any level of depth.

Tested with this change (and removing other activation events):

image

@ashishchoudhary001 ashishchoudhary001 changed the title removed on language activation events and switched to more a specific event removed on language activation events and switched to a more specific event Aug 18, 2022
package.json Show resolved Hide resolved
@ashishchoudhary001 ashishchoudhary001 merged commit 7ebad5f into main Aug 24, 2022
@ashishchoudhary001 ashishchoudhary001 deleted the users/aschoud/activationEventChange branch August 24, 2022 18:49
@Tyriar
Copy link
Member

Tyriar commented Aug 24, 2022

@davidjenni I just looked at the activation time after installing the extension on my 11th gen alderlake CPU and for 4 reloads it was:

579ms
741ms
607ms
546ms

This is time that the extension host is busy loading your extension, if you were using * this would be quite alarming. Since you're using onStartupFinished this impacts off the top of my head:

  • CPU/battery usage to load all the code that may not ever by used
  • RAM usage, which may make VS Code look bad (it's hard to measure this)
  • Other onStartupFinished extensions may have deferred loading, depending on the order they are loaded
  • This time could block early language server requests, or any extension host request for that matter (this is even more important for remote connections)
  • This applies to all extensions hosts (typically just 1 if you only use a local window)

I wouldn't say this is a major issue, but imo the extension is not being a good citizen by having this catch-all activation event. I personally disable all "heavy" extensions like this to avoid death by 1000 papercuts.

When this topic came up in our standup just now some people called out extension uninstall hooks. You could install to PATH on first activation and then remove it via the uninstall hook.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants