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

Sublime Text Becomes unresponsive when hovered on variable containing large data (Variable Storing embeded font data) #2398

Open
akash1474 opened this issue Jan 22, 2024 · 16 comments
Labels
enhancement language server issue Issues related to language servers communicating with this plugin

Comments

@akash1474
Copy link

Sublime Text Freezes when hovering over a variable holding large data

I am trying to load a font from an file which contains embedded font data as represented below. When ever I am using the variable FontAwesomeRegular and hovering over the variable the sublime text becomes unresponsive.

For Example

inline unsigned char FontAwesomeRegular[190016] = {
	0x4F, 0x54, 0x54, 0x4F, 0x00, 0x0A, 0x00, 0x80, 0x00, 0x03, 0x00, 0x20,
	0x43, 0x46, 0x46, 0x20, 0x1D, 0xD7, 0xBE, 0xB6, 0x00, 0x00, 0x7D, 0xAC,
	0x00, 0x02, 0x48, 0x50, 0x47, 0x53, 0x55, 0x42, 0xBE, 0xB5, 0xF3, 0x69,
	0x00, 0x02, 0xC9, 0x58, 0x00, 0x00, 0x1C, 0xE6, 0x4F, 0x53, 0x2F, 0x32,
	0x5F, 0x56, 0xDE, 0x6E, 0x00, 0x00, 0x01, 0x10, 0x00, 0x00, 0x00, ...   ]

Some Other File

const int fa_regular_size = IM_ARRAYSIZE(FontAwesomeRegular);
io.Fonts->AddFontFromMemoryTTF((void*)FontAwesomeRegular, fa_regular_size , font_size, &font_config);

Here When I am using FontAwesomeRegular variable in a different file and if I accidently hover over the variable FontAwesomeRegular the sublime text freezes and becomes unresponsive for 50-80 seconds.

The sublime text is trying to load all the details related to the variable in order to display it in variable detail popup and since the data is about almost a size of 1 MB. It is struggling and the sublime text becomes unresponsive.

If there are some fixes please do suggest

OS: Windows 11
Sublime Build: 4169

@LDAP
Copy link
Contributor

LDAP commented Jan 23, 2024

This is an issue with LSP and not specific to LSP-clangd.
@rchl can we transfer the issue to LSP?

@rchl
Copy link
Member

rchl commented Jan 23, 2024

Hover response for reference:

{
  "contents": {
    "kind": "markdown",
    "value": "### variable `FontAwesomeRegular`  \n\n---\nType: `unsigned char[190016]`  \n\n---\n```cpp\nunsigned char FontAwesomeRegular[190016] = {\n    0x4F, 0x54, 0x54, 0x4F, 0x00, 0x0A, 0x00, 0x80, 0x00, 0x03, 0x00, 0x20,\n    0x43, 0x46, 0x46, 0x20, 0x1D, 0xD7, 0xBE, 0xB6, 0x00, 0x00, 0x7D, 0xAC,\n    0x00, 0x02, 0x48, 0x50, 0x47, 0x53, 0x55, 0x42, 0xBE, 0xB5, 0xF3, 0x69,\n    0x00, 0x02, 0xC9, 0x58, 0x00, 0x00, 0x1C, 0xE6, 0x4F, 0x53, 0x2F, 0x32,\n    0x5F, 0x56, 0xDE, 0x6E, 0x00, 0x00, 0x01, 0x10, 0x00, 0x00, 0x00}\n```"
  },
  "range": {
    "end": {
      "character": 39,
      "line": 0
    },
    "start": {
      "character": 21,
      "line": 0
    }
  }
}

The blame could be put anywhere from clangd, ST, mdpopups to LSP :) Hard to say who should handle this better. It wouldn't fix the root issue but I feel like clangd should avoid showing such big outputs (maybe trim or elide in such cases).

@rchl rchl transferred this issue from sublimelsp/LSP-clangd Jan 23, 2024
@jwortmann
Copy link
Member

Is that response output shortened or is it the full response? It looks quite trivial to me, while the opening post says it would be 1 MB data. Or could it be "catastrophic regex backtracking" here?

I've just tested a popup with 1MB content from the ST console and it shows up immediately. So I'm sure ST is not to blame here. I guess the majority of the time is spent in mdpopups if it wants to apply syntax highlighting to a huge amount of text.

@akash1474
Copy link
Author

Is that response output shortened or is it the full response? It looks quite trivial to me, while the opening post says it would be 1 MB data. Or could it be "catastrophic regex backtracking" here?

I've just tested a popup with 1MB content from the ST console and it shows up immediately. So I'm sure ST is not to blame here. I guess the majority of the time is spent in mdpopups if it wants to apply syntax highlighting to a huge amount of text.

It is a full response with a scrollbar attached to it that displays the entire data related to that variable FontAwesomeRegular.

Here's a screenshot
Screenshot 2024-01-23 163033

@jwortmann
Copy link
Member

I was referring to the JSON payload in #2398 (comment)

The markdown content is only 522 bytes, so I guess it must be shortened.

Since we don't parse the content directly in LSP, I don't think it would be feasible to define some cutoff range after which content would be discarded (that could for example "destroy" the HTML/Markdown structure). But maybe there is a way to disable the syntax highlighting, if the content is to large and if it turns out that the syntax highlighting is the culprit.

I haven't tested and don't know if it works, but a starting point could be to override the syntax used for cpp code blocks and check if that reduces the rendering time.
mdpopups has some settings which could be relevant, for example "mdpopups.use_sublime_highlighter": true and something like (untested)

    "mdpopups.sublime_user_lang_map": {
        "language": [["cpp"], ["Text/Plain text"]]
    }

https://facelessuser.github.io/sublime-markdown-popups/settings/#mdpopupssublime_user_lang_map

@rchl
Copy link
Member

rchl commented Jan 23, 2024

Is that response output shortened or is it the full response?

Yeah, I've shortened it in my payload (or... I didn't have the full payload in the first place)

@rchl
Copy link
Member

rchl commented Jan 23, 2024

Could also be an issue with ST being slow with handling wrapping in the popup.

Should be easy to make the payload much bigger for testing by duplicating byte lines in the initial example until it gets 1MB big.

@rwols
Copy link
Member

rwols commented Jan 23, 2024

I used this repository for testing: https://github.com/akash1474/txedit

In the file src/fa-regular.h, I cannot reproduce clangd's behavior: https://github.com/sublimelsp/LSP/assets/2431823/b72d9849-b788-4b8e-a929-062187eb9f65

Payload:

{'range': {'start': {'character': 21, 'line': 4}, 'end': {'character': 38, 'line': 4}}, 'contents': {'value': '### variable `data_icon_regular`  \n\n---\nType: `unsigned char[190016]`  \nValue = `{79, 84, 84, 79, 0, 10, 0, 128, 0, 3, ...}`  \n\n---\n```objective-cpp\nunsigned char data_icon_regular[190016]\n```', 'kind': 'markdown'}}

I'm using LSP-clangd and using my system's clangd executable. Its version is:

/m/W/c/txedit (main|…) $ clangd --version
Ubuntu clangd version 18.0.0 (++20240121042215+a70d3101ba78-1~exp1~20240121042337.1445)
Features: linux+grpc
Platform: x86_64-pc-linux-gnu

I think this should ultimately be handled by the language server to trim large hover contents. What is your clangd version? It may be that this problem was noticed and consequently fixed.

@rchl
Copy link
Member

rchl commented Jan 23, 2024

This is what I see with the simple (trimmed) example:

Screenshot 2024-01-23 at 22 49 46

EDIT: And I get the same with Homebrew clangd version 17.0.6

@akash1474
Copy link
Author

@rwols @rchl This behavior is reproduced when you include the embedded font file( fa-regular.h ) in some other cpp file and then use the variable data_icon_regular same as FontAwesomeRegular(as discussed in earlier opening comments)

otherfile.cpp

#include "fa-regular.h"

int main(){
   const int size=sizeof(data_icon_regular)/sizeof(data_icon_regular[0]);
   return 0;
}

and When we hovering over data_icon_regular variable for details ,the sublime text becomes unresponsive.

Here's the link font header file link containing the font header data.
fa-regular.h

clangd version: 17.0.6

clangd Version

@jwortmann
Copy link
Member

I can reproduce this issue with the provided example #2398 (comment)

This is with the clangd version 17.0.3 that gets automatically installed by LSP-clangd when clicking "Yes" in the dialog promt after installing LSP-clangd.

ST freezes for 30 seconds here, but after disabling the syntax highlighting in popups per

    "mdpopups.use_sublime_highlighter": true,
    "mdpopups.sublime_user_lang_map": {
        "language": [["cpp"], ["Text/Plain text"]]
    },

in Preferences.sublime-settings this time can be reduced to ~ 8 seconds (which is still terrible). Server response time is around 2.5 seconds for this request. So the syntax highlighting seems to be a big part of the problem, but apparently mdpopups spents some more time in a blocking manner while parsing the content.

While it might be resolved in a later clangd version, I guess it would be better to have some kind of safety check in LSP to prevent passing overly huge Markdown content to mdpopups.
Or perhaps at least the freezing (but not the lag of the popup appearing) would already be fixed if this parsing would happen on the async thread? Note that currently it seems to happen on the UI thread:

sublime.set_timeout(lambda: self._show_hover(listener, point, only_diagnostics))

This function calls self.hover_content() -> minihtml() -> mdpopups.md2html(), apparently blocking the UI thread.

@rwols
Copy link
Member

rwols commented Jan 24, 2024

Maybe @facelessuser wants to be aware of this performance edge case as well. Is there anything mdpopups could do to speed up rendering?

@facelessuser
Copy link

Maybe @facelessuser wants to be aware of this performance edge case as well. Is there anything mdpopups could do to speed up rendering?

What would you suggest? I feel like this may be more on the side of LSP or LSP plugins. LSP controls how much Markdown is being sent to mdpopups. It can't be expected for massive blocks to be rendered quickly in a popup.

I'm not sure it should be mdpopups job to arbitrarily clip content, I think that should be decided by those using it.

If it isn't already allowed, we could add a way to disable highlighting per popup. I'd have to look into that. You are free to create an issue if this is a desirable feature.

@rwols
Copy link
Member

rwols commented Jan 25, 2024

I'm not sure it should be mdpopups job to arbitrarily clip content, I think that should be decided by those using it.

I was not suggesting for mdpopups to clip content but rather speed up processing.

It can't be expected for massive blocks to be rendered quickly in a popup.

That's fair.

@facelessuser
Copy link

I was not suggesting for mdpopups to clip content but rather speed up processing.

There are probably some places where I can "speed up processing", but I rely on other modules for dependencies and I'm not specially rewriting them. Could I possibly make ST syntax highlighting in code blocks faster, maybe.

The problem is that even when highlighting is disabled, it still takes 8 seconds, which is not great. Your real problem is that you have an unbounded content problem.

If you want to have instant/semi-instant popups, you have to take steps to make sure you manage the size of the content. You cannot expect that LSP can handle unbounded, infinitely expanding content in popups and still provide near-instant popups.

Let's say I cut highlighting time in half. That may drop your wait time from 80 seconds to maybe 48 seconds. Your problem of unbounded content still exists. You need to solve that problem as that's the root. Yes, would it be nice if mdpopups was faster? Sure. Can I look into improving highlight speed? Create an issue, and when I have time, we'll see. But that is a tangential request to this problem and one that will not solve the underlying issue.

@rwols
Copy link
Member

rwols commented Jan 28, 2024

I made a PR to fix it in the language server: llvm/llvm-project#79746

@jwortmann jwortmann added enhancement language server issue Issues related to language servers communicating with this plugin labels Oct 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement language server issue Issues related to language servers communicating with this plugin
Projects
None yet
Development

No branches or pull requests

6 participants