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

Typing is laggy when outline is visible #96914

Closed
joaomoreno opened this issue May 4, 2020 · 10 comments
Closed

Typing is laggy when outline is visible #96914

joaomoreno opened this issue May 4, 2020 · 10 comments
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug freeze-slow-crash-leak VS Code crashing, performance, freeze and memory leak issues perf verified Verification succeeded windows VS Code on Windows issues
Milestone

Comments

@joaomoreno
Copy link
Member

joaomoreno commented May 4, 2020

Steps:

  1. Open vscode-docs
  2. Open release-notes/v1_45.md
  3. Make sure outline is visible in the sidebar
  4. Type some text

Here's the profile screenshot with outline open:

image

With outline closed:

image

cc @jrieken

@joaomoreno joaomoreno added bug Issue identified by VS Code Team member as probable bug perf labels May 4, 2020
@joaomoreno joaomoreno added this to the May 2020 milestone May 4, 2020
@joaomoreno joaomoreno self-assigned this May 4, 2020
@joaomoreno
Copy link
Member Author

joaomoreno commented May 4, 2020

Figured out that this is called on every keystroke:

This is odd, especial because getLiveMarkers(textModel) always returns an empty collection:

for (const [range, marker] of this._markerDecorationService.getLiveMarkers(textModel)) {

So why is onDidChangeMarker actually firing?

this._editorDisposables.add(this._markerDecorationService.onDidChangeMarker(updateMarker));

@jrieken
Copy link
Member

jrieken commented May 4, 2020

Adding @sandy081 who wrote that marker service and because this likely affects the markers panel?

@joaomoreno joaomoreno removed their assignment May 4, 2020
@joaomoreno joaomoreno added the freeze-slow-crash-leak VS Code crashing, performance, freeze and memory leak issues label May 5, 2020
@jrieken jrieken added the info-needed Issue requires more information from poster label May 5, 2020
@jrieken
Copy link
Member

jrieken commented May 5, 2020

This doesn't reproduce for me, e.g no slow typing or freezing nor does the FPS change when toggling outline.problems.enabled. The CPU profiles shows that very little time is spend in updateMarker (when typing four times console.log();\n in TS files that already has some errors).

image

I am not arguing that it is smart to always call this function but I definitetly see no slowing, freezing and anything remotely related to that.

@jrieken jrieken added windows VS Code on Windows issues and removed info-needed Issue requires more information from poster labels May 5, 2020
@jrieken
Copy link
Member

jrieken commented May 5, 2020

Freeze only repos on windows (linux unknown, mac not)

@jrieken
Copy link
Member

jrieken commented May 5, 2020

The event firing soo often is actually the right thing because it means (bad naming) ranges of diagnostics markers of this file have changed. And that happens when typing... I will debounce this in the outline view

@jrieken
Copy link
Member

jrieken commented May 5, 2020

Pushed change to debounce the onDidChangeMarker inside the outline pane (see comment above) and I also don't send the event when before and after no marker where there

@sandy081 leaving the rest to you. Idk how else this service is used and if its internal content change listener should be debounced or not. I feel it should but idk how that affects other consumers. This call/event:

markerDecorations.register(model.onDidChangeContent(() => this._updateDecorations(markerDecorations)));

@joaomoreno
Copy link
Member Author

Thanks @jrieken, debouncing is definitely a good change.

But yeah, there's still the question of why is that even firing at all, when there aren't any changes to the markers.

@sandy081 sandy081 assigned jrieken and unassigned sandy081 May 5, 2020
@sandy081
Copy link
Member

sandy081 commented May 5, 2020

I introduced IMarkerDecorationsServiceinitially for Monaco editor to show squiggles and hovers.

@jrieken Seems you added onDidChangeMarker and getLiveMarkers afterwards and OutlinePane is the only consumer.

Please change it according to the needs of OutlinePane.

@jrieken
Copy link
Member

jrieken commented May 5, 2020

@sandy081 please read carefully. This is the underlying problem that's remaining: 404f66c

@jrieken jrieken assigned sandy081 and unassigned jrieken May 5, 2020
@sandy081
Copy link
Member

sandy081 commented May 5, 2020

Sorry @jrieken. I just focused on last comment - #96914 (comment)

Also debounced the internal content change event.

@joaomoreno joaomoreno added the verified Verification succeeded label Jun 4, 2020
@github-actions github-actions bot locked and limited conversation to collaborators Jun 19, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug freeze-slow-crash-leak VS Code crashing, performance, freeze and memory leak issues perf verified Verification succeeded windows VS Code on Windows issues
Projects
None yet
Development

No branches or pull requests

3 participants