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.

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

Wait a bit after change before sending diagnostics #1202

Merged
merged 1 commit into from
May 19, 2022

Conversation

angelozerr
Copy link
Contributor

Wait a bit after change before sending diagnostics

Fixes #1162

Signed-off-by: azerr [email protected]

@angelozerr
Copy link
Contributor Author

This PR publish diagnostics with 200ms as delay.

Here a demo with vscode-xml and linked edting range:

PublishDiagnosticsWithDelayDemo.

Instead of reporting a diagnostics per typed character, it generate few diagnotics.

@angelozerr
Copy link
Contributor Author

@vrubezhny could you review too this PR and tell me if it improves performance for Eclipse IDE with WWD please.

Copy link
Contributor

@AlexXuChen AlexXuChen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The delay is working as described, however, perhaps we can optimize diagnostic reporting further by only sending the affected diagnostic in the response (i.e. if there are 2 diagnostics in 2 separate files in the workspace, and one diagnostic is changed, the other still sends a publishDiagnostics response)

@angelozerr
Copy link
Contributor Author

When you say affected diagnostic you mean the document which is updating?

If it that the report of the diagnostic is just for the updated document.

But if you save the document in this case the server will report diagnostics for all opened document. Perhaps it is tour usecase

@AlexXuChen
Copy link
Contributor

But if you save the document in this case the server will report diagnostics for all opened document. Perhaps it is tour usecase

Yes, I update the diagnostic by saving, so all diagnostics are reported again.

@angelozerr
Copy link
Contributor Author

Yes, I update the diagnostic by saving, so all diagnostics are reported again.

It is the expected behvior. This PR provides delay for reporting diagnostic when document changed. When document it is saved, there is no delay and I think it is not annoying to report diagnostics for all opened files just on save.

The problem is when you change document, you must try to report few diagnotics to improve :

  • performance on XML languagse server side. if user type N characters, in master it validate N times and report N diagnostics. With this PR if user type N characters, it should validate one time and report one diagnotic
  • performance on LSP client side.

@angelozerr angelozerr merged commit 420076e into eclipse:master May 19, 2022
@angelozerr
Copy link
Contributor Author

Thanks @fbricon @AlexXuChen for your review!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Wait a bit after change before sending diagnostics
3 participants