Support limiting the number of outline symbols to avoid UI freezes #704
Conversation
There was a problem hiding this comment.
Let's just wait for eclipse-platform/eclipse.platform.ui#810
|
@mickaelistria another issue I realized is that the Language Server also takes a considerable amount of time to provide the list of outline symbols for large files. this does not block the UI but delays the moment when the outline viewer starts drawing entries for quite a while. Is there a way to tell the LS to limit the number of elements returned? I can see that DocumentSymbolParams extends WorkDoneProgressAndPartialResultParams but it is not clear if the decision to return partial results can only be made by the server or also requested by the client. |
|
I don't see anything that fits immediately in https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/ . I suggest you lookup in the issues reported against LSP, IIRC there was some discussion about "paginated" responses which could fit here. |
|
In LemMinx we have exactly this kind of problem (when we integrate LemMinx in vscode). It is the reason why we provide a custom limit for symbol and we throw a notfication to LSP client to notify that limit is reached to show this information in a popup information.
Indeed but folding specifies rangeLimit, see So it can be a good idea to have too a rangeLimit kind for symbols. Please note that symbol is used in breadcrumb in vscode, so I think symbols request should perhaps send too an information about the cursor location(just to give symbols hierachy for the cursor location). |
|
Overall, LSP and more particularly JSON-RPC may be missing some capacity for sending responses in (ordered) chunks so that clients such as LSP4J could get the output as a stream or returning potentially long sequences in a BlockingQueue... But this problem is deeply rooted in JSon-RPC/LSP and should rather be addressed there. |
akurtakov
force-pushed
the
limit-outline-elements
branch
from
c691bb6
to
35d23bf
Compare
|
Is this still needed after platform.ui change? |
sebthom
force-pushed
the
limit-outline-elements
branch
from
35d23bf
to
c21c835
Compare
|
A code change is still necessary because the platform.ui change is not enabled by default for all tables/tree viewers. However this change does only address one part of the UI freeze. With newer lsp4j releases the following issue popped up eclipse-lsp4j/lsp4j#815 |
sebthom
changed the title
sebthom
changed the title
sebthom
added
enhancement
|
I ran into performance issues as well. I think this PR is a good starting point. Can this be merged @mickaelistria ? |
|
Hi @ghentschke, but would the problem not also greatly improved (also increasing performance in all cases) if we would fix #907 without using experimental APIs? PD: If you agree to merge this PR, that is fine for me as well, regardless of what is the answer to my question. |
While the javadoc says it is experimental, it is actively used in eclipse.platform.ui and eclipse.jdt.ui so I doubt it is so experimental. |
|
IMO a better solution would be lazy loading. Fetch the data in the background and update the treeview when the user scrolls. But this PR is a good starting to point to keep eclipse responsive. |
I also suggested that in the platform issue but they somehow found the current solution better. |
|
The next step would be to refactor the |
This PR addresses #703 by limiting the maximum number of elements rendered in the UI to 5000 elements.
On my workstation Eclipse can add/render around 1.000 elements per second to the outline. Since adding elements is done in the UI thread, more elements mean longer freeze on initial population and each update.