Skip to content

Commit

Permalink
fix: Too many non-blocking read actions submitted at once in
Browse files Browse the repository at this point in the history 
LSPDiagnosticHandler

Fixes #1089

Signed-off-by: azerr <[email protected]>
  • Loading branch information
@angelozerr @fbricon
angelozerr authored and fbricon committed Aug 11, 2023
1 parent 267665c commit 34b16af
Show file tree
Hide file tree
Showing 8 changed files with 276 additions and 146 deletions.
44 changes: 44 additions & 0 deletions src/main/java/com/redhat/devtools/intellij/lsp4ij/client/CoalesceByKey.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/*******************************************************************************
* Copyright (c) 2023 Red Hat, Inc.
* Distributed under license by Red Hat, Inc. All rights reserved.
* This program is made available under the terms of the
* Eclipse Public License v2.0 which accompanies this distribution,
* and is available at https://www.eclipse.org/legal/epl-v20.html
*
* Contributors:
* Red Hat, Inc. - initial API and implementation
******************************************************************************/
package com.redhat.devtools.intellij.lsp4ij.client;

import java.util.Arrays;
import java.util.Objects;

/**
* A CoalesceBy key which defines a type (like LSP request) and a value which should be identified the request.
*/
public class CoalesceByKey {

private final String type;

private final Object[] value;

public CoalesceByKey(String type, Object... value) {
this.type = type;
this.value = value;
}

@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
CoalesceByKey that = (CoalesceByKey) o;
return Objects.equals(type, that.type) && Arrays.equals(value, that.value);
}

@Override
public int hashCode() {
int result = Objects.hash(type);
result = 31 * result + Arrays.hashCode(value);
return result;
}
}
14 changes: 13 additions & 1 deletion src/main/java/com/redhat/devtools/intellij/lsp4ij/client/IndexAwareLanguageClient.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,19 @@ public IndexAwareLanguageClient(Project project) {
* @return the output of the function code
*/
protected <R> CompletableFuture<R> runAsBackground(String progressTitle, Function<ProgressIndicator, R> code) {
return new LSPCompletableFuture<>(code, progressTitle, IndexAwareLanguageClient.this);
return runAsBackground(progressTitle, code, null);
}

/**
* Run the given function as a background task, wrapped in a read action
*
* @param progressTitle the progress title of the action being run
* @param code the function code to execute in the background
* @param <R> the return type
* @return the output of the function code
*/
protected <R> CompletableFuture<R> runAsBackground(String progressTitle, Function<ProgressIndicator, R> code, Object coalesceBy) {
return new LSPCompletableFuture<>(code, progressTitle, IndexAwareLanguageClient.this, coalesceBy);
}

/**
Expand Down
8 changes: 7 additions & 1 deletion src/main/java/com/redhat/devtools/intellij/lsp4ij/client/LSPCompletableFuture.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,12 +54,15 @@ public ResultOrError(R result, Exception error) {
private final IndexAwareLanguageClient languageClient;
private final String progressTitle;
private final AtomicInteger nbAttempt;

private final Object coalesceBy;
private CancellablePromise<ResultOrError<R>> nonBlockingReadActionPromise;

public LSPCompletableFuture(Function<ProgressIndicator, R> code, String progressTitle, IndexAwareLanguageClient languageClient) {
public LSPCompletableFuture(Function<ProgressIndicator, R> code, String progressTitle, IndexAwareLanguageClient languageClient, Object coalesceBy) {
this.code = code;
this.progressTitle = progressTitle;
this.languageClient = languageClient;
this.coalesceBy = coalesceBy;
this.nbAttempt = new AtomicInteger(0);
// if indexation is processing, we need to execute the promise in smart mode
var executeInSmartMode = DumbService.getInstance(languageClient.getProject()).isDumb();
Expand Down Expand Up @@ -136,6 +139,9 @@ private CancellablePromise<ResultOrError<R>> nonBlockingReadActionPromise(boolea
if (executeInSmartMode) {
action = action.inSmartMode(project);
}
if (coalesceBy != null) {
action = action.coalesceBy(coalesceBy);
}
return action
.submit(AppExecutorUtil.getAppExecutorService());
}
Expand Down
19 changes: 16 additions & 3 deletions ...java/com/redhat/devtools/intellij/lsp4ij/operations/diagnostics/LSPDiagnosticHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import com.intellij.openapi.application.ApplicationManager;
import com.intellij.openapi.application.ReadAction;
import com.intellij.openapi.module.Module;
import com.intellij.openapi.project.DumbService;
import com.intellij.openapi.project.Project;
import com.intellij.openapi.vfs.VirtualFile;
import com.intellij.psi.PsiFile;
Expand All @@ -25,6 +26,7 @@
import com.redhat.devtools.intellij.lsp4ij.LSPIJUtils;
import com.redhat.devtools.intellij.lsp4ij.LSPVirtualFileWrapper;
import com.redhat.devtools.intellij.lsp4ij.LanguageServerWrapper;
import com.redhat.devtools.intellij.lsp4ij.client.CoalesceByKey;
import org.eclipse.lsp4j.PublishDiagnosticsParams;

import java.util.function.Consumer;
Expand All @@ -45,12 +47,23 @@ public LSPDiagnosticHandler(LanguageServerWrapper languageServerWrapper) {

@Override
public void accept(PublishDiagnosticsParams params) {
Project project = languageServerWrapper.getProject();
if(project.isDisposed()) {
return;
}
if (ApplicationManager.getApplication().isReadAccessAllowed()) {
updateDiagnostics(params);
} else {
ReadAction.nonBlocking(() -> updateDiagnostics(params))
.submit(AppExecutorUtil.getAppExecutorService());

// Cancel if needed the previous "textDocument/publishDiagnostics" for a given uri.
var coalesceBy = new CoalesceByKey("textDocument/publishDiagnostics", params.getUri());
var executeInSmartMode = DumbService.getInstance(languageServerWrapper.getProject()).isDumb();
var action = ReadAction.nonBlocking(() -> updateDiagnostics(params))
.expireWith(languageServerWrapper)
.coalesceBy(coalesceBy);
if (executeInSmartMode) {
action.inSmartMode(project);
}
action.submit(AppExecutorUtil.getAppExecutorService());
}
}

Expand Down
26 changes: 21 additions & 5 deletions ...va/com/redhat/devtools/intellij/lsp4mp4ij/classpath/ClasspathResourceChangedNotifier.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
/**
* Source file change notifier with a debounce mode.
*/
public class ClasspathResourceChangedNotifier implements Disposable {
public class ClasspathResourceChangedNotifier implements Disposable {

private static final long DEBOUNCE_DELAY = 1000;

Expand All @@ -46,7 +46,8 @@ public class ClasspathResourceChangedNotifier implements Disposable {

private final Set<Pair<VirtualFile, Module>> sourceFiles;
private boolean librariesChanged;
private final List<RunnableProgress> processBeforeLibrariesChanged;
private final List<RunnableProgress> processBeforeLibrariesChanged;
private boolean disposed;

public ClasspathResourceChangedNotifier(Project project, List<RunnableProgress> preprocessors) {
this.project = project;
Expand Down Expand Up @@ -74,6 +75,9 @@ public synchronized void addSourceFile(Pair<VirtualFile, Module> pair) {
}

private void asyncNotifyChanges() {
if (isDisposed()) {
return;
}
if (ApplicationManager.getApplication().isUnitTestMode()) {
notifyChanges();
} else {
Expand All @@ -93,6 +97,9 @@ public void run() {
}

private void notifyChanges() {
if (isDisposed()) {
return;
}
synchronized (sourceFiles) {
// Java, config sources files has changed
project.getMessageBus().syncPublisher(ClasspathResourceChangedManager.TOPIC).sourceFilesChanged(sourceFiles);
Expand Down Expand Up @@ -121,8 +128,7 @@ public void run(@NotNull ProgressIndicator progressIndicator) {
for (var runnable : processBeforeLibrariesChanged) {
runnable.run(progressIndicator);
}
}
finally {
} finally {
// Send the libraries changed event
project.getMessageBus().syncPublisher(ClasspathResourceChangedManager.TOPIC).librariesChanged();
librariesChanged = false;
Expand All @@ -134,13 +140,23 @@ public void run(@NotNull ProgressIndicator progressIndicator) {
}
}

public boolean isDisposed() {
return disposed;
}

@Override
public void dispose() {
if (isDisposed()) {
return;
}
this.disposed = true;
if (debounceTask != null) {
debounceTask.cancel();
debounceTask =null;
}
if(debounceTimer != null) {
if (debounceTimer != null) {
debounceTimer.cancel();
debounceTimer = null;
}
}
}
5 changes: 5 additions & 0 deletions .../redhat/devtools/intellij/lsp4mp4ij/psi/internal/jaxrs/java/DefaultJaxRsInfoProvider.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@
package com.redhat.devtools.intellij.lsp4mp4ij.psi.internal.jaxrs.java;

import com.intellij.openapi.module.Module;
import com.intellij.openapi.progress.ProcessCanceledException;
import com.intellij.openapi.progress.ProgressIndicator;
import com.intellij.openapi.project.IndexNotReadyException;
import com.intellij.psi.*;
import com.intellij.psi.util.PsiTreeUtil;
import com.redhat.devtools.intellij.lsp4ij.LSPIJUtils;
Expand All @@ -28,6 +30,7 @@
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.concurrent.CancellationException;
import java.util.logging.Level;
import java.util.logging.Logger;

Expand Down Expand Up @@ -64,6 +67,8 @@ public List<JaxRsMethodInfo> getJaxRsMethodInfo(PsiFile typeRoot, JaxRsContext j
List<JaxRsMethodInfo> methodInfos = new ArrayList<>();
try {
collectJaxRsMethodInfo(typeRoot.getChildren(), null, methodInfos, jaxrsContext, utils, monitor);
} catch (IndexNotReadyException | ProcessCanceledException | CancellationException e) {
throw e;
} catch (Exception e) {
LOGGER.log(Level.SEVERE, "while collecting JAX-RS method info using the default method", e);
}
Expand Down
42 changes: 30 additions & 12 deletions src/main/java/com/redhat/devtools/intellij/quarkus/lsp/QuarkusLanguageClient.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import com.intellij.openapi.util.Pair;
import com.intellij.openapi.vfs.VirtualFile;
import com.intellij.util.messages.MessageBusConnection;
import com.redhat.devtools.intellij.lsp4ij.client.CoalesceByKey;
import com.redhat.devtools.intellij.lsp4mp4ij.psi.core.ProjectLabelManager;
import com.redhat.devtools.intellij.lsp4mp4ij.psi.core.PropertiesManager;
import com.redhat.devtools.intellij.lsp4mp4ij.psi.core.PropertiesManagerForJava;
Expand Down Expand Up @@ -112,65 +113,81 @@ private boolean isConfigSource(VirtualFile file) {

@Override
public CompletableFuture<MicroProfileProjectInfo> getProjectInfo(MicroProfileProjectInfoParams params) {
var coalesceBy = new CoalesceByKey("microprofile/projectInfo", params.getUri(), params.getScopes());
String filePath = getFilePath(params.getUri());
return runAsBackground("Computing MicroProfile properties for '" + filePath + "'.", monitor ->
PropertiesManager.getInstance().getMicroProfileProjectInfo(params, PsiUtilsLSImpl.getInstance(getProject()), monitor)
);
, coalesceBy);
}

@Override
public CompletableFuture<Hover> getJavaHover(MicroProfileJavaHoverParams javaParams) {
return runAsBackground("Computing MicroProfile Java hover", monitor -> PropertiesManagerForJava.getInstance().hover(javaParams, PsiUtilsLSImpl.getInstance(getProject())));
var coalesceBy = new CoalesceByKey("microprofile/java/hover", javaParams.getUri(), javaParams.getPosition());
return runAsBackground("Computing MicroProfile Java hover", monitor -> PropertiesManagerForJava.getInstance().hover(javaParams, PsiUtilsLSImpl.getInstance(getProject())), coalesceBy);
}

@Override
public CompletableFuture<List<PublishDiagnosticsParams>> getJavaDiagnostics(MicroProfileJavaDiagnosticsParams javaParams) {
return runAsBackground("Computing MicroProfile Java diagnostics", monitor -> PropertiesManagerForJava.getInstance().diagnostics(javaParams, PsiUtilsLSImpl.getInstance(getProject())));
// When project is indexing and user types a lot of characters in the Java editor, the MicroProfile language server
// validates the Java document and consumes a 'microprofile/java/diagnostics' for each typed character.
// The response of 'microprofile/java/diagnostics' are blocked (or takes some times) and we have
// "Too many non-blocking read actions submitted at once in". To avoid having this error, we create a coalesceBy key
// managed by IJ ReadAction.nonBlocking() to cancel the previous request.
var coalesceBy = new CoalesceByKey("microprofile/java/diagnostics", javaParams.getUris());
return runAsBackground("Computing MicroProfile Java diagnostics", monitor -> PropertiesManagerForJava.getInstance().diagnostics(javaParams, PsiUtilsLSImpl.getInstance(getProject())), coalesceBy);
}

@Override
public CompletableFuture<Location> getPropertyDefinition(MicroProfilePropertyDefinitionParams params) {
return runAsBackground("Computing property definition", monitor -> PropertiesManager.getInstance().findPropertyLocation(params, PsiUtilsLSImpl.getInstance(getProject())));
var coalesceBy = new CoalesceByKey("microprofile/propertyDefinition", params.getUri(), params.getSourceType(), params.getSourceField(), params.getSourceMethod());
return runAsBackground("Computing property definition", monitor -> PropertiesManager.getInstance().findPropertyLocation(params, PsiUtilsLSImpl.getInstance(getProject())), coalesceBy);
}

@Override
public CompletableFuture<ProjectLabelInfoEntry> getJavaProjectLabels(MicroProfileJavaProjectLabelsParams javaParams) {
return runAsBackground("Computing Java projects labels", monitor -> ProjectLabelManager.getInstance().getProjectLabelInfo(javaParams, PsiUtilsLSImpl.getInstance(getProject())));
var coalesceBy = new CoalesceByKey("microprofile/java/projectLabels", javaParams.getUri(), javaParams.getTypes());
return runAsBackground("Computing Java projects labels", monitor -> ProjectLabelManager.getInstance().getProjectLabelInfo(javaParams, PsiUtilsLSImpl.getInstance(getProject())), coalesceBy);
}

@Override
public CompletableFuture<List<ProjectLabelInfoEntry>> getAllJavaProjectLabels() {
return runAsBackground("Computing All Java projects labels", monitor -> ProjectLabelManager.getInstance().getProjectLabelInfo(PsiUtilsLSImpl.getInstance(getProject())));
var coalesceBy = new CoalesceByKey("microprofile/java/workspaceLabels");
return runAsBackground("Computing All Java projects labels", monitor -> ProjectLabelManager.getInstance().getProjectLabelInfo(PsiUtilsLSImpl.getInstance(getProject())),coalesceBy);
}

@Override
public CompletableFuture<JavaFileInfo> getJavaFileInfo(MicroProfileJavaFileInfoParams javaParams) {
return runAsBackground("Computing Java file info", monitor -> PropertiesManagerForJava.getInstance().fileInfo(javaParams, PsiUtilsLSImpl.getInstance(getProject())));
var coalesceBy = new CoalesceByKey("microprofile/java/fileInfo", javaParams.getUri());
return runAsBackground("Computing Java file info", monitor -> PropertiesManagerForJava.getInstance().fileInfo(javaParams, PsiUtilsLSImpl.getInstance(getProject())), coalesceBy);
}

@Override
public CompletableFuture<List<MicroProfileDefinition>> getJavaDefinition(MicroProfileJavaDefinitionParams javaParams) {
return runAsBackground("Computing Java definitions", monitor -> PropertiesManagerForJava.getInstance().definition(javaParams, PsiUtilsLSImpl.getInstance(getProject())));
var coalesceBy = new CoalesceByKey("microprofile/java/definition", javaParams.getUri(),javaParams.getPosition());
return runAsBackground("Computing Java definitions", monitor -> PropertiesManagerForJava.getInstance().definition(javaParams, PsiUtilsLSImpl.getInstance(getProject())), coalesceBy);
}

@Override
public CompletableFuture<MicroProfileJavaCompletionResult> getJavaCompletion(MicroProfileJavaCompletionParams javaParams) {
var coalesceBy = new CoalesceByKey("microprofile/java/completion", javaParams.getUri(),javaParams.getPosition());
return runAsBackground("Computing Java completion", monitor -> {
IPsiUtils utils = PsiUtilsLSImpl.getInstance(getProject());
CompletionList completionList = PropertiesManagerForJava.getInstance().completion(javaParams, utils);
JavaCursorContextResult cursorContext = PropertiesManagerForJava.getInstance().javaCursorContext(javaParams, utils);
return new MicroProfileJavaCompletionResult(completionList, cursorContext);
});
}, coalesceBy);
}

@Override
public CompletableFuture<List<? extends CodeLens>> getJavaCodelens(MicroProfileJavaCodeLensParams javaParams) {
return runAsBackground("Computing Java codelens", monitor -> PropertiesManagerForJava.getInstance().codeLens(javaParams, PsiUtilsLSImpl.getInstance(getProject()), monitor));
var coalesceBy = new CoalesceByKey("microprofile/java/codeLens", javaParams.getUri());
return runAsBackground("Computing Java codelens", monitor -> PropertiesManagerForJava.getInstance().codeLens(javaParams, PsiUtilsLSImpl.getInstance(getProject()), monitor), coalesceBy);
}

@Override
public CompletableFuture<List<CodeAction>> getJavaCodeAction(MicroProfileJavaCodeActionParams javaParams) {
return runAsBackground("Computing Java code actions", monitor -> (List<CodeAction>) PropertiesManagerForJava.getInstance().codeAction(javaParams, PsiUtilsLSImpl.getInstance(getProject())));
var coalesceBy = new CoalesceByKey("microprofile/java/codeAction", javaParams.getUri());
return runAsBackground("Computing Java code actions", monitor -> (List<CodeAction>) PropertiesManagerForJava.getInstance().codeAction(javaParams, PsiUtilsLSImpl.getInstance(getProject())),coalesceBy);
}

@Override
Expand All @@ -184,7 +201,8 @@ public CompletableFuture<CodeAction> resolveCodeAction(CodeAction unresolved) {

@Override
public CompletableFuture<JavaCursorContextResult> getJavaCursorContext(MicroProfileJavaCompletionParams params) {
return runAsBackground("Computing Java Cursor context", monitor -> PropertiesManagerForJava.getInstance().javaCursorContext(params, PsiUtilsLSImpl.getInstance(getProject())));
var coalesceBy = new CoalesceByKey("microprofile/java/javaCursorContext", params.getUri(), params.getPosition());
return runAsBackground("Computing Java Cursor context", monitor -> PropertiesManagerForJava.getInstance().javaCursorContext(params, PsiUtilsLSImpl.getInstance(getProject())), coalesceBy);
}

@Override
Expand Down
Loading

0 comments on commit 34b16af

Please sign in to comment.