Bad performance when working with non quarkus/MP projects #290

angelozerr · 2020-04-02T08:55:21Z

Bad perfomance when working with non quarkus/MP projects

angelozerr · 2020-04-02T09:04:26Z

At first, to test this PR you need the vscode-quarkus PR https://github.com/redhat-developer/vscode-quarkus/pull/242/files

To test this PR:

check that diagnostics, code action is working for a MP/Quarkus project
open a Java file which doesn't belong to a MP project (ex : Lemminx) an check that in Quarkus Tools trace you have none microprofile/java/* trace. The only trace with microprofile is microprofile/java/projectLabels (used to collect labels of the project of the java file). This service should for the first java file which belongs to the project.

An avanced test, is to change the pom.xml dependency by adding a MP/Quarkus dependency to Lemminx and check you have traces with microprofile/java/* when a java file from Lemminx is changed (codeAction, codelens, Hover, etc).

@fbricon could you give me feedback about this PR if your performance are better?

This PR is a draft PR because I need to clean my code and write some tests.

xorye · 2020-04-02T16:39:57Z

To test this PR:

check that diagnostics, code action is working for a MP/Quarkus project
open a Java file which doesn't belong to a MP project (ex : Lemminx) an check that in Quarkus Tools trace you have none microprofile/java/* trace. The only trace with microprofile is microprofile/java/projectLabels (used to collect labels of the project of the java file). This service should for the first java file which belongs to the project.

This works on my machine

An avanced test, is to change the pom.xml dependency by adding a MP/Quarkus dependency to Lemminx and check you have traces with microprofile/java/* when a java file from Lemminx is changed (codeAction, codelens, Hover, etc).

I am having some issues with this part. When I add Quarkus dependencies to the Lemminx pom.xml, I do not get microprofile/java/* traces on hover, codeaction etc. However, I do get them once I restart VS Code.

Same for the other way around. I get microprofile/java/* traces on Quarkus projects, but when I remove the Quarkus dependencies, I still get the microprofile/java/* traces.

Here are the Quarkus elements I added/removed in the pom.xml:
Inside the <properties> tag, I added:

    <compiler-plugin.version>3.8.1</compiler-plugin.version>
    <maven.compiler.parameters>true</maven.compiler.parameters>
    <maven.compiler.source>11</maven.compiler.source>
    <maven.compiler.target>11</maven.compiler.target>
    <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
    <project.reporting.outputEncoding>UTF-8</project.reporting.outputEncoding>
    <quarkus-plugin.version>1.3.1.Final</quarkus-plugin.version>
    <quarkus.platform.artifact-id>quarkus-universe-bom</quarkus.platform.artifact-id>
    <quarkus.platform.group-id>io.quarkus</quarkus.platform.group-id>
    <quarkus.platform.version>1.3.1.Final</quarkus.platform.version>
    <surefire-plugin.version>2.22.1</surefire-plugin.version>

Inside the <dependencies> tag, I added:

		<dependency>
			<groupId>io.quarkus</groupId>
			<artifactId>quarkus-resteasy</artifactId>
			<version>1.3.1.Final</version>
		</dependency>
		<dependency>
			<groupId>io.quarkus</groupId>
			<artifactId>quarkus-junit5</artifactId>
			<scope>test</scope>
			<version>1.3.1.Final</version>
		</dependency>
		<dependency>
			<groupId>io.rest-assured</groupId>
			<artifactId>rest-assured</artifactId>
			<scope>test</scope>
			<version>4.3.0</version>
		</dependency>

angelozerr · 2020-04-04T10:13:06Z

I am having some issues with this part.

It should work now, please retry

angelozerr · 2020-04-06T09:11:13Z

My PR is ready now (my code should be cleaned, tests are written).

@fbricon please tell me if it improves the performance problem that you had had with lemMinx.
@xorye please redo your test, it should work now.
@jeffmaury pay attention, you will need to implement MicroProfileJavaProjectLabelsProvider in your LSP4E client otherwise the Java support will be broken.

fbricon · 2020-04-06T10:40:16Z

...ain/java/com/redhat/microprofile/jdt/internal/core/ls/JavaProjectDelegateCommandHandler.java

@@ -31,9 +37,29 @@ public JavaProjectDelegateCommandHandler() {
 	public Object executeCommand(String commandId, List<Object> arguments, IProgressMonitor progress) throws Exception {
 		switch (commandId) {
 		case PROJECT_LABELS_COMMAND_ID:


should we deprecate this command?

This command is used for this issue. Before this pr this command returned a List of project used by typescript. I renamed this command by adding a s. In other words the two commands are requiered

So you changed the semantic of the existing command (bad) and the 2 command names are too close (bad).

So you changed the semantic of the existing command (bad)

Indeed but it's only TypeScript which consumes it and the command name uses "project" although it's for all project and not for a given project

and the 2 command names are too close (bad).

What is your suggestion?

...ain/java/com/redhat/microprofile/jdt/internal/core/ls/JavaProjectDelegateCommandHandler.java

....core/src/main/java/com/redhat/microprofile/commons/MicroProfileJavaProjectLabelsParams.java

...icroprofile.jdt.core/src/main/java/com/redhat/microprofile/jdt/core/ProjectLabelManager.java

fbricon · 2020-04-06T11:01:06Z

...com/redhat/microprofile/jdt/internal/core/ls/AbstractMicroProfileDelegateCommandHandler.java

+	private static final IMicroProfilePropertiesChangedListener LISTENER = (event) -> {
+		try {
+			// Execute client command with a timeout of 5 seconds to avoid blocking jobs.
+			JavaLanguageServerPlugin.getInstance().getClientConnection().executeClientCommand(


should be getClientConnection().sendNotification(), since we're not waiting for a response

Yes it should be that, but there is a bug on vscode-java. I tried to fix it with the following PR redhat-developer/vscode-java#1369

fbricon · 2020-04-06T11:05:33Z

...ain/java/com/redhat/microprofile/jdt/internal/core/ls/JavaProjectDelegateCommandHandler.java

@@ -31,9 +37,29 @@ public JavaProjectDelegateCommandHandler() {
 	public Object executeCommand(String commandId, List<Object> arguments, IProgressMonitor progress) throws Exception {
 		switch (commandId) {
 		case PROJECT_LABELS_COMMAND_ID:


So you changed the semantic of the existing command (bad) and the 2 command names are too close (bad).

fbricon · 2020-04-06T11:08:23Z

...redhat.microprofile.ls/src/main/java/com/redhat/microprofile/ls/JavaTextDocumentService.java

@@ -102,9 +102,12 @@ public void didClose(DidCloseTextDocumentParams params) {

 	@Override
 	public void didSave(DidSaveTextDocumentParams params) {
-		triggerValidationFor(documents.all().stream().map(TextDocument::getUri).collect(Collectors.toList()));
+		// validate all opened java files which belong to a MicroProfile project
+		triggerValidationForAll(null);


we're losing the info about which file was saved, it'll certainly come in handy later

we're losing the info about which file was saved, it'll certainly come in handy later

We don't need this information, the idea is when a java file is saved, all diagnostics are recomputed (to managed diagnostics for Java files which could depend on the saved java file).

You want I add the uri as parameter in triggerValidationForAll (even if it is not used)?

...m.redhat.microprofile.ls/src/test/java/com/redhat/microprofile/ls/JavaTextDocumentsTest.java

xorye · 2020-04-06T16:27:01Z

The fix works/looks great to me

See redhat-developer/vscode-quarkus#240 Signed-off-by: azerr <azerr@redhat.com>

angelozerr · 2020-04-06T16:58:13Z

The fix works/looks great to me

Thanks for the feedback.

I have just renamed the command (with projectlabels and workspaceLabels). Please update the vscode-quarkus PR https://github.com/redhat-developer/vscode-quarkus/pull/242/files too.

@xorye could you check that the call of JDT LS of workspaceLabels works like before in TypeScript side when you collect the labels of projects.

xorye · 2020-04-06T19:34:53Z

could you check that the call of JDT LS of workspaceLabels works like before in TypeScript side when you collect the labels of projects.

It is still working like before.

It looks like https://github.com/redhat-developer/vscode-quarkus/pull/242/files already has the updated workspaceLabels command. Is there something else I should update?

fbricon · 2020-04-07T14:35:53Z

Seems much better, but I still see microprofile/java/projectLabels requests whenever I open a new java file in LemMinX

angelozerr · 2020-04-07T14:43:27Z

Seems much better, but I still see microprofile/java/projectLabels requests whenever I open a new java file in LemMinX

When you open a Java file, you need to know the project URI owner. To do that, you need to trigger a JDT LS extension command. We could have a simple JDT LS command which returns the project URI of the java file uri and call after microprofile/java/projectLabels if the project is not cached, but as computing labels for a project is very quick, I consume it when file is opened.

I have not found a better way to get the project URI of the opened java file which requires a call of JDT LS command.

fbricon · 2020-04-07T14:51:43Z

ok fair enough. Since it's just a single call on open, it should be fine

angelozerr · 2020-04-07T14:57:05Z

ok fair enough. Since it's just a single call on open, it should be fine

Yes it's that.

angelozerr · 2020-04-07T15:26:54Z

@fbricon @xorye can we merge this PR please? I need it for the Java completion filter issue.

angelozerr force-pushed the java-improve-perf branch from 235b7c5 to 2499336 Compare April 2, 2020 16:30

angelozerr force-pushed the java-improve-perf branch 5 times, most recently from 20e98ad to 9dde208 Compare April 4, 2020 09:54

angelozerr force-pushed the java-improve-perf branch 2 times, most recently from 180479d to c78dd1f Compare April 6, 2020 09:04

angelozerr marked this pull request as ready for review April 6, 2020 09:07

angelozerr requested review from fbricon and xorye April 6, 2020 09:07

fbricon reviewed Apr 6, 2020

View reviewed changes

fbricon requested changes Apr 6, 2020

View reviewed changes

angelozerr force-pushed the java-improve-perf branch 2 times, most recently from e5949d1 to fe9a97d Compare April 6, 2020 12:20

angelozerr force-pushed the java-improve-perf branch from fe9a97d to e10e9c7 Compare April 6, 2020 16:54

Bad perfomance when working with non quarkus/MP projects

e10e9c7

See redhat-developer/vscode-quarkus#240 Signed-off-by: azerr <azerr@redhat.com>

fbricon approved these changes Apr 7, 2020

View reviewed changes

fbricon merged commit 8680aa9 into redhat-developer:master Apr 7, 2020

fbricon changed the title ~~Bad perfomance when working with non quarkus/MP projects~~ Bad performance when working with non quarkus/MP projects Apr 8, 2020

fbricon added bug Something isn't working performance labels Apr 15, 2020

fbricon added this to the v0.0.6 milestone Apr 15, 2020

fbricon mentioned this pull request Apr 15, 2020

CHANGELOG for 0.0.6 #305

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bad performance when working with non quarkus/MP projects #290

Bad performance when working with non quarkus/MP projects #290

angelozerr commented Apr 2, 2020

angelozerr commented Apr 2, 2020

xorye commented Apr 2, 2020

angelozerr commented Apr 4, 2020

angelozerr commented Apr 6, 2020

fbricon Apr 6, 2020

angelozerr Apr 6, 2020

fbricon Apr 6, 2020

angelozerr Apr 6, 2020

fbricon Apr 6, 2020

angelozerr Apr 6, 2020 •

edited

Loading

fbricon Apr 6, 2020

fbricon Apr 6, 2020

angelozerr Apr 6, 2020

xorye commented Apr 6, 2020

angelozerr commented Apr 6, 2020

xorye commented Apr 6, 2020

fbricon commented Apr 7, 2020

angelozerr commented Apr 7, 2020 •

edited

Loading

fbricon commented Apr 7, 2020

angelozerr commented Apr 7, 2020

angelozerr commented Apr 7, 2020

Bad performance when working with non quarkus/MP projects #290

Bad performance when working with non quarkus/MP projects #290

Conversation

angelozerr commented Apr 2, 2020

angelozerr commented Apr 2, 2020

xorye commented Apr 2, 2020

angelozerr commented Apr 4, 2020

angelozerr commented Apr 6, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

angelozerr Apr 6, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xorye commented Apr 6, 2020

angelozerr commented Apr 6, 2020

xorye commented Apr 6, 2020

fbricon commented Apr 7, 2020

angelozerr commented Apr 7, 2020 • edited Loading

fbricon commented Apr 7, 2020

angelozerr commented Apr 7, 2020

angelozerr commented Apr 7, 2020

angelozerr Apr 6, 2020 •

edited

Loading

angelozerr commented Apr 7, 2020 •

edited

Loading