From 14feb0f0c6e68564073d2f934057441ab428029e Mon Sep 17 00:00:00 2001 From: Kris De Volder Date: Mon, 28 Nov 2016 13:26:32 -0800 Subject: [PATCH] Make completions computation properly async --- .../VscodeCompletionEngineAdapter.java | 37 +++++++++++-------- .../util/SimpleTextDocumentService.java | 8 ++-- .../ide/vscode/commons/util/Futures.java | 8 ++-- .../yaml/completion/YamlCompletionEngine.java | 2 +- .../ApplicationPropertiesLanguageServer.java | 3 +- 5 files changed, 32 insertions(+), 26 deletions(-) diff --git a/vscode-extensions/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/completion/VscodeCompletionEngineAdapter.java b/vscode-extensions/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/completion/VscodeCompletionEngineAdapter.java index 8e62f6268..7bb9bf5f1 100644 --- a/vscode-extensions/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/completion/VscodeCompletionEngineAdapter.java +++ b/vscode-extensions/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/completion/VscodeCompletionEngineAdapter.java @@ -20,22 +20,23 @@ import org.springframework.ide.vscode.commons.languageserver.util.TextDocument; import org.springframework.ide.vscode.commons.util.Futures; import org.springframework.ide.vscode.commons.util.StringUtil; +import reactor.core.publisher.Mono; +import reactor.core.scheduler.Schedulers; + /** * Adapts a {@link ICompletionEngine}, wrapping it, to implement {@link VscodeCompletionEngine} */ public class VscodeCompletionEngineAdapter implements VscodeCompletionEngine { private final static int MAX_COMPLETIONS = 20; - private int maxCompletions = MAX_COMPLETIONS; - final static Logger logger = LoggerFactory.getLogger(VscodeCompletionEngineAdapter.class); - public static final String VS_CODE_CURSOR_MARKER = "{{}}"; private SimpleLanguageServer server; private ICompletionEngine engine; + public VscodeCompletionEngineAdapter(SimpleLanguageServer server, ICompletionEngine engine) { this.server = server; this.engine = engine; @@ -47,13 +48,20 @@ public class VscodeCompletionEngineAdapter implements VscodeCompletionEngine { @Override public CompletableFuture getCompletions(TextDocumentPositionParams params) { - //TODO: This returns a CompletableFuture which suggests we should try to do expensive work asyncly. - // We are currently just doing all this in a blocking way and wrapping the already computed list into - // a trivial pre-resolved future. - try { - SimpleTextDocumentService documents = server.getTextDocumentService(); - TextDocument doc = documents.get(params); - if (doc!=null) { + return getCompletionsMono(params).toFuture(); + } + + + + private Mono getCompletionsMono(TextDocumentPositionParams params) { + SimpleTextDocumentService documents = server.getTextDocumentService(); + TextDocument doc = documents.get(params); + if (doc!=null) { + return Mono.fromCallable(() -> { + //TODO: This callable is a 'big lump of work' so can't be canceled in pieces. + // Should we push using of reactive streems down further and compose this all + // using reactive style? If not then this is overkill could just as well use + // only standard Java AP such as Executor and CompletableFuture's directly. int offset = doc.toOffset(params.getPosition()); List completions = new ArrayList<>(engine.getCompletions(doc, offset)); Collections.sort(completions, ScoreableProposal.COMPARATOR); @@ -75,12 +83,11 @@ public class VscodeCompletionEngineAdapter implements VscodeCompletionEngine { } } list.setItems(items); - return Futures.of(list); - } - } catch (Exception e) { - logger.error("error computing completions", e); + return list; + }) + .subscribeOn(Schedulers.single()); //!!! without this the mono will just be computed on the same thread that calls it. } - return SimpleTextDocumentService.NO_COMPLETIONS; + return Mono.just(SimpleTextDocumentService.NO_COMPLETIONS); } private CompletionItem adaptItem(TextDocument doc, ICompletionProposal completion, SortKeys sortkeys) throws Exception { diff --git a/vscode-extensions/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/util/SimpleTextDocumentService.java b/vscode-extensions/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/util/SimpleTextDocumentService.java index 4871ec786..0351a22a6 100644 --- a/vscode-extensions/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/util/SimpleTextDocumentService.java +++ b/vscode-extensions/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/util/SimpleTextDocumentService.java @@ -166,11 +166,9 @@ public class SimpleTextDocumentService implements TextDocumentService { return doc; } - public final static CompletableFuture NO_COMPLETIONS = Futures.of( - new CompletionList(false, Collections.emptyList())); - - public final static CompletableFuture NO_HOVER = Futures.of(new Hover(ImmutableList.of(), null)); + public final static CompletionList NO_COMPLETIONS = new CompletionList(false, Collections.emptyList()); + public final static CompletableFuture NO_HOVER = CompletableFuture.completedFuture(new Hover(ImmutableList.of(), null)); @Override public CompletableFuture completion(TextDocumentPositionParams position) { @@ -178,7 +176,7 @@ public class SimpleTextDocumentService implements TextDocumentService { if (h!=null) { return completionHandler.handle(position); } - return NO_COMPLETIONS; + return CompletableFuture.completedFuture(NO_COMPLETIONS); } @Override diff --git a/vscode-extensions/commons/commons-util/src/main/java/org/springframework/ide/vscode/commons/util/Futures.java b/vscode-extensions/commons/commons-util/src/main/java/org/springframework/ide/vscode/commons/util/Futures.java index 305c6f495..4bb1e433a 100644 --- a/vscode-extensions/commons/commons-util/src/main/java/org/springframework/ide/vscode/commons/util/Futures.java +++ b/vscode-extensions/commons/commons-util/src/main/java/org/springframework/ide/vscode/commons/util/Futures.java @@ -4,10 +4,12 @@ import java.util.concurrent.CompletableFuture; public class Futures { + /** + * Depcrecated. Use {@link CompletableFuture}.completedFuture() instead. + */ + @Deprecated public static CompletableFuture of(T value) { - CompletableFuture f = new CompletableFuture(); - f.complete(value); - return f; + return CompletableFuture.completedFuture(value); } } diff --git a/vscode-extensions/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/completion/YamlCompletionEngine.java b/vscode-extensions/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/completion/YamlCompletionEngine.java index 1e2ca714a..7fe4572f7 100644 --- a/vscode-extensions/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/completion/YamlCompletionEngine.java +++ b/vscode-extensions/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/completion/YamlCompletionEngine.java @@ -21,12 +21,12 @@ import org.springframework.ide.vscode.commons.languageserver.util.IDocument; import org.springframework.ide.vscode.commons.util.Assert; import org.springframework.ide.vscode.commons.yaml.path.YamlPath; import org.springframework.ide.vscode.commons.yaml.structure.YamlDocument; -import org.springframework.ide.vscode.commons.yaml.structure.YamlStructureProvider; import org.springframework.ide.vscode.commons.yaml.structure.YamlStructureParser.SKeyNode; import org.springframework.ide.vscode.commons.yaml.structure.YamlStructureParser.SNode; import org.springframework.ide.vscode.commons.yaml.structure.YamlStructureParser.SNodeType; import org.springframework.ide.vscode.commons.yaml.structure.YamlStructureParser.SRootNode; import org.springframework.ide.vscode.commons.yaml.structure.YamlStructureParser.SSeqNode; +import org.springframework.ide.vscode.commons.yaml.structure.YamlStructureProvider; import org.springframework.ide.vscode.commons.yaml.util.YamlIndentUtil; /** diff --git a/vscode-extensions/vscode-application-properties/src/main/java/org/springframework/ide/vscode/application/properties/ApplicationPropertiesLanguageServer.java b/vscode-extensions/vscode-application-properties/src/main/java/org/springframework/ide/vscode/application/properties/ApplicationPropertiesLanguageServer.java index aca17fd9c..1c4b66ab2 100644 --- a/vscode-extensions/vscode-application-properties/src/main/java/org/springframework/ide/vscode/application/properties/ApplicationPropertiesLanguageServer.java +++ b/vscode-extensions/vscode-application-properties/src/main/java/org/springframework/ide/vscode/application/properties/ApplicationPropertiesLanguageServer.java @@ -39,8 +39,7 @@ public class ApplicationPropertiesLanguageServer extends SimpleLanguageServer { private VscodeCompletionEngineAdapter completionEngine; private SpringPropertiesReconcileEngine reconcileEngine; private VscodeHoverEngineAdapter hoverEngine; - - + public ApplicationPropertiesLanguageServer(SpringPropertyIndexProvider indexProvider, TypeUtilProvider typeUtilProvider, JavaProjectFinder javaProjectFinder) { this.indexProvider = indexProvider; this.typeUtilProvider = typeUtilProvider;