Fix perf issue by preventing unnecessary building when open workspace. #756

yaohaizh · 2018-08-15T08:04:59Z

The force build on initialized will block the LSP protocol to wait for its response. And this action is not necessary to triggered every time when the auto build is true.
Restore the auto building value will make building as a worker thread and response to the client quickly.

With this change, eclipse.jdt.ls will match the performance of eclipse on import/reopen scenario.

fbricon

-1: diagnostics are no longer reported after reopening a existing project

yaohaizh · 2018-08-20T16:01:27Z

Diagnostic will be triggered by auto build in the back ground. This change is remove the force build in the foreground.

fbricon · 2018-08-20T16:07:50Z

No longer publishing diagnostics is a regression we can't allow. Background thread or not. They are absolutely not reported after opening the folder. I'm seeing that regression with this PR

yaohaizh · 2018-08-21T02:05:20Z

The diagnostic information is published after this change for auto build scenario.

The diagnostic information is NOT published if disable auto build. I can enable the force build for disable auto build scenario, but it is weird behavior.

fbricon · 2018-08-21T04:01:56Z

try opening https://github.com/spring-projects/spring-amqp in vscode with this PR
You can even set "java.autobuild.enabled": true just to be sure.

1st time the project opens, all diagnostics are published
restart vscode, no diagnostics are published. None.

I tried with the an empty workspaceStorage in vscode insiders

yaohaizh · 2018-08-21T05:08:01Z

This is an interesting thing. I originally used https://github.com/elastic/elasticsearch for testing gradle which is a large scale project, which makes diagnostic information published when reload.

Given these information, I think this should be the cause for the difference: #758

Given these, I will firstly make the force build as a worker thread to publish diagnostics.

yaohaizh · 2018-08-27T05:50:44Z

@fbricon Updated the PR to handle diagnostic situation.

fbricon · 2018-08-29T20:47:38Z

...e.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/WorkspaceDiagnosticsHandler.java

@@ -128,6 +143,66 @@ else if (projectsManager.isBuildFile(file)) {
 		return false;
 	}

+	public List<IMarker> publichDiagnostic(IProgressMonitor monitor) throws CoreException {


publishDiagnostics

fbricon · 2018-08-29T20:50:40Z

...e.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/WorkspaceDiagnosticsHandler.java

+		for (IProject project : projects) {
+			if (monitor != null && monitor.isCanceled()) {
+				throw new OperationCanceledException();
+			}


ignore default project, I think

It turns out this is the problem markers, but it will be ignored by publish diagnostics, which is the same behavior as the current design.

yaohaizh · 2018-08-30T01:34:00Z

@fbricon Fixed the issues.

yaohaizh · 2018-08-30T07:29:14Z

test this please

yaohaizh · 2018-08-30T15:01:17Z

@fbricon What happened with Jenkins?

fbricon · 2018-08-30T15:19:16Z

The PR fails to compile:

03:32:19 [ERROR] Failed to execute goal org.eclipse.tycho:tycho-compiler-plugin:1.2.0:compile (default-compile) on project org.eclipse.jdt.ls.tests: Compilation failure: Compilation failure: 
03:32:19 [ERROR] /jobs/genie.ls/jdt-ls-pr/workspace/org.eclipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/handlers/WorkspaceDiagnosticsHandlerTest.java:[249] 
03:32:19 [ERROR] 	initHandler.addWorkspaceDiagnosticsHandler();
03:32:19 [ERROR] 	            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
03:32:19 [ERROR] The method addWorkspaceDiagnosticsHandler() is undefined for the type InitHandler
03:32:19 [ERROR] 1 problem (1 error)

fbricon · 2018-08-31T01:40:36Z

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

@@ -62,7 +62,7 @@ public BuildWorkspaceStatus buildWorkspace(boolean forceReBuild, IProgressMonito
 				}
 			}
 			ResourcesPlugin.getWorkspace().build(forceReBuild ? IncrementalProjectBuilder.FULL_BUILD : IncrementalProjectBuilder.INCREMENTAL_BUILD, monitor);
-			List<IMarker> problemMarkers = workspaceDiagnosticsHandler.publichDiagnostic(monitor);
+			List<IMarker> problemMarkers = workspaceDiagnosticsHandler.publichDiagnostics(monitor);


it's publishDiagnostics with sh, not ch

fbricon · 2018-08-31T01:41:02Z

test this please

Signed-off-by: Yaohai Zheng <[email protected]>

yaohaizh · 2018-08-31T05:06:45Z

@fbricon Updated PR

yyoncho mentioned this pull request Aug 15, 2018

Really slow when first open a java file emacs-lsp/lsp-java#43

Closed

snjeza self-requested a review August 15, 2018 18:24

snjeza approved these changes Aug 15, 2018

View reviewed changes

fbricon requested changes Aug 20, 2018

View reviewed changes

yaohaizh force-pushed the yaohai_dev branch from 27c0f67 to 1d8e893 Compare August 26, 2018 05:33

yaohaizh mentioned this pull request Aug 27, 2018

Should publish diagnostic information for project level #759

Merged

yaohaizh force-pushed the yaohai_dev branch from 8411e53 to ee98c5a Compare August 28, 2018 07:16

fbricon reviewed Aug 29, 2018

View reviewed changes

yaohaizh force-pushed the yaohai_dev branch from ee98c5a to 7a08d70 Compare August 30, 2018 01:33

yaohaizh added 3 commits August 30, 2018 23:18

Fix perf issue by preventing unnecessary building when open workspace.

818426a

Refactor diagnostic code.

c8038bf

Fix test cases.

fe5c75e

yaohaizh force-pushed the yaohai_dev branch from 7a08d70 to 08d2292 Compare August 31, 2018 00:11

fbricon reviewed Aug 31, 2018

View reviewed changes

Address code review feedback.

4514d79

Signed-off-by: Yaohai Zheng <[email protected]>

yaohaizh force-pushed the yaohai_dev branch from 08d2292 to 4514d79 Compare August 31, 2018 04:57

fbricon added this to the End August 2018 milestone Aug 31, 2018

fbricon added enhancement performance labels Aug 31, 2018

fbricon merged commit 5b3ba86 into master Aug 31, 2018

yaohaizh deleted the yaohai_dev branch September 3, 2018 14:45

fbricon mentioned this pull request Oct 16, 2018

[regression] Maven diagnostics show up and disappear on save #829

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix perf issue by preventing unnecessary building when open workspace. #756

Fix perf issue by preventing unnecessary building when open workspace. #756

yaohaizh commented Aug 15, 2018

fbricon left a comment

yaohaizh commented Aug 20, 2018 •

edited

Loading

fbricon commented Aug 20, 2018

yaohaizh commented Aug 21, 2018 •

edited

Loading

fbricon commented Aug 21, 2018

yaohaizh commented Aug 21, 2018 •

edited

Loading

yaohaizh commented Aug 27, 2018

fbricon Aug 29, 2018

fbricon Aug 29, 2018

yaohaizh Aug 31, 2018

yaohaizh commented Aug 30, 2018 •

edited

Loading

yaohaizh commented Aug 30, 2018

yaohaizh commented Aug 30, 2018

fbricon commented Aug 30, 2018

fbricon Aug 31, 2018

fbricon commented Aug 31, 2018

yaohaizh commented Aug 31, 2018

Fix perf issue by preventing unnecessary building when open workspace. #756

Fix perf issue by preventing unnecessary building when open workspace. #756

Conversation

yaohaizh commented Aug 15, 2018

fbricon left a comment

Choose a reason for hiding this comment

yaohaizh commented Aug 20, 2018 • edited Loading

fbricon commented Aug 20, 2018

yaohaizh commented Aug 21, 2018 • edited Loading

fbricon commented Aug 21, 2018

yaohaizh commented Aug 21, 2018 • edited Loading

yaohaizh commented Aug 27, 2018

fbricon Aug 29, 2018

Choose a reason for hiding this comment

fbricon Aug 29, 2018

Choose a reason for hiding this comment

yaohaizh Aug 31, 2018

Choose a reason for hiding this comment

yaohaizh commented Aug 30, 2018 • edited Loading

yaohaizh commented Aug 30, 2018

yaohaizh commented Aug 30, 2018

fbricon commented Aug 30, 2018

fbricon Aug 31, 2018

Choose a reason for hiding this comment

fbricon commented Aug 31, 2018

yaohaizh commented Aug 31, 2018

yaohaizh commented Aug 20, 2018 •

edited

Loading

yaohaizh commented Aug 21, 2018 •

edited

Loading

yaohaizh commented Aug 21, 2018 •

edited

Loading

yaohaizh commented Aug 30, 2018 •

edited

Loading