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

Postpone wait for build job #2527

Merged
merged 1 commit into from
Mar 13, 2023
Merged

Postpone wait for build job #2527

merged 1 commit into from
Mar 13, 2023

Conversation

snjeza
Copy link
Contributor

@snjeza snjeza commented Mar 10, 2023

A related issue - #2519

Signed-off-by: Snjezana Peco <snjezana.peco@redhat.com>
@CsCherrYY
Copy link
Contributor

Just tried this patch, the capabilities are registered indeed before building and after the project is imported, however, the language features (e.g., definition, completion) are still unusable when building (we can call these requests, but no response). After the building process is finished, the features are usable.

Or is there anything I'm missing?

@snjeza
Copy link
Contributor Author

snjeza commented Mar 13, 2023

@CsCherrYY You may want to take a look at #2519 (comment)

...
I think it is possible. However, which language feature do you expect will be available without proper project configuration? Completion for example requires at least that the dependencies are available to be helpful.
...

Copy link
Contributor

@rgrunber rgrunber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine with merging, it's just difficult to find cases where the benefit is very clear.

I think any delegate command should benefit by being able to respond sooner since it no longer relies on the autobuild completing. However, there are delegate commands that basically rely on the index, or maybe they rely on certain parts of JDT that ultimately depend on the autobuild. Testing this change, I can definitely see the paste event being properly handled immediately after ServiceReady whereas before it would need to wait for the autobuild to also complete. Unfortunately I also see cases where the autobuild blocks the paste event from continuing to execute.

From my understanding of the protocol, if ServiceReady is sent out sooner, then the client will begin sending other requests (eg. delegate commands), and if they're handled sooner that should be an improvement.

@snjeza snjeza merged commit 1226891 into eclipse-jdtls:master Mar 13, 2023
@testforstephen
Copy link
Contributor

Another root cause about features not responding during autobuild is lock conflicts. According to our analysis, we see that when a feature is not responding, it actually never enters the LSP dispatcher class JDTLanguageServer.java yet. This means that lsp4j's jsonrpc processor is blocked by some handler during autobuild, and subsequent lsp requests are queued at lsp4j jsonrpc layer. This is another key performance issue #2518 we need to improve.

@rgrunber rgrunber added this to the Mid March 2023 milestone Mar 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants