Skip to content

Reduce the impact of build job on language server responsiveness #2519

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

Open
testforstephen opened this issue Mar 9, 2023 · 8 comments
Open

Comments

@testforstephen
Copy link
Contributor

Goals:

  • Discuss how to reduce feature wait time and improve language server responsiveness.

Non-Goals:

  • Investigate the slowness of autobuild job, and improve its performance.

Situation:

In current jdtls implementation, the sevice ready time for language server includes project import (e.g. downloading dependencies and generating Java projects) and build time. Before service ready, all language features are not usable. This is a big pain when working on large projects, because the projects will take a long time to import and build, and you cannot use any features during that time.

Thoughts:

For the project import optimization, we will work on adopting the maven parallel downloading capability. See more info on eclipse-m2e/m2e-core#1169 (comment).

This issue will focus on how to reduce the impact of build job on the language server performance.

  • Idea 1:
    We still keep autobuild on by default, but just leave it running in the background without blocking other language features such as completion. Not sure if this is possible, does anyone have more input on this?

    Back to jdtls code, here is where the initialzation is blocked by build job. Why do we need to wait for build jobs during workspace initialization job? Can we remove it but register language server features and make service ready earlier?
    https://github.com/eclipse/eclipse.jdt.ls/blob/57432947f5fe74ca43bd80079fc75614f5ebc8a7/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/JDTLanguageServer.java#L270-L288

  • Idea 2:
    If we cannot solve the blocking between build job and other language features, then the second option is to turn off autobuild by default. This saves build time and makes serivce ready earlier. This is also the default behavior of other IDEs like IJ.
    A side effect without autobuild is that we lose the diagnostics for the possible breaking caused by new changes. This will require additional work to maintain the reference relationship between classes in memory, and use that to report the potential breaking diagnostics when changing a method signature of a Java file.

// @akaroml @fbricon @rgrunber @mickaelistria @gayanper @jdneo @CsCherrYY @Eskibear
WDYT?

@mickaelistria
Copy link
Contributor

What's the ratio of project import vs build time on a regular project? I don't think much can be done to improve build time per se; but I believe a build is a mandatory phase to enable interesting language features anyway.
If you want to consider it from a JDT POV, the build part (actually generating the .class file) isn't the most expensive bit I believe; the expensive part is code analysis (parsing, getting a DOM...), that is mandatory to enable most language features. But that would need to be benchmarked; if it reveals to be profitable, we could try to investigate whether JDT can be made to work reliably without generating .class files, saving some CPU cycles.

We still keep autobuild on by default, but just leave it running in the background without blocking other language features such as completion. Not sure if this is possible, does anyone have more input on this?

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.
If it's only syntax you want, then you may consider having a "buildDidRun" flag in JDT-LS and use this flag to switch between the syntax server when buildDidRun==false and to the powerful LS when buildDidRun==true.

Why do we need to wait for build jobs during workspace initialization job? Can we remove it but register language server features and make service ready earlier?

I also wondered the same when touching this code recently. I don't think that explicitly waiting for build is interesting in that early stage. If some feature need a full build to work, then the particular feature should take care of waiting (or return incomplete results), not the whole LS.

Turn off autobuild by default. This saves build time and makes serivce ready earlier

That depends on what you mean by "ready". I personally wouldn't qualify as ready a language service that has a longer feedback loop than what it can offer (ie I don't see diagnostics on save).

In general, I think autobuild behaves and performs pretty well and JDT has tons of optimizations that make that the impact of auto-build is really not huge in CPU or other resources; so I would recommend to keep leveraging this good piece of work as long as it's useful.

@gayanper
Copy link
Contributor

gayanper commented Mar 9, 2023

@testforstephen I agree with what is mentioned by @mickaelistria. For example until the AST parsing happens, we could enable operations such as symbol navigations because they are mainly based on searchindex, of cause some operations might need AST resolving.

Another aspect is, do we really want to resolve all the maven and gradle stuff before enabling these operations ? Don't we cached classpath containers like Eclipse IDE does as .classpath files some where (I have not seen or looked into this on my side in jdt.ls) ?

@snjeza
Copy link
Contributor

snjeza commented Mar 9, 2023

Another aspect is, do we really want to resolve all the maven and gradle stuff before enabling these operations ? Don't we cached classpath containers like Eclipse IDE does as .classpath files some where (I have not seen or looked into this on my side in jdt.ls) ?

We only do this when we import the project for the first time.
m2e and buldship cache their configuration. Java LS uses it similar as Eclipse IDE. See

@snjeza
Copy link
Contributor

snjeza commented Mar 9, 2023

Back to jdtls code, here is where the initialzation is blocked by build job. Why do we need to wait for build jobs during workspace initialization job? Can we remove it but register language server features and make service ready earlier?

A related issue - #469

@testforstephen
Copy link
Contributor Author

What's the ratio of project import vs build time on a regular project? I don't think much can be done to improve build time per se; but I believe a build is a mandatory phase to enable interesting language features anyway.

We don't have accurate measure of the ratio of project import vs build time on a same project. On all vscode java sessions (include new project opens, and second opens), the ratio of project import time vs build time is about 5 : 3 at the 95th percentile.

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.

When users open VS Code, I believe the first features they expect to try immediately are navigation features (document symbol, go to definition, etc.) and code completion. I understand these features are not available before project import (project configuration resolving) finishes. But I don't think they have dependencies on the autobuild job (specifically the compilation of project). They should be available immediately after project configuration is imported.

Why do we need to wait for build jobs during workspace initialization job? Can we remove it but register language server features and make service ready earlier?

I also wondered the same when touching this code recently. I don't think that explicitly waiting for build is interesting in that early stage. If some feature need a full build to work, then the particular feature should take care of waiting (or return incomplete results), not the whole LS.

Agree with @mickaelistria. it should not block the whole LS.

@snjeza The PR #469 you mentioned didn't convince me why we need to wait for build job and block the whole LS. Could you give examples of features that need to delay after autobuild job finishes?

That depends on what you mean by "ready". I personally wouldn't qualify as ready a language service that has a longer feedback loop than what it can offer (ie I don't see diagnostics on save).
In general, I think autobuild behaves and performs pretty well and JDT has tons of optimizations that make that the impact of auto-build is really not huge in CPU or other resources; so I would recommend to keep leveraging this good piece of work as long as it's useful.

https://github.com/eclipse/eclipse.jdt.ls/blob/57432947f5fe74ca43bd80079fc75614f5ebc8a7/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/JDTLanguageServer.java#L293

"service ready" means JLS is ready to respond to all kinds of LSP requests. I see diagnostics as an auxiliary feature, not a user-triggered feature. Its speed does not affect the responsiveness users can perceive.

If the autobuild behavior doesn't block the responsiveness of language server, it's ok to leave it running. Currently looks like it has impact, that's why we discuss here to reduce its impact.

In addition, we have seen some side effects of autobuild. for example, for maven projects, jdt autobuild job watches on the target folder, this can conflict with maven command triggered by the user in the terminal.

@snjeza
Copy link
Contributor

snjeza commented Mar 10, 2023

In addition, we have seen some side effects of autobuild. for example, for maven projects, jdt autobuild job watches on the target folder, this can conflict with maven command triggered by the user in the terminal.

You may want to take a look at eclipse-m2e/m2e-core#1275

The PR #469 you mentioned didn't convince me why we need to wait for build job and block the whole LS. Could you give examples of features that need to delay after autobuild job finishes?

Could you check #2527 ? The PR only wait for build job in order to publish diagnostics.

@gayanper
Copy link
Contributor

By the way I use vscode with mono repo which contains around 10 microservices, and also I use vscode with PDE plugin to work on both jdt.ui and jdt.core repositories in the same workspace.

Some times back, I had to wait every time I open the vscode, But now I don't really see much difference from other tools like Eclipse or IJ. I think vscode is much responsive starting from the second run.

@Polish-Civil
Copy link

Polish-Civil commented Mar 19, 2023

I dont think i have much of value to say about this but i will drop my few cents.

I've also been struggling with large codebases and having responsive lsp. What i've ended up doing is preety simple - disable everything.

I dont know how exactly the auto configuration system works but i suspect it has alot of friction on the build tool being used, in my scenario is gradle. So i've disabled it completely and focus on generating .classpath and .project files manually with gradle's provided eclipse plugin. Works like a charm.

For the building part i've also disabled the gradle ones and focus on providing valid .classpath and .project files so that the internal eclipse build system also works like a charm. I guess that would disable the gradle's source gen and other custom build stuff, but i'm all fine with that. (even if i do source gen with gradle i would just adjust eclipse plugin to link the dirs and do source gen manually from gradlew)

My point being, instead of shipping multiple build systems/configuration that resolves configuration dynamically in code, generate stable raw eclipse's structure based on these tools. This would provide clear and well known structure to be used for both configuring and building.
There are some caveats especially with the module system, i've heard that eclipse uses build ship? which yeah, doesn't necessary work well with my setup and provides friction as i have said.

The default settings and configuration is usable for the small projects but it was hard for me to find lightweight configuration settings for big ones, which i think should be mentioned in the readme, as people like me might not really know what flags have resource implications.

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

No branches or pull requests

5 participants