eclipse / xtext

Eclipse Xtext™ is a language development framework
http://www.eclipse.org/Xtext
Eclipse Public License 2.0
769 stars 321 forks source link

LSP triggers build request on every keypress #3131

Open awadammar opened 3 months ago

awadammar commented 3 months ago

Slow performing LSP that triggers a build request on each keypress on one of the dsl files, The problem relies on the didChange method which triggers a build request after each keypress.

Additionally, almost all the server capabilities (i.e. content assist) are blocked until the build request is fulfilled.

IMO build requests have to be triggered only on didOpen and didSave. Still, I don't know how it's bad only to update stateful files that keep a state like WorkspaceManager#openDocuments without triggering a build request on didChange.

    @Override
    public void didChange(DidChangeTextDocumentParams params) {
        runBuildable(() -> toBuildable(params));
    }

    /**
     * Evaluate the params and deduce the respective build command.
     */
    protected Buildable toBuildable(DidChangeTextDocumentParams params) {
        VersionedTextDocumentIdentifier textDocument = params.getTextDocument();
        return workspaceManager.didChangeTextDocumentContent(getURI(textDocument), textDocument.getVersion(),
                params.getContentChanges());
    }
cdietrich commented 3 months ago

Can you provide more context

-client -model size / count -build times -why is build so slow if you change only one file

without a build services won’t see complete picture

awadammar commented 3 months ago

-client: VScode -model size/count: on average, 1000 file -build times: I don't have a concrete number here -why is build so slow if you change only one file: I think because we have a lot of model elements that are being referenced from multiple resources, and then the validation will have to be triggered on all the affected resources.

cdietrich commented 3 months ago

i fear without build none of the services would properly work. did you experiment with overriding/adjusting the behaviour?

nadeeshaan commented 3 months ago

As far as I understand, we have to execute the build operation for each key press because the contextual/ semantic meaning of the source is going to be changed each time we update the source. Since you mentioned that the issue is happening in one of the files, couldn't the problem is with the content in that particular file?

awadammar commented 3 months ago

i fear without build none of the services would properly work. did you experiment with overriding/adjusting the behaviour?

I tried to disable the build on didChange, but always get an error that indicates that the document isn't updated.

This is the error I got when I tried to trigger the codeCompeteionafter appending a few newlines on the document: 42683 [pool-3-thread-2] ERROR t.ide.server.concurrent.ReadRequest - Error during request: java.lang.IndexOutOfBoundsException: Position [ line = 49 character = 0 ] text was :

Stacktrace: at org.eclipse.xtext.ide.server.Document.getOffSet(Document.java:62) at org.eclipse.xtext.ide.server.contentassist.ContentAssistService.createCompletionList(ContentAssistService.java:75) at org.eclipse.xtext.ide.server.LanguageServerImpl.lambda$26(LanguageServerImpl.java:587) at org.eclipse.xtext.ide.server.WorkspaceManager.doRead(WorkspaceManager.java:458x) at org.eclipse.xtext.ide.server.LanguageServerImpl.completion(LanguageServerImpl.java:586) at org.eclipse.xtext.ide.server.LanguageServerImpl.lambda$25(LanguageServerImpl.java:572) at org.eclipse.xtext.ide.server.concurrent.ReadRequest.lambda$1(ReadRequest.java:66) at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:539) at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264) at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136) at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635) at java.base/java.lang.Thread.run(Thread.java:833)

I was looking into a way to update the document content without triggering the build, but as you've mentioned it seems not doable without the build.

awadammar commented 3 months ago

As far as I understand, we have to execute the build operation for each key press because the contextual/ semantic meaning of the source is going to be changed each time we update the source. Since you mentioned that the issue is happening in one of the files, couldn't the problem is with the content in that particular file?

Actually, the fact that our DSL relies heavily on references is the problem, if the touched/edited model element is referenced from multiple files then the server will have to build and validate the edited file plus the referencing files.

cdietrich commented 3 months ago

i dont know if there is a easy way to trick around this do you @szarnekow @tivervac

tivervac commented 3 months ago

Actually, the fact that our DSL relies heavily on references is the problem, if the touched/edited model element is referenced from multiple files then the server will have to build and validate the edited file plus the referencing files.

Isn't this what you want, though? If a file is updated, any of the files referencing it needs to consider whether this impacts them as well. Note that a build doesn't start on every keystroke, but whenever the model changes and a given reconciler time is surpassed. If the user thus types fast enough, no build will be triggered until he's done typing. I suppose you could try to

awadammar commented 3 months ago

I appreciate your suggestions, but I didn't get what you exactly meant by batching the requests, I assume you're referring to the didChange request. The idea of increasing the reconciler time seems promising, any hints regarding where I can start from?

tivervac commented 2 months ago

@awadammar Yes, you could aggregate the changes and only really let them trigger the build every once in a while. You'd thus have fewer builds. To increase the reconciler delay, have a look at org.eclipse.xtext.ui.editor.reconciler.XtextReconciler.setDelay(int)

awadammar commented 2 months ago

Thank you @tivervac

cdietrich commented 2 months ago

please note: this is about lsp not xtext.eclipse.ui i guess you would have to collect the changes in languageserver impl