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

Goto Symbol boots slowly on large files, first characters are inserted in the code as a result #2346

Closed
sylbru opened this issue Oct 26, 2023 · 6 comments · Fixed by #2347
Closed

Comments

@sylbru
Copy link

sylbru commented Oct 26, 2023

Describe the bug
LSP Goto Symbol suddenly started being a little slower to "boot", and I end up littering my code with unwanted characters as a result: I hit Ctrl-R to open LSP Goto Symbol, and immediately start typing the name of the function I’m looking for, as I always do, but now it fires a little bit late and the first character of my function name is inserted in the code.

To Reproduce
Steps to reproduce the behavior:

  1. Open a source file that is long enough for the problem to be noticeable and annoying (4-5k lines on my computer)
  2. Hit Ctrl-R or whatever your keybinding is for "LSP Goto Symbol"…
  3. …and start typing right away. The characters you type before the input panel is up are inserted into the code, generating syntax errors just before you jump to another part of the code as Goto Symbol starts accepting your input (that makes it hard to notice you’re adding these characters).

Expected behavior
I’m not sure which is the main issue, but both would be good to fix I guess:

  • have the LSP Goto Symbol command fire near-instantly, even in large files (examples: legacy applications, languages which encourage large files)
  • have the view be blocked while the input panel is loading (is it possible?)

Environment (please complete the following information):

  • OS: Linux Mint 21.2 x86_64
  • Sublime Text version: 4152
  • LSP version: 1.26.0
  • Language servers used: intelephense
@rchl
Copy link
Member

rchl commented Oct 26, 2023

Note that we have logic for preventing keys from modifying the view when document symbols are requested but maybe some recent changes opened some timing window where this doesn't apply. Maybe due to extra input handler

@jwortmann
Copy link
Member

jwortmann commented Oct 26, 2023

I guess it might happen because we still erase that setting directly when the response arrives at

self.view.settings().erase(SUPPRESS_INPUT_SETTING_KEY)
but after that there might still be a small delay before the overlay opens because we run it through window.run_command. Maybe we can move that line somewhere into the input handler class. But I'll have to take a closer look later, not that we possibly miss some code path in that case where the setting accidentally won't get erased.

It might also be possible now to catch the typed input during that time and insert it into the text field when it opens, via initial_text of the newly introduced input handler. But I guess I'll need a sufficiently large file to be able to test this; when I run the command the overlay still opens basically instantly even on files with several thousand lines.

@rchl
Copy link
Member

rchl commented Oct 26, 2023

When testing on a server (tsc) that is still initializing I can easily trigger a case where documentSymbols will take a long time to respond but in that case I can't reproduce the issue (the input is blocked in that case). So I guess it's as you are saying that it's the period between when the response arrives and when the list is shown that is problematic.

@jwortmann
Copy link
Member

jwortmann commented Oct 26, 2023

I was able to reproduce it and I already have a fix for it, but I'll see if I can also capture the typed input while the overlay isn't yet open and insert it in the input field when it opens.

Edit: discarded the capturing idea for now, because I think for that we would need an individual command for each key then.

@sylbru
Copy link
Author

sylbru commented Oct 26, 2023

I didn’t have a chance to answer before, but it’s great that you were able to fix this! Thanks!

@jwortmann
Copy link
Member

jwortmann commented Oct 28, 2023

So the bug that the text accidenatlly gets inserted into the file is fixed now on main, but I'm still investigating why the new Goto Symbol overlay is a little bit slower as before.

I tried it first with the largest Julia file I found (3000+ lines) and with a lot of symbols in it (~ 500k characters in the response payload) and the overlay still opens basically instantly, before I can move my fingers from the key binding back to the normal typing position. But now I tested with a huge JSON file with 1.3M character response payload and there is indeed a noticeable delay before the overlay opens.

The overlay was rewritten using a different Sublime API method (ListInputHandler instead of window.show_quick_panel), to make it possible to filter the symbols by symbol kind if desired. I hope that users find this new functionality useful, personally I like it a lot. But I don't think this alone has an effect on the performance.

However, I've got two ideas how to possibly improve the performance:

  1. When generating the list items for the overlay, we create the items with a ListInputItem.value like
    {"kind": 5, "region": [10, 20], "deprecated": false}
    And I remember from the completions handling that the Sublime API might be a bit slow if huge data from thousands of items is transported over the plugin host, so maybe it can be optimized like
    {"k": 5, "r": [10, 20], "d": false}
    or even
    [5, [10, 20], false]
  2. At the moment the symbol positions (which are used for highlighting and scrolling the view when you scroll through the items in the overlay) are converted from LSP row/column encoding to Sublime region offsets when the list items are generated. This means that there are two API calls needed for each list item. To be fair, the exactly same was done before and there were (potentially) even more API calls because previously we processed both the selectionRange and also the full range of each symbol, so this cannot really be a cause of performance regression. But nevertheless it's probably a bad idea to do this computation beforehand, and instead it could be done lazily in ListInputHandler.preview just when the item gets highlighted. The sorting of the symbols can likewise be done from LSP row/column encoding.

I will test if those points make any difference.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants