Improve marker resolution performance

by not connecting the file where we retrieve marker information. That is an unnecessary step which can be expensive (for example the Xtext language server rebuilds the source) and causes timeouts on checkMarkerResoultion
eclipse-lsp4e · Jun 23, 2022 · 432f259 · 432f259
1 parent d942d41
commit 432f259
Show file tree

Hide file tree

Showing 7 changed files with 59 additions and 31 deletions.
diff --git a/org.eclipse.lsp4e.test/META-INF/MANIFEST.MF b/org.eclipse.lsp4e.test/META-INF/MANIFEST.MF
@@ -2,7 +2,7 @@ Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-Name: Tests for language server bundle (Incubation)
 Bundle-SymbolicName: org.eclipse.lsp4e.test;singleton:=true
-Bundle-Version: 0.13.12.qualifier
+Bundle-Version: 0.13.13.qualifier
 Fragment-Host: org.eclipse.lsp4e
 Bundle-Vendor: Eclipse.org
 Bundle-RequiredExecutionEnvironment: JavaSE-11

diff --git a/org.eclipse.lsp4e.test/pom.xml b/org.eclipse.lsp4e.test/pom.xml
@@ -8,7 +8,7 @@
 	</parent>
 	<artifactId>org.eclipse.lsp4e.test</artifactId>
 	<packaging>eclipse-test-plugin</packaging>
-	<version>0.13.12-SNAPSHOT</version>
+	<version>0.13.13-SNAPSHOT</version>
 
 	<properties>
 		<os-jvm-flags /> <!-- for the default case -->

diff --git a/org.eclipse.lsp4e.test/src/org/eclipse/lsp4e/test/LanguageServiceAccessorTest.java b/org.eclipse.lsp4e.test/src/org/eclipse/lsp4e/test/LanguageServiceAccessorTest.java
@@ -101,7 +101,7 @@ public void testGetLSPDocumentInfoForInvalidTextEditor() throws CoreException, I
 	public void testGetLanguageServerInvalidFile() throws Exception {
 		IFile testFile = TestUtils.createFile(project, "not_associated_with_ls.abc", "");
 		Collection<LanguageServer> servers = new ArrayList<>();
-		for (CompletableFuture<LanguageServer> future :LanguageServiceAccessor.getInitializedLanguageServers(testFile, capabilites -> Boolean.TRUE)){
+		for (CompletableFuture<LanguageServer> future :LanguageServiceAccessor.getInitializedLanguageServers(testFile, capabilites -> Boolean.TRUE, false)){
 			servers.add(future.get(1, TimeUnit.SECONDS));
 		}
 		assertTrue(servers.isEmpty());
@@ -111,7 +111,7 @@ public void testGetLanguageServerInvalidFile() throws Exception {
 	public void testLSAsExtension() throws Exception {
 		IFile testFile = TestUtils.createFile(project, "shouldUseExtension.lspt", "");
 		LanguageServer info = LanguageServiceAccessor
-				.getInitializedLanguageServers(testFile, capabilites -> Boolean.TRUE).iterator().next()
+				.getInitializedLanguageServers(testFile, capabilites -> Boolean.TRUE, false).iterator().next()
 				.get(1, TimeUnit.SECONDS);
 		assertNotNull(info);
 	}
@@ -120,7 +120,7 @@ public void testLSAsExtension() throws Exception {
 	public void testLSAsRunConfiguration() throws Exception {
 		IFile testFile = TestUtils.createFile(project, "shouldUseRunConfiguration.lspt2", "");
 		LanguageServer info = LanguageServiceAccessor
-				.getInitializedLanguageServers(testFile, capabilites -> Boolean.TRUE).iterator().next()
+				.getInitializedLanguageServers(testFile, capabilites -> Boolean.TRUE, false).iterator().next()
 				.get(1, TimeUnit.SECONDS);
 		assertNotNull(info);
 	}
@@ -154,14 +154,14 @@ public void testReuseSameLSforMultiContentType() throws Exception {
 		// wrap in HashSet as a workaround of https://github.com/eclipse/lsp4j/issues/106
 		Collection<LanguageServer> file1LanguageServers = new ArrayList<>();
 		for (CompletableFuture<LanguageServer> future : LanguageServiceAccessor.getInitializedLanguageServers(testFile1,
-				capabilites -> Boolean.TRUE)) {
+				capabilites -> Boolean.TRUE, false)) {
 			file1LanguageServers.add(future.get(1, TimeUnit.SECONDS));
 		}
 		assertEquals(1, file1LanguageServers.size());
 		LanguageServer file1LS = file1LanguageServers.iterator().next();
 		Collection<LanguageServer> file2LanguageServers = new ArrayList<>();
 		for (CompletableFuture<LanguageServer> future : LanguageServiceAccessor.getInitializedLanguageServers(testFile2,
-				capabilites -> Boolean.TRUE)) {
+				capabilites -> Boolean.TRUE, false)) {
 			file2LanguageServers.add(future.get(1, TimeUnit.SECONDS));
 		}
 		assertEquals(2, file2LanguageServers.size());
@@ -179,9 +179,9 @@ public void testGetOnlyRunningLanguageServers() throws Exception {
 		IEditorPart editor1 = TestUtils.openEditor(testFile1);
 		IEditorPart editor2 = TestUtils.openEditor(testFile2);
 
-		LanguageServiceAccessor.getInitializedLanguageServers(testFile1, capabilities -> Boolean.TRUE).iterator()
+		LanguageServiceAccessor.getInitializedLanguageServers(testFile1, capabilities -> Boolean.TRUE, false).iterator()
 				.next();
-		LanguageServiceAccessor.getInitializedLanguageServers(testFile2, capabilities -> Boolean.TRUE).iterator()
+		LanguageServiceAccessor.getInitializedLanguageServers(testFile2, capabilities -> Boolean.TRUE, false).iterator()
 				.next();
 
 		List<LanguageServer> runningServers = LanguageServiceAccessor.getActiveLanguageServers(capabilities -> Boolean.TRUE);
@@ -195,7 +195,7 @@ public void testGetOnlyRunningLanguageServers() throws Exception {
 		assertEquals(0, LanguageServiceAccessor.getActiveLanguageServers(capabilities -> Boolean.TRUE).size());
 
 		editor1 = TestUtils.openEditor(testFile1);
-		LanguageServiceAccessor.getInitializedLanguageServers(testFile1, capabilities -> Boolean.TRUE).iterator()
+		LanguageServiceAccessor.getInitializedLanguageServers(testFile1, capabilities -> Boolean.TRUE, false).iterator()
 				.next();
 
 		new LSDisplayHelper(() -> LanguageServiceAccessor.getActiveLanguageServers(capabilities -> Boolean.TRUE).size() == 1)
@@ -208,7 +208,7 @@ public void testCreateNewLSAfterInitialProjectGotDeleted() throws Exception {
 		IFile testFile1 = TestUtils.createUniqueTestFile(project, "");
 
 		TestUtils.openEditor(testFile1);
-		LanguageServiceAccessor.getInitializedLanguageServers(testFile1, capabilities -> Boolean.TRUE).iterator()
+		LanguageServiceAccessor.getInitializedLanguageServers(testFile1, capabilities -> Boolean.TRUE, false).iterator()
 				.next();
 		new LSDisplayHelper(() -> MockLanguageServer.INSTANCE.isRunning()).waitForCondition(Display.getCurrent(), 5000,
 				300);
@@ -228,7 +228,7 @@ public void testCreateNewLSAfterInitialProjectGotDeleted() throws Exception {
 		IFile testFile2 = TestUtils.createUniqueTestFile(project, "");
 
 		TestUtils.openEditor(testFile2);
-		LanguageServiceAccessor.getInitializedLanguageServers(testFile2, capabilities -> Boolean.TRUE).iterator()
+		LanguageServiceAccessor.getInitializedLanguageServers(testFile2, capabilities -> Boolean.TRUE, false).iterator()
 				.next();
 		new LSDisplayHelper(() -> MockLanguageServer.INSTANCE.isRunning()).waitForCondition(Display.getCurrent(), 5000,
 				300);
@@ -245,7 +245,7 @@ public void testReuseMultirootFolderLSAfterInitialProjectGotDeleted() throws Exc
 		IFile testFile1 = TestUtils.createUniqueTestFile(project, "lsptWithMultiRoot", "");
 
 		TestUtils.openEditor(testFile1);
-		LanguageServiceAccessor.getInitializedLanguageServers(testFile1, capabilities -> Boolean.TRUE).iterator()
+		LanguageServiceAccessor.getInitializedLanguageServers(testFile1, capabilities -> Boolean.TRUE, false).iterator()
 				.next();
 		new LSDisplayHelper(() -> MockLanguageServerMultiRootFolders.INSTANCE.isRunning())
 				.waitForCondition(Display.getCurrent(), 5000, 300);
@@ -265,7 +265,7 @@ public void testReuseMultirootFolderLSAfterInitialProjectGotDeleted() throws Exc
 		IFile testFile2 = TestUtils.createUniqueTestFile(project, "lsptWithMultiRoot", "");
 
 		TestUtils.openEditor(testFile2);
-		LanguageServiceAccessor.getInitializedLanguageServers(testFile2, capabilities -> Boolean.TRUE).iterator()
+		LanguageServiceAccessor.getInitializedLanguageServers(testFile2, capabilities -> Boolean.TRUE, false).iterator()
 				.next();
 		new LSDisplayHelper(() -> MockLanguageServerMultiRootFolders.INSTANCE.isRunning())
 				.waitForCondition(Display.getCurrent(), 5000, 300);

diff --git a/org.eclipse.lsp4e.test/src/org/eclipse/lsp4e/test/RunningLanguageServerTest.java b/org.eclipse.lsp4e.test/src/org/eclipse/lsp4e/test/RunningLanguageServerTest.java
@@ -58,7 +58,7 @@ public void testOpenCloseLanguageServer() throws Exception {
 		// open and close the editor several times
 		for(int i = 0; i < 10; i++) {
 			IEditorPart editor = TestUtils.openEditor(testFile);
-			LanguageServiceAccessor.getInitializedLanguageServers(testFile, capabilities -> Boolean.TRUE).iterator()
+			LanguageServiceAccessor.getInitializedLanguageServers(testFile, capabilities -> Boolean.TRUE, false).iterator()
 					.next();
 			assertTrue("language server is started for iteration #" + i,
 					new LSDisplayHelper(() -> MockLanguageServer.INSTANCE.isRunning()).waitForCondition(display, 5000));
@@ -80,7 +80,7 @@ public void testDisabledLanguageServer() throws Exception {
 
 		TestUtils.openEditor(testFile);
 		List<CompletableFuture<LanguageServer>> initializedLanguageServers = LanguageServiceAccessor
-				.getInitializedLanguageServers(testFile, capabilities -> Boolean.TRUE);
+				.getInitializedLanguageServers(testFile, capabilities -> Boolean.TRUE, false);
 		assertNotNull(initializedLanguageServers);
 		assertEquals("language server should not be started because it is disabled", 0,
 				initializedLanguageServers.size());

diff --git a/org.eclipse.lsp4e.test/src/org/eclipse/lsp4e/test/WorkspaceFoldersTest.java b/org.eclipse.lsp4e.test/src/org/eclipse/lsp4e/test/WorkspaceFoldersTest.java
@@ -57,7 +57,7 @@ public void testRecycleLSAfterInitialProjectGotDeletedIfWorkspaceFolders() throw
 		IFile testFile1 = TestUtils.createUniqueTestFile(project, "");
 
 		TestUtils.openEditor(testFile1);
-		LanguageServiceAccessor.getInitializedLanguageServers(testFile1, capabilities -> Boolean.TRUE).iterator()
+		LanguageServiceAccessor.getInitializedLanguageServers(testFile1, capabilities -> Boolean.TRUE, false).iterator()
 				.next();
 		new LSDisplayHelper(() -> MockLanguageServer.INSTANCE.isRunning()).waitForCondition(Display.getCurrent(), 5000,
 				300);
@@ -77,7 +77,7 @@ public void testRecycleLSAfterInitialProjectGotDeletedIfWorkspaceFolders() throw
 		IFile testFile2 = TestUtils.createUniqueTestFile(project, "");
 
 		TestUtils.openEditor(testFile2);
-		LanguageServiceAccessor.getInitializedLanguageServers(testFile2, capabilities -> Boolean.TRUE).iterator()
+		LanguageServiceAccessor.getInitializedLanguageServers(testFile2, capabilities -> Boolean.TRUE, false).iterator()
 				.next();
 		new LSDisplayHelper(() -> MockLanguageServer.INSTANCE.isRunning()).waitForCondition(Display.getCurrent(), 5000,
 				300);
@@ -96,7 +96,7 @@ public void testPojectCreate() throws Exception {
 		IFile testFile1 = TestUtils.createUniqueTestFile(project, "");
 
 		TestUtils.openEditor(testFile1);
-		LanguageServiceAccessor.getInitializedLanguageServers(testFile1, capabilities -> Boolean.TRUE).iterator()
+		LanguageServiceAccessor.getInitializedLanguageServers(testFile1, capabilities -> Boolean.TRUE, false).iterator()
 				.next();
 		new LSDisplayHelper(() -> MockLanguageServer.INSTANCE.isRunning()).waitForCondition(Display.getCurrent(), 5000,
 				300);
@@ -123,7 +123,7 @@ public void testProjectClose() throws Exception {
 		IFile testFile1 = TestUtils.createUniqueTestFile(project, "");
 
 		TestUtils.openEditor(testFile1);
-		LanguageServiceAccessor.getInitializedLanguageServers(testFile1, capabilities -> Boolean.TRUE).iterator()
+		LanguageServiceAccessor.getInitializedLanguageServers(testFile1, capabilities -> Boolean.TRUE, false).iterator()
 				.next();
 		new LSDisplayHelper(() -> MockLanguageServer.INSTANCE.isRunning()).waitForCondition(Display.getCurrent(), 5000,
 				300);
@@ -147,7 +147,7 @@ public void testProjectDelete() throws Exception {
 		IFile testFile1 = TestUtils.createUniqueTestFile(project, "");
 
 		TestUtils.openEditor(testFile1);
-		LanguageServiceAccessor.getInitializedLanguageServers(testFile1, capabilities -> Boolean.TRUE).iterator()
+		LanguageServiceAccessor.getInitializedLanguageServers(testFile1, capabilities -> Boolean.TRUE, false).iterator()
 				.next();
 		new LSDisplayHelper(() -> MockLanguageServer.INSTANCE.isRunning()).waitForCondition(Display.getCurrent(), 5000,
 				300);
@@ -176,7 +176,7 @@ public void projectReopenTest() throws Exception {
 		IFile testFile1 = TestUtils.createUniqueTestFile(project, "");
 
 		TestUtils.openEditor(testFile1);
-		LanguageServiceAccessor.getInitializedLanguageServers(testFile1, capabilities -> Boolean.TRUE).iterator()
+		LanguageServiceAccessor.getInitializedLanguageServers(testFile1, capabilities -> Boolean.TRUE, false).iterator()
 				.next();
 		new LSDisplayHelper(() -> MockLanguageServer.INSTANCE.isRunning()).waitForCondition(Display.getCurrent(), 5000,
 				300);

diff --git a/org.eclipse.lsp4e/src/org/eclipse/lsp4e/LanguageServiceAccessor.java b/org.eclipse.lsp4e/src/org/eclipse/lsp4e/LanguageServiceAccessor.java
@@ -142,15 +142,23 @@ public boolean isActive() {
 	}
 
 
+	@Deprecated(forRemoval = true)
 	public static @NonNull List<CompletableFuture<LanguageServer>> getInitializedLanguageServers(@NonNull IFile file,
 			@Nullable Predicate<ServerCapabilities> request) throws IOException {
+			return getInitializedLanguageServers(file, request, true);
+	}
+
+	public static @NonNull List<CompletableFuture<LanguageServer>> getInitializedLanguageServers(@NonNull IFile file,
+			@Nullable Predicate<ServerCapabilities> request, boolean forceConnectFile) throws IOException {
 		synchronized (startedServers) {
 			Collection<LanguageServerWrapper> wrappers = getLSWrappers(file, request);
 			return wrappers.stream().map(wrapper -> wrapper.getInitializedServer().thenApplyAsync(server -> {
-				try {
-					wrapper.connect(file, null);
-				} catch (IOException e) {
-					LanguageServerPlugin.logError(e);
+				if (forceConnectFile) {
+					try {
+						wrapper.connect(file, null);
+					} catch (IOException e) {
+						LanguageServerPlugin.logError(e);
+					}
 				}
 				return server;
 			})).collect(Collectors.toList());
@@ -184,7 +192,7 @@ public static void enableLanguageServerContentType(
 							&& contentType.equals(contentDesc.getContentType())
 							&& lsDefinition != null) {
 						try {
-							getInitializedLanguageServer(editorFile, lsDefinition, capabilities -> true);
+							getInitializedLanguageServer(editorFile, lsDefinition, capabilities -> true, false);
 						} catch (IOException e) {
 							LanguageServerPlugin.logError(e);
 						}
@@ -203,7 +211,7 @@ public static void enableLanguageServerContentType(
 	 * @param lsDefinition
 	 * @param capabilitiesPredicate a predicate to check capabilities
 	 * @return a LanguageServer for the given file, which is defined with provided server ID and conforms to specified request
-	 * @deprecated use {@link #getInitializedLanguageServer(IFile, LanguageServerDefinition, Predicate)} instead.
+	 * @deprecated use {@link #getInitializedLanguageServer(IFile, LanguageServerDefinition, Predicate, boolean)} instead.
 	 */
 	@Deprecated
 	public static LanguageServer getLanguageServer(@NonNull IFile file, @NonNull LanguageServerDefinition lsDefinition,
@@ -225,13 +233,33 @@ public static LanguageServer getLanguageServer(@NonNull IFile file, @NonNull Lan
 	 * @return a LanguageServer for the given file, which is defined with provided server ID and conforms to specified request.
 	 *  If {@code capabilitesPredicate} does not test positive for the server's capabilities, {@code null} is returned.
 	 */
+	@Deprecated(forRemoval = true)
 	public static CompletableFuture<LanguageServer> getInitializedLanguageServer(@NonNull IFile file,
 			@NonNull LanguageServerDefinition lsDefinition,
 			Predicate<ServerCapabilities> capabilitiesPredicate)
 			throws IOException {
+		return getInitializedLanguageServer(file, lsDefinition, capabilitiesPredicate, true);
+	}
+
+	/**
+	 * Get the requested language server instance for the given file. Starts the language server if not already started.
+	 * @param file
+	 * @param lsDefinition
+	 * @param capabilitiesPredicate a predicate to check capabilities
+	 * @param forceConnectFile if the file should be connected
+	 * @return a LanguageServer for the given file, which is defined with provided server ID and conforms to specified request.
+	 *  If {@code capabilitesPredicate} does not test positive for the server's capabilities, {@code null} is returned.
+	 */
+	public static CompletableFuture<LanguageServer> getInitializedLanguageServer(@NonNull IFile file,
+			@NonNull LanguageServerDefinition lsDefinition,
+			Predicate<ServerCapabilities> capabilitiesPredicate,
+			boolean forceConnectFile)
+			throws IOException {
 		LanguageServerWrapper wrapper = getLSWrapper(file.getProject(), lsDefinition, file.getFullPath());
 		if (capabilitiesComply(wrapper, capabilitiesPredicate)) {
-			wrapper.connect(file, null);
+			if (forceConnectFile) {
+				wrapper.connect(file, null);
+			}
 			return wrapper.getInitializedServer();
 		}
 		return null;

diff --git a/...pse.lsp4e/src/org/eclipse/lsp4e/operations/codeactions/LSPCodeActionMarkerResolution.java b/...pse.lsp4e/src/org/eclipse/lsp4e/operations/codeactions/LSPCodeActionMarkerResolution.java
@@ -155,7 +155,7 @@ private List<CompletableFuture<LanguageServer>> getLanguageServerFutures(@NonNul
 				CompletableFuture<LanguageServer> serverFuture = LanguageServiceAccessor
 						.getInitializedLanguageServer(file, definition,
 								serverCapabilities -> serverCapabilities == null
-										|| providesCodeActions(serverCapabilities));
+										|| providesCodeActions(serverCapabilities), false);
 				if (serverFuture != null) {
 					languageServerFutures.add(serverFuture);
 				}
@@ -174,7 +174,7 @@ private List<CompletableFuture<LanguageServer>> getLanguageServerFutures(@NonNul
 							return true;
 						}
 						return false;
-					}));
+					}, false));
 		}
 		return languageServerFutures;
 	}