Wait for jobs to complete when resolving the classpaths #1476
Conversation
| */ | ||
| public static ClasspathResult getClasspaths(String uri, ClasspathOptions options) throws CoreException, URISyntaxException { | ||
| public static ClasspathResult getClasspaths(String uri, ClasspathOptions options) throws CoreException, URISyntaxException, InterruptedException { | ||
| JobHelpers.waitForJobsToComplete(new NullProgressMonitor()); |
There was a problem hiding this comment.
That's too broad. You'll wait for unrelated jobs to finish
There was a problem hiding this comment.
Can I narrow to:
JobHelpers.waitForJobs(IConstants.UPDATE_PROJECT_FAMILY, new NullProgressMonitor());
JobHelpers.waitForBuildJobs(10 * 1000);Also wait for build jobs since update request will also trigger a clean build in MavenBuilder. But return the result no matter build finishes or not after 10 secs.
There was a problem hiding this comment.
I don't think you need to wait for build jobs. During a build all the classpaths are supposed to have been set already, so building projects won't change anything to that
There was a problem hiding this comment.
Yes you are right. I found out that the case will fail after the invocation of JavaCore.setClasspathContainer(), which will trigger SetContainerOperation
I'm deciding to add following code:
IWorkspace workspace = ResourcesPlugin.getWorkspace();
workspace.run(new IWorkspaceRunnable() {
@Override
public void run(IProgressMonitor monitor) {
}
}, workspace.getRoot(), 0, new NullProgressMonitor());to wait for it. (JavaModeChangeOperation will lock the workspace tree). Does this make sense?
There was a problem hiding this comment.
@snjeza your opinion on that?
There was a problem hiding this comment.
Just add more background here: The cases might fail with a very high possibility after BuildPathManager#mavenProjectChanged(), where the SetContainerOperation will be triggered.
There was a problem hiding this comment.
you can try
IWorkspace workspace = ResourcesPlugin.getWorkspace();
workspace.run(new IWorkspaceRunnable() {
@Override
public void run(IProgressMonitor monitor) {
}
}, workspace.getRoot(), IWorkspace.AVOID_UPDATE, new NullProgressMonitor());
jdneo
force-pushed
the
cs/classpath-update
branch
2 times, most recently
from
38b1a59
to
901d0af
Compare
| projectsManager.updateProject(project, true); | ||
|
|
||
| for (int i = 0; i < 10; i++) { | ||
| options.scope = "test"; |
|
Just came up in mind that maybe implement by this way will have much less impact on the perf? IWorkspace workspace = ResourcesPlugin.getWorkspace();
ISchedulingRule currentRule = Job.getJobManager().currentRule();
ISchedulingRule schedulingRule;
if (currentRule != null && currentRule.contains(javaProject.getSchedulingRule())) {
schedulingRule = null;
} else {
schedulingRule = javaProject.getSchedulingRule();
}
workspace.run(new IWorkspaceRunnable() {
@Override
public void run(IProgressMonitor monitor) throws CoreException {
// get classpath
}
}, schedulingRule, IWorkspace.AVOID_UPDATE, new NullProgressMonitor()); |
Signed-off-by: Sheng Chen <sheche@microsoft.com>
jdneo
force-pushed
the
cs/classpath-update
branch
from
901d0af
to
2d98c27
Compare
|
Thanks @jdneo ! |
When I'm working for this PR: redhat-developer/vscode-java#1484, I found that the test case
getClasspathwill randomly fail, see: https://travis-ci.org/github/redhat-developer/vscode-java/jobs/697117335It's because if we try to get the classpath when the m2e is updating the projects, the return results might only contain the source classes without dependency jars. so I add a wait here.
Signed-off-by: Sheng Chen sheche@microsoft.com