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

Improve anti flicker heuristic #49975

Closed
jrieken opened this issue May 16, 2018 · 5 comments
Closed

Improve anti flicker heuristic #49975

jrieken opened this issue May 16, 2018 · 5 comments
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug outline Source outline view issues verified Verification succeeded
Milestone

Comments

@jrieken
Copy link
Member

jrieken commented May 16, 2018

The outline is pretty dumb and simply requests updates when the code changes... That can cause nervous flicker. We should (a) increate the debounce rate and (b) have some kind of heuristics that delays updating the tree when the number of symbols changes drastically from the number of lines.

may-16-2018 14-53-30

@jrieken jrieken self-assigned this May 16, 2018
@jrieken jrieken added bug Issue identified by VS Code Team member as probable bug outline Source outline view issues labels May 16, 2018
@jrieken jrieken added this to the May 2018 milestone May 16, 2018
@Krzysztof-Cieslak
Copy link
Contributor

Krzysztof-Cieslak commented May 16, 2018

Would it be possible to let extensions decide when outline view should be refreshed?

For our code outline view in Ionide we refresh the tree only after we got event that current file got parsed (and typechecked) by F# Language Services. And parsing file request are throttled (i.e we send the parse request after user stopped writing for X ms [depending on file size]). If this was not done in such order (outline view is refreshed before parsing the file) language server would return outdated results for document symbols, and hence the tree shown in code outline view wouldn't mirror current state of editor.

I think any baked in heuristics may have assumptions that may not work fine for different language services, and it would be best to delegate refresh control to language specific extensions.

@jrieken
Copy link
Member Author

jrieken commented May 17, 2018

That's always hard because the model is provider based. Tho, a provider can always delay things (be artificially slow). Today, we debounce a little which gives your extension enough time to enqueue parsing. The request for the outline can then be waiting for that request (e.g. the version property of document might come handy here)

@Krzysztof-Cieslak
Copy link
Contributor

I’m not too familiar with a details as I haven’t looked at this part of code base but there are couple of providers that expose refresh event as part of their API - CodeLens and TreeView from top of my head.

And yes, maybe debuncing will work for some cases, but in principle I believe that language extensions/servers has best, most accurate information to decide when refresh should happen.

@jrieken
Copy link
Member Author

jrieken commented May 17, 2018

CodeLens and TreeView from top of my head.

Right, we have it for the CodeLensProvider but it purpose isn't to signal that specific code lenses have changed but to signal that it when from disabled to enabled. The model is still a pull model (which it is whenever something is call XYZProvider) and the idea is that the UI defines the rate at which it asks instead of the other way around. Also, language providers aren't just used by the UI but also by API commands which makes adding state to them harder.

And yes, maybe debuncing will work for some cases, but in principle I believe that language extensions/servers has best, most accurate information to decide when refresh should happen.

That's what I like to call the push model because extension would push data into the UI (which is actually how diagnostics work). There is not just advantages to that model because extensions can overwhelm the UI with that approach easily and it asks extensions to listen, e.g listen to document changes et al.

most accurate information to decide when refresh should happen

I'd say outline is a simple case because it is driven by the fact that the document changed. We also make sure to send the document change event first and then we ask for things like IntelliSense, code actions, outline etc and that gives extensions a way to pipeline and enqueue requests.

@jrieken
Copy link
Member Author

jrieken commented Jun 5, 2018

Leaving the heuristic like it is for now and awaiting more feedback

@jrieken jrieken closed this as completed Jun 5, 2018
@mjbvz mjbvz added the verified Verification succeeded label Jun 27, 2018
@vscodebot vscodebot bot locked and limited conversation to collaborators Jul 20, 2018
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 outline Source outline view issues verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

3 participants