Skip to content

Conversation

@jrieken
Copy link
Member

@jrieken jrieken commented Dec 1, 2021

Profiling has shown that reading contribution points updates configurations a lot 1. The event of the configuration registry fire in my dev setup around 50 times. There is only 4 listeners to these events but assuming a listener takes 1ms it still makes 200ms (50*4*1) to fire them.

This PR adds a way to pause/resume eventing of the configuration registry and adopts that in the extension service when reading contribution points.

I measured the gains against todays insiders and things are looking pretty good. I have tested with only builtin extensions and with the 30 extensions that I have installed (using my data as real world representative, it is also pretty close to the real world average duration for this). The measurements are the times between the willHandleExtensionPoints and didHandleExtensionPoints perf-marks (F1 > Startup Performance)

Insiders (0cc0904c56) PR 7907361 sandy081/perf c4bd3f8
builtin extensions 185.6 71 (-114.6ms) 94.5 (-90.2ms)
builtin extensions and 30 more 448.8 148.2 (-300.6ms) 163.8 (-285ms)

--

1 Looks like most events are triggered from custom editors and notebooks. Something to drill into further

…t in extension service

profiling has shown that reading contribution points updates configurations a lot. The event of the configuration registry fire in my dev setup around 50 times. I see 4 listeners and when it only takes 1ms to process the event that's 200ms down the drain
@jrieken jrieken added this to the November 2021 milestone Dec 1, 2021
Copy link
Member

@alexdima alexdima left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm only referring to the change in abstractExtensionService.ts: This is good enough for an emergency fix if we want to ship this in November, but I don't think we should be coupling things so tightly. Maybe extension points could have an onBeforeHandling/onAfterHandling where interested parties could hook into. The configuration extension point could then pause/resume the event delivery of the ConfigurationRegistry.

@sandy081
Copy link
Member

sandy081 commented Dec 1, 2021

I discussed with @jrieken offline and I think pausing/resuming config change events while extension points are being registered is good but can lead to unexpected behavior. Instead I proposed a much low risk and local change in the editor configuration land which is aggressively updating configuration (atleast 20 times) during this time frame. The third column in the above perf comparisons table is with these changes - Here is the PR for that #138302

@jrieken
Copy link
Member Author

jrieken commented Dec 2, 2021

Closing this since #138302 has been merged. There is also talks about a new lifecycle phase that signals extensions are registered. Very similar to the whenInstalledExtensionsRegistered-function

@jrieken jrieken closed this Dec 2, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Jan 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

4 participants