Improve order of operations when importing multi-module Maven projects. #3150

snjeza · 2024-05-06T00:37:46Z

Fixes redhat-developer/vscode-java#3637

Steps to reproduce:

exit VS Code
Clean the workspace directory
clone the repository quarkus-langchain4j
cd quarkus-langchain4j
code .
wait for the build to finish
reload the VS Code window (it freezes without the PR; takes several seconds with the PR or VS Code 1.32.0)

Test vsix

testforstephen · 2024-05-06T05:37:06Z

@snjeza could you pls explain the root cause of this issue?

snjeza · 2024-05-06T18:48:53Z

could you pls explain the root cause of this issue?

There are several issues:

ProjectManager.registerWatchers() is called too early -cfb076c#diff-f82616576dc1fa363177feaadbbde3568ec33ec20588a8e7656ba45ded34bc52R330
StandardProjectsManager.configureSettings(Preferences, boolean) is always called from InitHandler.triggerInitialization(Collection) line 292 - IProjectsManager.registerListeners()
JavaCore.setOptions() is called even when the options have not been changed. It causes rebuild - cfb076c#diff-f23d0258ce24117c110acd6753e8490a09fbdf0bc039d662bdffe279c8c28469R362
Java LS updates maven projects always if the root maven project has the .mvn directory - cfb076c#diff-4b8c66d1045232aafc8ad5fc1c52decc87f1e67eb86815b224de947215ba48d2R197

snjeza · 2024-05-06T18:49:10Z

test this please

rgrunber · 2024-05-07T19:02:29Z

From testing this out, I don't see a major improvement in terms of build time or memory :

On quarkus-langchain4j project :
Prior to this change, initialization & "service ready" took about 75s and build time took around 190s. With this change, it's about the same (based on report from telemetry/event).

Without PR

With PR

We can still look at the changes here on their own based on whether they are an improvement (logically) but it's probably best if the commenters on MacOS can verify whether the change does anything for them in the issue.

snjeza · 2024-05-07T20:13:12Z

@rgrunber Could you try to restart the server after it has been successfully built?
When restarting, the server freezes without the patch. With the patch, it takes several seconds.
I suppose @philippart-s and @maxandersen have faced the issue when restarting the server,

@rgrunber You can try to restart the keycloak with and without the PR too - redhat-developer/vscode-java#3639

snjeza · 2024-05-07T21:48:19Z

@rgrunber I have updated the steps to reproduce. See steps 6 and 7 - #3150 (comment)
Step 6 works with and without the PR. The issue happens in step 7. It can't be reproduced in Eclipse.

snjeza · 2024-05-09T12:05:36Z

@rgrunber The workspace is always rebuilt when "java.jdt.ls.lombokSupport.enabled": true.
Try to set "java.jdt.ls.lombokSupport.enabled": false and restart the workspace.

Note: quarkus-langchain4j and keycloak have been built properly.
my settings.json:

{
    "java.import.generatesMetadataFilesAtProjectRoot": true,
    "java.compile.nullAnalysis.mode": "disabled",
    "java.gradle.buildServer.enabled": "off",
    "java.server.launchMode": "Standard",
    "java.jdt.ls.vmargs": "-Dlog.level=ALL -Djdt.ls.debug=true -XX:+UseParallelGC -XX:GCTimeRatio=4 -XX:AdaptiveSizePolicyWeight=90 -Dsun.zip.disableMemoryMapping=true -Xmx6G -Xms512m -Xlog:disable",
    "java.import.exclusions": [
        "**/node_modules/**",
        "**/node/**",
        "**/js/**",
        "**/distribution/**",
        "**/themes/**",
        "**/testsuite/**",
    ],
    "java.trace.server": "messages",
    "java.jdt.ls.lombokSupport.enabled": false,

rgrunber · 2024-05-09T16:54:31Z

I'm pretty confident this will improve things. In particular (prior to this PR), If I attempted to reload the workspace, I would see :

Basically a bunch of jobs to update individual projects. With your PR, this no longer happens and the language server never gets stuck or runs out of memory.

I did notice there's some extra build jobs in the case of lombok support being enabled but at least the builds were not getting stuck. I think all that's left for me is to go through each of the changes.

maxandersen · 2024-05-09T19:00:53Z

Was the use of of Lombok the trigger or how?

snjeza · 2024-05-09T19:30:57Z

Was the use of of Lombok the trigger or how?

@maxandersen There are several issues. Please see #3150 (comment)

snjeza · 2024-05-09T19:35:27Z

Basically a bunch of jobs to update individual projects. With your PR, this no longer happens and the language server never gets stuck or runs out of memory.

@rgrunber Please see #3150 (comment) issue 4

rgrunber

Just a few questions and comments.

...lipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/managers/StandardProjectsManager.java

...clipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/WorkspaceEventsHandler.java

org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/InitHandler.java

rgrunber · 2024-05-15T17:24:12Z

org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/JDTLanguageServer.java

@@ -577,7 +580,7 @@ public void didChangeConfiguration(DidChangeConfigurationParams params) {
 		try {
 			boolean isAutobuildEnabled = preferenceManager.getPreferences().isAutobuildEnabled();
 			boolean autoBuildChanged = ProjectsManager.setAutoBuilding(isAutobuildEnabled);
-			if (jvmChanged || nullAnalysisOptionsUpdated && isAutobuildEnabled) {
+			if ((jvmChanged || nullAnalysisOptionsUpdated) && isAutobuildEnabled) {


Good catch! So if we had any old issues of builds being triggered on a jvm change / null analsysis options update despite it being off, this would fix that also ?

rgrunber · 2024-05-15T18:15:10Z

....jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/preferences/StandardPreferenceManager.java

+					boolean hasMavenProjects = false;
+					for (IProject project : ProjectUtils.getAllProjects()) {
+						if (ProjectUtils.isMavenProject(project)) {
+							hasMavenProjects = true;
+							break;
+						}
+					}
+					projectManager.setShouldUpdateProjects(hasMavenProjects);


Wouldn't the part in the else always need to run because update(Preferences) is guaranteed to be called from BaseInitHandler.handleInitializationOptions(..) ? That runs before the build is finished so this is definitely always triggered.

If so then I would think you don't need the shouldUpdateProjects API, but let me know if I'm wrong.

We need it. I want to delay updating the projects after the initialization of Java LS.
See

cfb076c#diff-f82616576dc1fa363177feaadbbde3568ec33ec20588a8e7656ba45ded34bc52R332

cfb076c#diff-f23d0258ce24117c110acd6753e8490a09fbdf0bc039d662bdffe279c8c28469R671

org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/managers/IProjectsManager.java

snjeza · 2024-05-15T21:30:46Z

@rgrunber I have updated the PR.

snjeza · 2024-05-16T00:39:03Z

test this please

rgrunber

Only other change I would make is to the commit message. Rename this to something like :

Improve order of operations when importing multi-module Maven projects.

- Reduce unnecessary calls to updating projects

- Reduce unnecessary calls to updating projects Signed-off-by: Snjezana Peco <snjezana.peco@redhat.com>

snjeza · 2024-05-17T18:03:57Z

Only other change I would make is to the commit message. Rename this to something like :

@rgrubner I have done it.

snjeza added the in progress label May 6, 2024

snjeza requested review from fbricon, rgrunber, jdneo and testforstephen May 6, 2024 00:37

snjeza mentioned this pull request May 6, 2024

Building a project never finishes redhat-developer/vscode-java#3639

Open

snjeza force-pushed the issue-3637 branch from 6ac414d to cfb076c Compare May 6, 2024 19:13

snjeza changed the title ~~[WIP] VSCode JLS error when loading project with lot of Maven sub modules~~ VSCode JLS error when loading project with lot of Maven sub modules May 6, 2024

snjeza removed the in progress label May 6, 2024

rgrunber reviewed May 15, 2024

View reviewed changes

snjeza changed the title ~~VSCode JLS error when loading project with lot of Maven sub modules~~ [WIP] VSCode JLS error when loading project with lot of Maven sub modules May 15, 2024

snjeza added the in progress label May 15, 2024

snjeza force-pushed the issue-3637 branch from cfb076c to d866d75 Compare May 15, 2024 21:28

snjeza changed the title ~~[WIP] VSCode JLS error when loading project with lot of Maven sub modules~~ VSCode JLS error when loading project with lot of Maven sub modules May 15, 2024

snjeza removed the in progress label May 15, 2024

snjeza changed the title ~~VSCode JLS error when loading project with lot of Maven sub modules~~ [WIP] VSCode JLS error when loading project with lot of Maven sub modules May 15, 2024

snjeza added the in progress label May 15, 2024

snjeza force-pushed the issue-3637 branch from d866d75 to cfd1ec2 Compare May 16, 2024 00:03

snjeza changed the title ~~[WIP] VSCode JLS error when loading project with lot of Maven sub modules~~ VSCode JLS error when loading project with lot of Maven sub modules May 16, 2024

snjeza removed the in progress label May 16, 2024

rgrunber added this to the End May 2024 milestone May 17, 2024

rgrunber approved these changes May 17, 2024

View reviewed changes

Improve order of operations when importing multi-module Maven projects.

7d14cd0

- Reduce unnecessary calls to updating projects Signed-off-by: Snjezana Peco <snjezana.peco@redhat.com>

snjeza force-pushed the issue-3637 branch from cfd1ec2 to 7d14cd0 Compare May 17, 2024 18:02

rgrunber changed the title ~~VSCode JLS error when loading project with lot of Maven sub modules~~ Improve order of operations when importing multi-module Maven projects. May 17, 2024

rgrunber merged commit bfd86a5 into eclipse-jdtls:master May 17, 2024
6 of 7 checks passed

snjeza mentioned this pull request May 21, 2024

VSCode reloads Java projects on each startup which takes a very long time redhat-developer/vscode-java#2643

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve order of operations when importing multi-module Maven projects. #3150

Improve order of operations when importing multi-module Maven projects. #3150

snjeza commented May 6, 2024 •

edited

testforstephen commented May 6, 2024

snjeza commented May 6, 2024 •

edited

snjeza commented May 6, 2024

rgrunber commented May 7, 2024 •

edited

snjeza commented May 7, 2024

snjeza commented May 7, 2024 •

edited

snjeza commented May 9, 2024 •

edited

rgrunber commented May 9, 2024

maxandersen commented May 9, 2024

snjeza commented May 9, 2024

snjeza commented May 9, 2024

rgrunber left a comment

rgrunber May 15, 2024

rgrunber May 15, 2024

snjeza May 15, 2024

snjeza commented May 15, 2024

snjeza commented May 16, 2024

rgrunber left a comment

snjeza commented May 17, 2024

Improve order of operations when importing multi-module Maven projects. #3150

Improve order of operations when importing multi-module Maven projects. #3150

Conversation

snjeza commented May 6, 2024 • edited

testforstephen commented May 6, 2024

snjeza commented May 6, 2024 • edited

snjeza commented May 6, 2024

rgrunber commented May 7, 2024 • edited

snjeza commented May 7, 2024

snjeza commented May 7, 2024 • edited

snjeza commented May 9, 2024 • edited

rgrunber commented May 9, 2024

maxandersen commented May 9, 2024

snjeza commented May 9, 2024

snjeza commented May 9, 2024

rgrunber left a comment

Choose a reason for hiding this comment

rgrunber May 15, 2024

Choose a reason for hiding this comment

rgrunber May 15, 2024

Choose a reason for hiding this comment

snjeza May 15, 2024

Choose a reason for hiding this comment

snjeza commented May 15, 2024

snjeza commented May 16, 2024

rgrunber left a comment

Choose a reason for hiding this comment

snjeza commented May 17, 2024

snjeza commented May 6, 2024 •

edited

snjeza commented May 6, 2024 •

edited

rgrunber commented May 7, 2024 •

edited

snjeza commented May 7, 2024 •

edited

snjeza commented May 9, 2024 •

edited