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

No need to run the reconcile operation in WorkspaceJob #2660

Merged
merged 1 commit into from May 23, 2023

Conversation

testforstephen
Copy link
Contributor

The validation/diagnostics job is used to update the Java model structure with the workingcopy buffer contents. Since it does not modify the underlying resource, it should be fine to run in a regular job instead of a workspace job. A workspace job is meant for operations that modify the workspace resource, as per the Eclipse WorkspaceJob spec and Concurrency and the workspace. Similarly, the Java Reconciler job in Eclipse runs in a regular daemon thread without a scheduling rule, so we may not need a scheduling rule for the validation/diagnostics job either.

This can speed up the document lifecycle job and allow other features that rely on lifecycle job to enable eariler during build stage, such as go-to-definition, documentSymbol and code actions, etc.

@rgrunber
Copy link
Contributor

rgrunber commented May 18, 2023

Why is it guaranteed that a compilation unit working copy never modifies its underlying resource (the compilation unit) when cu.makeConsistent(..) is called ? For example, if a didChange is called on a particular source file, and we apply the changes to the underlying document, once triggerValidation(..) is called, I would think cu.makeConsistent(..) would do something in the workspace.

I think if the reconciler job doesn't run with a scheduling rule then that'd be a good argument to avoid locking the resource that the compilation unit represents though.

@testforstephen
Copy link
Contributor Author

The underlying resource I'm talking about is Eclipse IResource.
didChange updates the Java model element (ICompilationUnit) from in-memory buffer, but not the underlying Eclipse resource (IResource or IFile). All file changes are applied by the VS Code client, and the Java language server refreshes the Eclipse IResource based on events such as didSave and didChangedWatchedFiles. Usually, a workspace job is required when modifying an Eclipse IResource.

@testforstephen testforstephen marked this pull request as ready for review May 22, 2023 06:31
@testforstephen testforstephen requested review from jdneo, snjeza and rgrunber and removed request for jdneo May 22, 2023 06:31
@testforstephen
Copy link
Contributor Author

testforstephen commented May 22, 2023

This is how it works in Eclipse. When you do a clean build, the outline view and diagnostics is still able to sync with the new changes instantly as you edit.

eclipse.mp4

Here is the Java reconciler job in Eclipse.
image

@testforstephen testforstephen merged commit 420a67d into eclipse-jdtls:master May 23, 2023
6 checks passed
@testforstephen testforstephen deleted the jinbo_reconcile branch May 23, 2023 05:26
@testforstephen testforstephen added this to the End May 2023 milestone May 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants