[go: up one dir, main page]
Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

LSP triggers build request on every keypress #3131

Open
awadammar opened this issue Aug 8, 2024 · 12 comments
Open

LSP triggers build request on every keypress #3131

awadammar opened this issue Aug 8, 2024 · 12 comments

Comments

@awadammar
Copy link
Contributor
awadammar commented Aug 8, 2024 •
edited
Loading

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
Copy link
Member

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
Copy link
Contributor Author

-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
Copy link
Member

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

@nadeeshaan
Copy link

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Member

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

@tivervac
Copy link
Contributor

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

  • batch requests, but this will delay feedback to the user
  • increase the time before the reconciler kicks in
  • ignore the references until the file is saved/closed. This doesn't seem very user-friendly, though...

@awadammar
Copy link
Contributor Author

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
Copy link
Contributor

@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
Copy link
Contributor Author
awadammar commented Aug 20, 2024 •
edited
Loading

Thank you @tivervac

@cdietrich
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@cdietrich @nadeeshaan @tivervac @awadammar