Improve project initialization #2252
Conversation
|
Fix for #2192 |
|
Thank you @gayanper, I'll try the change later. Meanwhile, I have a question: the current job is already wrapped as a |
|
@jdneo If we check the code at @rgrunber do you know anything different than what I have found above ? |
|
@jdneo I missed the line 278, seems like it should set the rule for the whole job, But still I think we have the issue of stoping the build job if it was triggered before the import start. I need to validate if the current fix handles that. |
|
Regarding the autobuild job, I see two drawbacks: one is slowing down the project import performance, another is conflict with maven commands running in the terminal. So I'm thinking if we can set the autobuild flag to |
Thats sounds like how IJ works. I guess then we delay the build to the point where we try to run/debug. Thats fine as well based on the workflow. But on the other hand, I’m not sure if some syntax errors will go away if auto build is disabled. I have not used eclipse without autobuild for a long time. |
|
When auto build is disabled, jdt.ls still reports compilation errors for the opened files (workingcopy). This is because it uses a separate validation job to report diagnostics for these active files and doesn't depend on the auto build. |
|
So you mean if i rename a method in a file and then open the file which has a reference for that method it should show me a compilation error in jdt.ls is it ? The rename is just changing the name not rename refactoring |
|
A related issue - #2256 |
I think this is a good idea as well, If we all are ok with it I can try that out too. For mean time I have update the PR by adding support to interrupt the build manager before start the project import since while project import workspace job is running the build job will not kick in. Also the interruption implementation has a fallback if Eclipse core changes the internal implementation of IWorkspace. |
Yes, if you rename a method in file A without refactoring. Then open file B, which has a reference to that old method, jdt.ls will report undefined method error even if autobuild is disabled. |
|
@gayanper Which project and OS you are using? |
I found this issue on Windows working with a project which has around 1k source files. But I don't have access to that machine nor the project anymore. |
@gayanper This issue is caused by redhat-developer/vscode-java#793 |
|
The idea looks good. interrupting the build job should be better than toggling the auto build state. Regarding to the performance, I tried it with redhat-developer/vscode-java#880 on Windows, but did not observe a significant improvement. Looks like the impact of |
Do you mean the changes done in this PR or the other linked PR @jdneo ? |
@jdneo could you check redhat-developer/vscode-java#793 ? See |
|
Sorry I didn't say it clearly.
I mean the change in this PR. @snjeza Changes in redhat-developer/vscode-java#880 are fine. |
|
@jdneo yes may be with the other fix the impact is lower now of switching build state. But do you think the fix is not better than simply toggling the build mode when importing projects ? |
|
I think interrupting is better than toggling. I was just trying to share my observation in case that this is not related with my environment. :) |
|
@jdneo thanks for clarifying. I will try to resolve the conflict soon |
Today when importing projects we disable auto build and reset it back to previous value to avoid conflicts between project imports and build jobs. But resetting back auto build to true cause the whole workspace to build again which takes considerable time for large projects which blocks all other operations. The improvement use workspace scheduling rule to make sure another job cannot be executed at the same time such as a auto build according to how eclipse workspace sensitive jobs are implemented internally and try to interrupt the auto build before starting project import. Fix tests - removing unnecessary stubs reported by mockito - add wait for project import
gayanper
force-pushed
the
improve_project_import
branch
from
538d62c
to
deaf7d4
Compare
There was a problem hiding this comment.
Though the performance improvement is not that significant, but I can still observe that after applying this interruption mechanism, less jobs can be observed when opening a Gradle project which has already imported beforehand:
Below is the outputs of the build status terminal after opening a project.
Before
52ebba19 Importing Gradle project(s) [Done]
64fceaa2 Synchronizing projects [Done]
eedb8d0d Synchronizing projects [Done]
a35904c2 Building [Done]
d207e4d3 Building [Done]
a7eafa7d Building [Done]
After
b7b590bb Importing Gradle project(s) [Done]
8b198365 Synchronizing projects [Done]
3d196bdb Synchronizing projects [Done]
Today when importing projects we disable auto build and reset it back to previous value to avoid conflicts between project imports and build jobs. But resetting back auto build to true cause the whole workspace to build again which takes considerable time for large projects which blocks all other operations.
The improvement use workspace scheduling rule to make sure another job cannot be executed at the same time such as a auto build according to how eclipse workspace sensitive jobs are implemented internally.
Must further future improvement would be to apply this rule at each project import iteration if further optimization is needed.