microsoft / vscode

Visual Studio Code
https://code.visualstudio.com
MIT License
164.25k stars 29.3k forks source link

`DebugService` has high cost #159886

Open jrieken opened 2 years ago

jrieken commented 2 years ago

That itself is cheap but it "force creates" the delay/lazy debug service which causes a ripple of events and other things. It would be better if we can keep this lazy and depend on the service to soon

Screenshot 2022-09-02 at 12 55 10
jrieken commented 2 years ago

This might a bit of a cat and mouse game because once this contribution is more lazy, the next is on the hook (BreakpointEditorContribution). Still, I think it's worth it. After editor creation and while loading the contents for the editor there could be enough idle time to create the debug service.

Alternatively, creation of the debug service could be made more light weight...

bpasero commented 2 years ago

👍 also saw this in the profiles

roblourens commented 2 years ago

Sorry, what do I need to do to make this editor contribution lazy?

Both of these contribs' constructors only depend on the debugService to add some event listeners, and there's no reason to force the service to be created just for that, but I don't know how to make an editor contribution lazy.

roblourens commented 2 years ago

Could wrap its registration in a workbench contribution that is created delayed?

jrieken commented 2 years ago

Editor contributions cannot be made lazy, they are picked up on start. However, you can make it more lightweight. Today the contribution listens to debug events straight away (and that subscribing instantiates the service) but at that point there is no point in listening because the editor has no model yet. So, this case will always happen.

Listening only when having a model might give you enough slack/idle time in which the debug service can be constructed but I have doubts.

I think the underlying issue is the size and breath of the debug service. The service has many dependencies and does many things when it's being created and I wonder if all that must be in a single service or if the service can be more lazy.

Looking at what's happening:

Screenshot 2022-10-06 at 11 53 32

I believe each of these steps should critically looked at and we should also examine down-level effects, e.g why is does registering a file cause such an expensive label-re-render everywhere etc... I'll rename the title of this issue accordingly

roblourens commented 2 years ago

Made these changes

So at least the DebugService is not created until the editor has a model, but some of the work is still earlier than needed.

It would be cool if simply listening to an event would not instantiate the service, but would return a proxy event emitter that would be hooked up to the real thing when the service gets instantiated for some other reason. That would require the InstantiationService proxy to know that something is asking for an event, maybe by looking at the onDid... name pattern.

The DebugService creation looks like this now

image

More notes

jrieken commented 2 years ago

It would be cool if simply listening to an event would not instantiate the service, but would return a proxy event emitter that would be hooked up to the real thing when the service gets instantiated for some other reason

Love it. @alexdima and I had discussed the same thing yesterday. I'll give it a try and see how much we can save like that

jrieken commented 2 years ago

but would return a proxy event emitter that would be hooked up to the real thing when the service gets instantiated for some other reason

➡️ https://github.com/microsoft/vscode/pull/162928

jrieken commented 2 years ago

Maybe the ConfigurationManager can lazy load its configs. They should only be needed when the debug view is opened, or when the statusbar item appears, which is a lazy workbench contrib

FYI - we have IdleValue and Lazy to make such tricks easy

The context key initialization seems slow because it causes views to be enabled, which causes actions to be registered, but I don't think there's a way to avoid all that.

Dragging in @sandy081 - it might actually be better to register the actions as soon as the views are registered (not when they are enabled) and to enable the actions with the same context keys

The breadcrumbs/filesystem provider should be easy at least, I leave that to you

Will do - it is bizarre. A single (1) file label is being rendered and it seems that inferring class names for the theme/file icon is very expensive...

roblourens commented 2 years ago

Sweet, that was fast. But I have to do more refactoring to take advantage of it, I have all of this this.debugService.getModel().onDid... and this.debugService.getViewModel().onDid... which has always bugged me. Those can probably be separate services too.

             ->    DebugModelService
DebugService ->    DebugViewService
             ->    DebugConfigurationService
sandy081 commented 2 years ago

May I know if you meant about following actions?

https://github.com/microsoft/vscode/blob/9cc932ee6bb1b6b9a8ed0cc4500fb33dae1d1db6/src/vs/workbench/services/views/browser/viewDescriptorService.ts#L740

roblourens commented 2 years ago

I think so