Skip to content
This repository has been archived by the owner on Apr 21, 2023. It is now read-only.

Commit

Permalink
[#1820] jdtqueuedbuilddataexperiments
Browse files Browse the repository at this point in the history 
Signed-off-by: Christian Dietrich <christian.dietrich@itemis.de>
  • Loading branch information
@cdietrich
cdietrich committed Mar 25, 2022
1 parent a43d37a commit fdf2db5
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 7 deletions.
8 changes: 2 additions & 6 deletions org.eclipse.xtext.builder/src/org/eclipse/xtext/builder/impl/XtextBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -662,20 +662,16 @@ protected void addInfosFromTaskAndClean(ToBeBuilt toBeBuilt, Task task, IProgres
* @since 2.19
*/
protected void pollQueuedBuildData() {
boolean needRebuild = false;
if (pollQueuedBuildData(getProject())) {
needRebuild = true;
triggerRequestProjectRebuild();

This comment has been minimized.

Sign in to view

Copy link
@cdietrich

cdietrich Mar 25, 2022

Author Member

is this the correct method?

This comment has been minimized.

Sign in to view

Copy link
@jukzi

jukzi Mar 25, 2022

i feel only 1 issue here: there is the possibility that a project calls both
triggerRequestProjectRebuild() and triggerRequestProjectsRebuild(project).
The javadoc in eclipse.platform does not state what happens then. - it should avoid endless loops then.

This comment has been minimized.

Sign in to view

Copy link
@szarnekow

szarnekow Mar 25, 2022

Contributor

True, triggerRequestProjectRebuild(); should be equivalent to triggerRequestProjectRebuild(getProject());.
We might want to expose only on the second variant of that API and remove the overload without a project.
}
for(IProject project: interestingProjects) {
if (!XtextProjectHelper.hasNature(project)) {
if (pollQueuedBuildData(project)) {
needRebuild = true;
triggerRequestProjectsRebuild(project);
}
}
}
if(needRebuild) {
triggerRequestProjectRebuild();
}
}

/**
Expand Down
5 changes: 4 additions & 1 deletion ...ipse.xtext.builder/src/org/eclipse/xtext/builder/impl/javasupport/JdtQueuedBuildData.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ protected boolean doNeedRebuild(JavaBuilderState javaBuilderState, Procedure1<?
Iterator<UnconfirmedStructuralChangesDelta> ucDeltas = unconfirmedDeltas.iterator();
int buildNumber = javaBuilderState.getBuildNumber().intValue();
IProject project = javaBuilderState.getProject();
boolean result = false;
while (ucDeltas.hasNext()) {
UnconfirmedStructuralChangesDelta ucDelta = ucDeltas.next();
if ((ucDelta.getBuildNumber() < buildNumber)
Expand All @@ -107,9 +108,11 @@ protected boolean doNeedRebuild(JavaBuilderState javaBuilderState, Procedure1<?
if (processor != null) {
processor.apply(ucDelta);
}
} else if (ucDelta.getProject().equals(project)) {
result = true;

This comment has been minimized.

Sign in to view

Copy link
@jukzi

jukzi Mar 25, 2022

works well :-)

This comment has been minimized.

Sign in to view

Copy link
@szarnekow

szarnekow Mar 25, 2022

Contributor

ucDelta.getProject().equals(project)

This is checked in both branches but the delta is only removed in one of both cases.
I'm afraid it's increasingly hard to reason about the flow of information / managed state with the Java integration. Since we have seen performance problems with the JavaChangeQueueFiller also in workspaces, that do not even use the Java integration, we might consider a more holistic approach to fix the various challenges related to the Xtext<->JDT integration.
}
}
return !unconfirmedDeltas.isEmpty();
return result;
}

protected boolean namesIntersect(IResourceDescription resourceDescription, Set<QualifiedName> names) {
Expand Down

0 comments on commit fdf2db5

Please sign in to comment.