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.

Sign up for GitHub

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

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

Merged
merged 4 commits into from
Aug 31, 2018
Merged

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

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
818426a
Fix perf issue by preventing unnecessary building when open workspace.
yaohaizh Aug 15, 2018
c8038bf
Refactor diagnostic code.
yaohaizh Aug 25, 2018
fe5c75e
Fix test cases.
yaohaizh Aug 27, 2018
4514d79
Address code review feedback.
yaohaizh Aug 30, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 4 additions & 68 deletions ...ipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/BuildWorkspaceHandler.java
View file
Original file line number Diff line number Diff line change
@@ -13,38 +13,25 @@
import static org.eclipse.jdt.ls.core.internal.JavaLanguageServerPlugin.logError;
import static org.eclipse.jdt.ls.core.internal.JavaLanguageServerPlugin.logException;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;

import org.eclipse.core.resources.IFile;
import org.eclipse.core.resources.IMarker;
import org.eclipse.core.resources.IProject;
import org.eclipse.core.resources.IResource;
import org.eclipse.core.resources.IncrementalProjectBuilder;
import org.eclipse.core.resources.ResourcesPlugin;
import org.eclipse.core.runtime.CoreException;
import org.eclipse.core.runtime.IProgressMonitor;
import org.eclipse.core.runtime.OperationCanceledException;
import org.eclipse.jdt.core.ICompilationUnit;
import org.eclipse.jdt.core.IJavaModelMarker;
import org.eclipse.jdt.core.JavaCore;
import org.eclipse.jdt.core.JavaModelException;
import org.eclipse.jdt.ls.core.internal.BuildWorkspaceStatus;
import org.eclipse.jdt.ls.core.internal.JDTUtils;
import org.eclipse.jdt.ls.core.internal.JavaClientConnection;
import org.eclipse.jdt.ls.core.internal.JavaLanguageServerPlugin;
import org.eclipse.jdt.ls.core.internal.ProjectUtils;
import org.eclipse.jdt.ls.core.internal.ResourceUtils;
import org.eclipse.jdt.ls.core.internal.managers.ProjectsManager;
import org.eclipse.jface.text.IDocument;
import org.eclipse.lsp4j.Diagnostic;
import org.eclipse.lsp4j.Position;
import org.eclipse.lsp4j.PublishDiagnosticsParams;
import org.eclipse.lsp4j.Range;

/**
* @author xuzho
@@ -53,10 +40,12 @@
public class BuildWorkspaceHandler {
private JavaClientConnection connection;
private final ProjectsManager projectsManager;
private final WorkspaceDiagnosticsHandler workspaceDiagnosticsHandler;

public BuildWorkspaceHandler(JavaClientConnection connection, ProjectsManager projectsManager) {
public BuildWorkspaceHandler(JavaClientConnection connection, ProjectsManager projectsManager, WorkspaceDiagnosticsHandler workspaceDiagnosticHandler) {
this.connection = connection;
this.projectsManager = projectsManager;
this.workspaceDiagnosticsHandler = workspaceDiagnosticHandler;
}

public BuildWorkspaceStatus buildWorkspace(boolean forceReBuild, IProgressMonitor monitor) {
@@ -73,8 +62,7 @@ public BuildWorkspaceStatus buildWorkspace(boolean forceReBuild, IProgressMonito
}
}
ResourcesPlugin.getWorkspace().build(forceReBuild ? IncrementalProjectBuilder.FULL_BUILD : IncrementalProjectBuilder.INCREMENTAL_BUILD, monitor);
List<IMarker> problemMarkers = getProblemMarkers(monitor);
publishDiagnostics(problemMarkers);
List<IMarker> problemMarkers = workspaceDiagnosticsHandler.publishDiagnostics(monitor);
List<String> errors = problemMarkers.stream().filter(m -> m.getAttribute(IMarker.SEVERITY, 0) == IMarker.SEVERITY_ERROR).map(e -> convertMarker(e)).collect(Collectors.toList());
if (errors.isEmpty()) {
return BuildWorkspaceStatus.SUCCEED;
@@ -92,58 +80,6 @@ public BuildWorkspaceStatus buildWorkspace(boolean forceReBuild, IProgressMonito
}
}

private static List<IMarker> getProblemMarkers(IProgressMonitor monitor) throws CoreException {
IProject[] projects = ResourcesPlugin.getWorkspace().getRoot().getProjects();
List<IMarker> markers = new ArrayList<>();
for (IProject project : projects) {
if (monitor.isCanceled()) {
throw new OperationCanceledException();
}
markers.addAll(Arrays.asList(project.findMarkers(IMarker.PROBLEM, true, IResource.DEPTH_INFINITE)));
}
return markers;
}

private void publishDiagnostics(List<IMarker> markers) {
Map<IResource, List<IMarker>> map = markers.stream().collect(Collectors.groupingBy(IMarker::getResource));
for (Map.Entry<IResource, List<IMarker>> entry : map.entrySet()) {
IResource resource = entry.getKey();
// ignore problems caused by standalone files
if (JavaLanguageServerPlugin.getProjectsManager().getDefaultProject().equals(resource.getProject())) {
continue;
}
if (resource instanceof IProject) {
String uri = JDTUtils.getFileURI(resource);
Range range = new Range(new Position(0, 0), new Position(0, 0));
List<Diagnostic> diagnostics = WorkspaceDiagnosticsHandler.toDiagnosticArray(range, entry.getValue().toArray(new IMarker[0]));
connection.publishDiagnostics(new PublishDiagnosticsParams(ResourceUtils.toClientUri(uri), diagnostics));
continue;
}
IFile file = resource.getAdapter(IFile.class);
if (file == null) {
continue;
}
IDocument document = null;
String uri = JDTUtils.getFileURI(resource);
if (JavaCore.isJavaLikeFileName(file.getName())) {
ICompilationUnit cu = JDTUtils.resolveCompilationUnit(uri);
try {
document = JsonRpcHelpers.toDocument(cu.getBuffer());
} catch (JavaModelException e) {
logException("Failed to publish diagnostics.", e);
}
}
else if (projectsManager.isBuildFile(file)) {
document = JsonRpcHelpers.toDocument(file);
}

if (document != null) {
List<Diagnostic> diagnostics = WorkspaceDiagnosticsHandler.toDiagnosticsArray(document, entry.getValue().toArray(new IMarker[0]));
connection.publishDiagnostics(new PublishDiagnosticsParams(ResourceUtils.toClientUri(uri), diagnostics));
}
}
}

private static String convertMarker(IMarker marker) {
StringBuilder builder = new StringBuilder();
String message = marker.getAttribute(IMarker.MESSAGE, "<no message>");
17 changes: 1 addition & 16 deletions org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/InitHandler.java
View file
Original file line number Diff line number Diff line change
@@ -21,7 +21,6 @@
import java.util.Map;
import java.util.Set;

import org.eclipse.core.resources.IResourceChangeEvent;
import org.eclipse.core.resources.ResourcesPlugin;
import org.eclipse.core.resources.WorkspaceJob;
import org.eclipse.core.runtime.CoreException;
@@ -66,7 +65,6 @@ final public class InitHandler {
private ProjectsManager projectsManager;
private JavaClientConnection connection;
private PreferenceManager preferenceManager;
private static WorkspaceDiagnosticsHandler workspaceDiagnosticsHandler;

public InitHandler(ProjectsManager manager, PreferenceManager preferenceManager, JavaClientConnection connection) {
this.projectsManager = manager;
@@ -123,7 +121,6 @@ InitializeResult initialize(InitializeParams param) {
}
preferenceManager.getPreferences().setRootPaths(rootPaths);
triggerInitialization(rootPaths);
addWorkspaceDiagnosticsHandler();
Integer processId = param.getProcessId();
if (processId != null) {
JavaLanguageServerPlugin.getLanguageServer().setParentProcessId(processId.longValue());
@@ -221,6 +218,7 @@ public IStatus runInWorkspace(IProgressMonitor monitor) {
try {
projectsManager.setAutoBuilding(false);
projectsManager.initializeProjects(roots, subMonitor);
projectsManager.setAutoBuilding(preferenceManager.getPreferences().isAutobuildEnabled());
JavaLanguageServerPlugin.logInfo("Workspace initialized in " + (System.currentTimeMillis() - start) + "ms");
connection.sendStatus(ServiceStatus.Started, "Ready");
} catch (OperationCanceledException e) {
@@ -261,17 +259,4 @@ private <T> T getInitializationOption(Map<?, ?> initializationOptions, String ke
return null;
}

@Deprecated
public static void removeWorkspaceDiagnosticsHandler() {
if (workspaceDiagnosticsHandler != null) {
ResourcesPlugin.getWorkspace().removeResourceChangeListener(workspaceDiagnosticsHandler);
workspaceDiagnosticsHandler = null;
}
}

public void addWorkspaceDiagnosticsHandler() {
removeWorkspaceDiagnosticsHandler();
workspaceDiagnosticsHandler = new WorkspaceDiagnosticsHandler(connection, projectsManager);
ResourcesPlugin.getWorkspace().addResourceChangeListener(workspaceDiagnosticsHandler, IResourceChangeEvent.POST_CHANGE);
}
}
26 changes: 18 additions & 8 deletions org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/JDTLanguageServer.java
View file
Original file line number Diff line number Diff line change
@@ -124,6 +124,7 @@ public class JDTLanguageServer implements LanguageServer, TextDocumentService, W
private LanguageServerWorkingCopyOwner workingCopyOwner;
private PreferenceManager preferenceManager;
private DocumentLifeCycleHandler documentLifeCycleHandler;
private WorkspaceDiagnosticsHandler workspaceDiagnosticsHandler;

private Set<String> registeredCapabilities = new HashSet<>(3);

@@ -205,12 +206,18 @@ public void initialized(InitializedParams params) {
// we do not have the user setting initialized yet at this point but we should
// still call to enable defaults in case client does not support configuration changes
syncCapabilitiesToSettings();
try {
boolean autoBuildChanged = pm.setAutoBuilding(preferenceManager.getPreferences().isAutobuildEnabled());
buildWorkspace(autoBuildChanged);
} catch (CoreException e) {
JavaLanguageServerPlugin.logException(e.getMessage(), e);
}

workspaceDiagnosticsHandler = new WorkspaceDiagnosticsHandler(this.client, pm);
workspaceDiagnosticsHandler.addResourceChangeListener();

computeAsync((monitor) -> {
try {
workspaceDiagnosticsHandler.publishDiagnostics(monitor);
} catch (CoreException e) {
logException(e.getMessage(), e);
}
return new Object();
});
}

/**
@@ -250,7 +257,10 @@ public CompletableFuture<Object> shutdown() {
logInfo(">> shutdown");
return computeAsync((monitor) -> {
try {
InitHandler.removeWorkspaceDiagnosticsHandler();
if (workspaceDiagnosticsHandler != null) {
workspaceDiagnosticsHandler.removeResourceChangeListener();
workspaceDiagnosticsHandler = null;
}
ResourcesPlugin.getWorkspace().save(true, monitor);
} catch (CoreException e) {
logException(e.getMessage(), e);
@@ -687,7 +697,7 @@ public void projectConfigurationUpdate(TextDocumentIdentifier param) {
@Override
public CompletableFuture<BuildWorkspaceStatus> buildWorkspace(boolean forceReBuild) {
logInfo(">> java/buildWorkspace (" + (forceReBuild ? "full)" : "incremental)"));
BuildWorkspaceHandler handler = new BuildWorkspaceHandler(client, pm);
BuildWorkspaceHandler handler = new BuildWorkspaceHandler(client, pm, workspaceDiagnosticsHandler);
return computeAsync((monitor) -> handler.buildWorkspace(forceReBuild, monitor));
}

75 changes: 75 additions & 0 deletions ...dt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/WorkspaceDiagnosticsHandler.java
View file
Original file line number Diff line number Diff line change
@@ -10,22 +10,29 @@
*******************************************************************************/
package org.eclipse.jdt.ls.core.internal.handlers;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;
import java.util.stream.Stream;

import org.eclipse.core.resources.IFile;
import org.eclipse.core.resources.IMarker;
import org.eclipse.core.resources.IProject;
import org.eclipse.core.resources.IResource;
import org.eclipse.core.resources.IResourceChangeEvent;
import org.eclipse.core.resources.IResourceChangeListener;
import org.eclipse.core.resources.IResourceDelta;
import org.eclipse.core.resources.IResourceDeltaVisitor;
import org.eclipse.core.resources.ResourcesPlugin;
import org.eclipse.core.runtime.CoreException;
import org.eclipse.core.runtime.IProgressMonitor;
import org.eclipse.core.runtime.OperationCanceledException;
import org.eclipse.jdt.core.ICompilationUnit;
import org.eclipse.jdt.core.IJavaModelMarker;
import org.eclipse.jdt.core.JavaCore;
import org.eclipse.jdt.core.JavaModelException;
import org.eclipse.jdt.ls.core.internal.JDTUtils;
import org.eclipse.jdt.ls.core.internal.JavaClientConnection;
import org.eclipse.jdt.ls.core.internal.JavaLanguageServerPlugin;
@@ -56,6 +63,14 @@ public WorkspaceDiagnosticsHandler(JavaClientConnection connection, ProjectsMana
this.projectsManager = projectsManager;
}

public void addResourceChangeListener() {
ResourcesPlugin.getWorkspace().addResourceChangeListener(this, IResourceChangeEvent.POST_CHANGE);
}

public void removeResourceChangeListener() {
ResourcesPlugin.getWorkspace().removeResourceChangeListener(this);
}

@Override
public void resourceChanged(IResourceChangeEvent event) {
try {
@@ -128,6 +143,66 @@ else if (projectsManager.isBuildFile(file)) {
return false;
}

public List<IMarker> publishDiagnostics(IProgressMonitor monitor) throws CoreException {
List<IMarker> problemMarkers = getProblemMarkers(monitor);
publishDiagnostics(problemMarkers);
return problemMarkers;
}

private List<IMarker> getProblemMarkers(IProgressMonitor monitor) throws CoreException {
IProject[] projects = ResourcesPlugin.getWorkspace().getRoot().getProjects();
List<IMarker> markers = new ArrayList<>();
for (IProject project : projects) {
if (monitor != null && monitor.isCanceled()) {
throw new OperationCanceledException();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

ignore default project, I think

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

markers.addAll(Arrays.asList(project.findMarkers(IJavaModelMarker.JAVA_MODEL_PROBLEM_MARKER, true, IResource.DEPTH_INFINITE)));
markers.addAll(Arrays.asList(project.findMarkers(IJavaModelMarker.TASK_MARKER, true, IResource.DEPTH_INFINITE)));
}
return markers;
}

private void publishDiagnostics(List<IMarker> markers) {
Map<IResource, List<IMarker>> map = markers.stream().collect(Collectors.groupingBy(IMarker::getResource));
for (Map.Entry<IResource, List<IMarker>> entry : map.entrySet()) {
IResource resource = entry.getKey();
// ignore problems caused by standalone files
if (JavaLanguageServerPlugin.getProjectsManager().getDefaultProject().equals(resource.getProject())) {
continue;
}
if (resource instanceof IProject) {
String uri = JDTUtils.getFileURI(resource);
Range range = new Range(new Position(0, 0), new Position(0, 0));
List<Diagnostic> diagnostics = WorkspaceDiagnosticsHandler.toDiagnosticArray(range, entry.getValue().toArray(new IMarker[0]));
connection.publishDiagnostics(new PublishDiagnosticsParams(ResourceUtils.toClientUri(uri), diagnostics));
continue;
}
IFile file = resource.getAdapter(IFile.class);
if (file == null) {
continue;
}
IDocument document = null;
String uri = JDTUtils.getFileURI(resource);
if (JavaCore.isJavaLikeFileName(file.getName())) {
ICompilationUnit cu = JDTUtils.resolveCompilationUnit(uri);
try {
document = JsonRpcHelpers.toDocument(cu.getBuffer());
} catch (JavaModelException e) {
JavaLanguageServerPlugin.logException("Failed to publish diagnostics.", e);
}
} else if (projectsManager.isBuildFile(file)) {
document = JsonRpcHelpers.toDocument(file);
}

if (document != null) {
List<Diagnostic> diagnostics = WorkspaceDiagnosticsHandler.toDiagnosticsArray(document, entry.getValue().toArray(new IMarker[0]));
connection.publishDiagnostics(new PublishDiagnosticsParams(ResourceUtils.toClientUri(uri), diagnostics));
}
}
}



/**
* Transforms {@link IMarker}s into a list of {@link Diagnostic}s
*
2 changes: 1 addition & 1 deletion ...jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/handlers/BuildWorkspaceHandlerTest.java
View file
Original file line number Diff line number Diff line change
@@ -35,7 +35,7 @@ public class BuildWorkspaceHandlerTest extends AbstractProjectsManagerBasedTest

@Before
public void setUp() throws Exception {
handler = new BuildWorkspaceHandler(javaClient, projectsManager);
handler = new BuildWorkspaceHandler(javaClient, projectsManager, new WorkspaceDiagnosticsHandler(javaClient, projectsManager));
importProjects("maven/salut2");
file = linkFilesToDefaultProject("singlefile/Single.java");
}
Loading