From 8bffa957420ab7b987e2278111d53f78a5bb176b Mon Sep 17 00:00:00 2001 From: Kris De Volder Date: Tue, 28 May 2019 10:12:25 -0700 Subject: [PATCH] Towards batched classpath events - add parameter to 'addClasspathListener' to request batched events. - Create a single SendClasspathNotificationsJob that processes classpath notifications in batches. --- eclipse-language-servers/local-build.sh | 2 +- .../commons/STS4LanguageClientImpl.java | 2 +- .../ReusableClasspathListenerHandler.java | 133 +++------------- .../SendClasspathNotificationsJob.java | 143 ++++++++++++++++++ .../extension/ClasspathListenerHandler.java | 10 +- 5 files changed, 174 insertions(+), 116 deletions(-) create mode 100644 headless-services/jdt-ls-extension/org.springframework.tooling.jdt.ls.commons/src/org/springframework/tooling/jdt/ls/commons/classpath/SendClasspathNotificationsJob.java diff --git a/eclipse-language-servers/local-build.sh b/eclipse-language-servers/local-build.sh index 961ee1169..075d55731 100755 --- a/eclipse-language-servers/local-build.sh +++ b/eclipse-language-servers/local-build.sh @@ -5,4 +5,4 @@ cd ../headless-services ./mvnw clean install -Dmaven.test.skip=true cd $workdir -./mvnw -Pe410 clean install -Dmaven.test.skip=true \ No newline at end of file +./mvnw -Pe411 clean install -Dmaven.test.skip=true diff --git a/eclipse-language-servers/org.springframework.tooling.ls.eclipse.commons/src/org/springframework/tooling/ls/eclipse/commons/STS4LanguageClientImpl.java b/eclipse-language-servers/org.springframework.tooling.ls.eclipse.commons/src/org/springframework/tooling/ls/eclipse/commons/STS4LanguageClientImpl.java index 309700c86..b39976e8b 100644 --- a/eclipse-language-servers/org.springframework.tooling.ls.eclipse.commons/src/org/springframework/tooling/ls/eclipse/commons/STS4LanguageClientImpl.java +++ b/eclipse-language-servers/org.springframework.tooling.ls.eclipse.commons/src/org/springframework/tooling/ls/eclipse/commons/STS4LanguageClientImpl.java @@ -360,7 +360,7 @@ public class STS4LanguageClientImpl extends LanguageClientImpl implements STS4La @Override public CompletableFuture addClasspathListener(ClasspathListenerParams params) { - return CompletableFuture.completedFuture(classpathService.addClasspathListener(params.getCallbackCommandId())); + return CompletableFuture.completedFuture(classpathService.addClasspathListener(params.getCallbackCommandId(), params.isBatched())); } @Override diff --git a/headless-services/jdt-ls-extension/org.springframework.tooling.jdt.ls.commons/src/org/springframework/tooling/jdt/ls/commons/classpath/ReusableClasspathListenerHandler.java b/headless-services/jdt-ls-extension/org.springframework.tooling.jdt.ls.commons/src/org/springframework/tooling/jdt/ls/commons/classpath/ReusableClasspathListenerHandler.java index 19dc08f90..ebeccc797 100644 --- a/headless-services/jdt-ls-extension/org.springframework.tooling.jdt.ls.commons/src/org/springframework/tooling/jdt/ls/commons/classpath/ReusableClasspathListenerHandler.java +++ b/headless-services/jdt-ls-extension/org.springframework.tooling.jdt.ls.commons/src/org/springframework/tooling/jdt/ls/commons/classpath/ReusableClasspathListenerHandler.java @@ -10,107 +10,61 @@ *******************************************************************************/ package org.springframework.tooling.jdt.ls.commons.classpath; -import java.io.File; -import java.net.URI; import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.Comparator; import java.util.HashMap; -import java.util.HashSet; import java.util.Map; -import java.util.Set; import java.util.function.Supplier; import org.eclipse.core.resources.IProject; import org.eclipse.core.resources.ResourcesPlugin; import org.eclipse.core.runtime.CoreException; -import org.eclipse.core.runtime.IProgressMonitor; -import org.eclipse.core.runtime.IStatus; -import org.eclipse.core.runtime.Status; -import org.eclipse.core.runtime.jobs.Job; import org.eclipse.jdt.core.IJavaProject; import org.eclipse.jdt.core.JavaCore; -import org.springframework.ide.vscode.commons.protocol.java.Classpath; import org.springframework.tooling.jdt.ls.commons.Logger; import org.springframework.tooling.jdt.ls.commons.classpath.ClasspathListenerManager.ClasspathListener; +import org.springframework.tooling.jdt.ls.commons.classpath.SendClasspathNotificationsJob.Notification; /** * {@link ReusableClasspathListenerHandler} is an 'abstracted' version of the jdtls ClasspathListenerHandler. */ public class ReusableClasspathListenerHandler { - private final ClientCommandExecutor conn; private final Logger logger; private final Supplier> projectSorterFactory; + private final SendClasspathNotificationsJob sendNotificationJob; public ReusableClasspathListenerHandler(Logger logger, ClientCommandExecutor conn) { this(logger, conn, null); } public ReusableClasspathListenerHandler(Logger logger, ClientCommandExecutor conn, Supplier> projectSorterFactory) { - this.conn = conn; this.logger = logger; this.projectSorterFactory = projectSorterFactory; + this.sendNotificationJob = new SendClasspathNotificationsJob(logger, conn); logger.log("Instantiating ReusableClasspathListenerHandler"); } - /** - * To keep track of project locations. Without this we can't properly handle deleting events because - * deleted projects (no longer) have a location. So we can only send a proper 'project with this location' - * was deleted' events if we keep track of project locations ourselves. - */ - private Map projectLocations = new HashMap<>(); - - private URI getProjectLocation(IJavaProject jp) { - URI loc = jp.getProject().getLocationURI(); - if (loc!=null) { - return loc; - } else { - //fallback on what we stored ourselves. - return projectLocations.get(jp.getElementName()); - } - } - - private boolean projectExists(IJavaProject jp) { - //We can't really deal with projects that don't exist in disk. So using this more strict 'exists' check - //makes sure anything that looks like it doesn't exist on disk is treated as if it simply doesn't exist - //at all. This kind of addresses a issue caused by Eclipse's idiotic behavior when it comes to deleting - //a project's files from the file system... Eclipse recreates a 'vanilla' project in the workspace, - //simply refusing to accept the fact that the project is actually gone. - if (jp.exists()) { - try { - URI loc = getProjectLocation(jp); - if (loc!=null) { - File f = new File(loc); - return f.isDirectory() && jp.getProject().hasNature(JavaCore.NATURE_ID); - } - } catch (Exception e) { - //Something bogus about this project... so just pretend it doesn't exist. - } - } - return false; - } - - class Subscriptions { - private Set subscribers = null; + private Map subscribers = null; private ClasspathListenerManager classpathListener = null; - public synchronized void subscribe(String callbackCommandId) { - logger.log("subscribing to classpath changes: " + callbackCommandId); + public synchronized void subscribe(String callbackCommandId, boolean isBatched) { + logger.log("subscribing to classpath changes: " + callbackCommandId +" isBatched = "+isBatched); if (subscribers==null) { //First subscriber - subscribers = new HashSet<>(); + subscribers = new HashMap<>(); classpathListener = new ClasspathListenerManager(logger, new ClasspathListener() { @Override public void classpathChanged(IJavaProject jp) { - sendNotification(jp, subscribers); + sendNotification(jp, subscribers.keySet()); } }); } - subscribers.add(callbackCommandId); + subscribers.put(callbackCommandId, isBatched); logger.log("subsribers = " + subscribers); sendInitialEvents(callbackCommandId); } @@ -138,61 +92,13 @@ public class ReusableClasspathListenerHandler { } logger.log("Sending initial event for all projects DONE"); } - + + private void sendNotification(IJavaProject jp, Collection callbackIds) { - //TODO: make one Job to accumulate all requested notification and work more efficiently by batching - // and avoiding multiple executions of duplicated requests. - Job job = new Job("SendClasspath notification") { - @Override - protected IStatus run(IProgressMonitor monitor) { - synchronized (projectLocations) { //Could use some Eclipse job rule. But its really a bit of a PITA to create the right one. - try { - logger.log("Preparing classpath changed notification " + jp.getElementName()); - URI projectLoc = getProjectLocation(jp); - if (projectLoc==null) { - logger.log("Could not send event for project because no project location: "+jp.getElementName()); - } else { - boolean exsits = projectExists(jp); - boolean open = true; // WARNING: calling jp.isOpen is unreliable and subject to race condition. After a POST_CHAGE project open event - // this should be true but it typically is not unless you wait for some time. No idea how you would know - // how long you should wait (200ms is not enough, and that seems pretty long). Isn't it kind of the point - // for a 'POST_CHANGE' event to come **after** model has already changed? I guess not in Eclipse. - // So we will just pretend / assume project is always open. If resolving classpath fails because it is not - // open... so be it (there will be no classpath... this is expected for closed project, so that is fine). - boolean deleted = !(exsits && open); - logger.log("exists = "+exsits +" open = "+open +" => deleted = "+deleted); - String projectName = jp.getElementName(); - - Classpath classpath = Classpath.EMPTY; - if (deleted) { - // projectLocations.remove(projectName); - } else { - projectLocations.put(projectName, projectLoc); - try { - classpath = ClasspathUtil.resolve(jp, logger); - } catch (Exception e) { - logger.log(e); - } - } - for (String callbackCommandId : callbackIds) { - try { - logger.log("executing callback "+callbackCommandId+" "+projectName+" "+deleted+" "+ classpath.getEntries().size()); - Object r = conn.executeClientCommand(callbackCommandId, projectLoc.toString(), projectName, deleted, classpath); - logger.log("executing callback "+callbackCommandId+" SUCCESS ["+r+"]"); - } catch (Exception e) { - logger.log("executing callback "+callbackCommandId+" FAILED"); - logger.log(e); - } - } - } - } catch (Exception e) { - logger.log(e); - } - return Status.OK_STATUS; - } - } - }; - job.schedule(); + for (String callbackId : callbackIds) { + sendNotificationJob.queue.add(new Notification(jp, callbackId)); + } + sendNotificationJob.schedule(); } public synchronized void unsubscribe(String callbackCommandId) { @@ -224,9 +130,14 @@ public class ReusableClasspathListenerHandler { return "ok"; } + @Deprecated public Object addClasspathListener(String callbackCommandId) { - logger.log("ClasspathListenerHandler addClasspathListener " + callbackCommandId); - subscribptions.subscribe(callbackCommandId); + return addClasspathListener(callbackCommandId, false); + } + + public Object addClasspathListener(String callbackCommandId, boolean isBatched) { + logger.log("ClasspathListenerHandler addClasspathListener " + callbackCommandId + "isBatched = "+isBatched); + subscribptions.subscribe(callbackCommandId, isBatched); logger.log("ClasspathListenerHandler addClasspathListener " + callbackCommandId + " => OK"); return "ok"; } diff --git a/headless-services/jdt-ls-extension/org.springframework.tooling.jdt.ls.commons/src/org/springframework/tooling/jdt/ls/commons/classpath/SendClasspathNotificationsJob.java b/headless-services/jdt-ls-extension/org.springframework.tooling.jdt.ls.commons/src/org/springframework/tooling/jdt/ls/commons/classpath/SendClasspathNotificationsJob.java new file mode 100644 index 000000000..c8554162c --- /dev/null +++ b/headless-services/jdt-ls-extension/org.springframework.tooling.jdt.ls.commons/src/org/springframework/tooling/jdt/ls/commons/classpath/SendClasspathNotificationsJob.java @@ -0,0 +1,143 @@ +/******************************************************************************* + * Copyright (c) 2018, 2019 Pivotal, Inc. + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * https://www.eclipse.org/legal/epl-v10.html + * + * Contributors: + * Pivotal, Inc. - initial API and implementation + *******************************************************************************/ +package org.springframework.tooling.jdt.ls.commons.classpath; + +import java.io.File; +import java.net.URI; +import java.util.HashMap; +import java.util.Map; +import java.util.Queue; +import java.util.concurrent.ConcurrentLinkedQueue; + +import org.eclipse.core.runtime.IProgressMonitor; +import org.eclipse.core.runtime.IStatus; +import org.eclipse.core.runtime.Status; +import org.eclipse.core.runtime.jobs.Job; +import org.eclipse.jdt.core.IJavaProject; +import org.eclipse.jdt.core.JavaCore; +import org.springframework.ide.vscode.commons.protocol.java.Classpath; +import org.springframework.tooling.jdt.ls.commons.Logger; + +public class SendClasspathNotificationsJob extends Job { + + private final ClientCommandExecutor conn; + private final Logger logger; + + public SendClasspathNotificationsJob(Logger logger, ClientCommandExecutor conn) { + super("Send Classpath Notifications"); + this.logger = logger; + this.conn = conn; + } + + public static class Notification { + IJavaProject jp; + String callbackId; + + public Notification(IJavaProject jp, String callbackId) { + this.jp = jp; + this.callbackId = callbackId; + } + } + + /** + * To keep track of project locations. Without this we can't properly handle deletion events because + * deleted projects no longer have a location. So we can only send a proper 'project with this location' + * was deleted' event if we keep track of project locations ourselves. + */ + private Map projectLocations = new HashMap<>(); + + private URI getProjectLocation(IJavaProject jp) { + URI loc = jp.getProject().getLocationURI(); + if (loc!=null) { + return loc; + } else { + synchronized (projectLocations) { + //fallback on what we stored ourselves. + return projectLocations.get(jp.getElementName()); + } + } + } + + public final Queue queue = new ConcurrentLinkedQueue<>(); + + private boolean projectExists(IJavaProject jp) { + //We can't really deal with projects that don't exist in disk. So using this more strict 'exists' check + //makes sure anything that looks like it doesn't exist on disk is treated as if it simply doesn't exist + //at all. This kind of addresses a issue caused by Eclipse's idiotic behavior when it comes to deleting + //a project's files from the file system... Eclipse recreates a 'vanilla' project in the workspace, + //simply refusing to accept the fact that the project is actually gone. + if (jp.exists()) { + try { + URI loc = getProjectLocation(jp); + if (loc!=null) { + File f = new File(loc); + return f.isDirectory() && jp.getProject().hasNature(JavaCore.NATURE_ID); + } + } catch (Exception e) { + //Something bogus about this project... so just pretend it doesn't exist. + } + } + return false; + } + + + @Override + protected IStatus run(IProgressMonitor monitor) { + synchronized (projectLocations) { //Could use some Eclipse job rule. But its really a bit of a PITA to create the right one. + try { + for (Notification notification = queue.poll(); notification!=null; notification = queue.poll()) { + IJavaProject jp = notification.jp; + logger.log("Preparing classpath changed notification " + jp.getElementName()); + URI projectLoc = getProjectLocation(jp); + if (projectLoc==null) { + logger.log("Could not send event for project because no project location: "+jp.getElementName()); + } else { + boolean exsits = projectExists(jp); + boolean open = true; // WARNING: calling jp.isOpen is unreliable and subject to race condition. After a POST_CHAGE project open event + // this should be true but it typically is not unless you wait for some time. No idea how you would know + // how long you should wait (200ms is not enough, and that seems pretty long). + // So we will just pretend / assume project is always open. If resolving classpath fails because it is not + // open... so be it (there will be no classpath... this is expected for closed project, so that is fine). + boolean deleted = !(exsits && open); + logger.log("exists = "+exsits +" open = "+open +" => deleted = "+deleted); + String projectName = jp.getElementName(); + + Classpath classpath = Classpath.EMPTY; + if (deleted) { + // projectLocations.remove(projectName); + } else { + projectLocations.put(projectName, projectLoc); + try { + classpath = ClasspathUtil.resolve(jp, logger); + } catch (Exception e) { + logger.log(e); + } + } + String callbackCommandId = notification.callbackId; + { + try { + logger.log("executing callback "+callbackCommandId+" "+projectName+" "+deleted+" "+ classpath.getEntries().size()); + Object r = conn.executeClientCommand(callbackCommandId, projectLoc.toString(), projectName, deleted, classpath); + logger.log("executing callback "+callbackCommandId+" SUCCESS ["+r+"]"); + } catch (Exception e) { + logger.log("executing callback "+callbackCommandId+" FAILED"); + logger.log(e); + } + } + } + } + } catch (Exception e) { + logger.log(e); + } + return Status.OK_STATUS; + } + } +} diff --git a/headless-services/jdt-ls-extension/org.springframework.tooling.jdt.ls.extension/src/org/springframework/tooling/jdt/ls/extension/ClasspathListenerHandler.java b/headless-services/jdt-ls-extension/org.springframework.tooling.jdt.ls.extension/src/org/springframework/tooling/jdt/ls/extension/ClasspathListenerHandler.java index 07c3c21a1..b48c04593 100644 --- a/headless-services/jdt-ls-extension/org.springframework.tooling.jdt.ls.extension/src/org/springframework/tooling/jdt/ls/extension/ClasspathListenerHandler.java +++ b/headless-services/jdt-ls-extension/org.springframework.tooling.jdt.ls.extension/src/org/springframework/tooling/jdt/ls/extension/ClasspathListenerHandler.java @@ -48,7 +48,11 @@ public class ClasspathListenerHandler implements IDelegateCommandHandler { logger.log("ClasspathListenerHandler executeCommand " + commandId + ", " + arguments); switch (commandId) { case "sts.java.addClasspathListener": - return addClasspathListener((String) arguments.get(0)); + boolean isBatched = false; + if (arguments.size()>=2) { + isBatched = (Boolean)arguments.get(1); + } + return addClasspathListener((String) arguments.get(0), isBatched); case "sts.java.removeClasspathListener": return removeClasspathListener((String) arguments.get(0)); default: @@ -61,9 +65,9 @@ public class ClasspathListenerHandler implements IDelegateCommandHandler { return handlerImpl.removeClasspathListener(callbackCommandId); } - private Object addClasspathListener(String callbackCommandId) { + private Object addClasspathListener(String callbackCommandId, boolean isBatched) { logger.log("ClasspathListenerHandler addClasspathListener " + callbackCommandId); - handlerImpl.addClasspathListener(callbackCommandId); + handlerImpl.addClasspathListener(callbackCommandId, isBatched); logger.log("ClasspathListenerHandler addClasspathListener " + callbackCommandId + " => OK"); return "ok"; }