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

VS Code event emitter performance improvements #185789

Closed
connor4312 opened this issue Jun 21, 2023 · 1 comment · Fixed by #185889
Closed

VS Code event emitter performance improvements #185789

connor4312 opened this issue Jun 21, 2023 · 1 comment · Fixed by #185889
Assignees
Labels
insiders-released Patch has been released in VS Code Insiders perf
Milestone

Comments

@connor4312
Copy link
Member

In working on the remote tunnels SDK, now being used in part for the 'exec server', Martin suggested we publish an official event npm module. I already had a small VS Code compatible module for use in my own code. I updated it to support the full set of EmitterOptions that we do in core, and out of curiosity compared it against the implementation in core. I suspect our usage of linked lists is responsible for the bulk of the performance delta: it's very nice a computer sciencey, but I think that we get dramatically punished by cache locality in practice. There is of course memory overhead for nodes as well.

image

I then gathered some initial data
on how we use events in vscode.

I initially hypothesized we'd have a bimodal distribution between listeners with a small number of events, and ones with large number of events. In reality, it's much closer to a power-law distribution. The plurality of event emitters have no listeners at all, and second most common is a single listener. This data was collected after VS Code started up with an editor open:

image

We can also estimate how long was spent in the delivery mechanics during boot. This is the time to create the delivery queue, not call each individual function, so it slightly underestimates the actual time.

Stats for the first 13 seconds of the editor:
Time spent in emitter overhead (ms) 33.0
Total number of event emissions: 13882

From a heap snapshot, I manually tallied memory usage per for emitters, which comes to a total usage of about 335KB. Again, this is an underestimation.

I believe the biggest benefit is an expected ~15ms improvement in startup time.

@connor4312
Copy link
Member Author

My reworkings saved time, slightly less than expected, which I attribute to having to implement some more subtleties of vscode's emitter that my initial implementation lacked and weren't covered by tests. But, regardless, it seems to save roughly 10ms and is almost certainly faster than the old implementation when looking at startup performance.

image

You can check out the full analysis and data in the notebook https://github.com/connor4312/evt/blob/compat/event-analysis/nb.ipynb

@vscodenpa vscodenpa added unreleased Patch has not yet been released in VS Code Insiders insiders-released Patch has been released in VS Code Insiders and removed unreleased Patch has not yet been released in VS Code Insiders labels Jun 22, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Aug 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
insiders-released Patch has been released in VS Code Insiders perf
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants