Community
Participate
Working Groups
We have many (say n) xtext projects. Each requests rebuild. Since Bug 568299 every rebuild request restarts buildloop from scratch. Results in a try to build every project n times. BuildManager.needsBuild(InternalBuilder, int) returns false for the previous projects, so at least they get not really rebuild. But, there is now O(n^2) calls to needsBuild(). The progress bar is always at 100% after the first of 2*n needed iterations since the progress Estimation in BuildManager.basicBuildLoop wrongly assumes there will be only a single iteration. Please either revert 568299 and it's follow ups, or somehow make and use a better API which makes more clear that only the current project should be rebuild (is that the intention of xtext's rebuild request?)- not all. And fix the progress calculation appropriately.
I think the main problem is here: *why* xtext builder constantly request rebuilds? *This* should be understood and addressed.
(In reply to Jörg Kubitz from comment #0) > We have many (say n) xtext projects. Each requests rebuild. Since Bug 568299 > every rebuild request restarts buildloop from scratch. Not sure why do you think this is due bug 568299 changes. That change only assures that if one builder on a single project requests a rebuild, *following* builders on *same* project aren't executed. The "outer" build loop behavior was unchanged. In your case it would mean, if you had other builders configured to run *after* xtext on the project, they were executed before *anyway* every time xtext builder requests a rebuild, so you should see now *less* work and not more. So I would really make sure that you understand why xtext build requests rebuilds. We are also using Xtext and have ~200 plugin projects in the typical workspace, but without anyone noticing any "endless" rebuilds. Have you tried to start IDE with the -Dorg.eclipse.core.resources.disallowEarlyInnerBuildLoopExit=false system property to restore old behavior? Is that really solves you problem? If so, your xtext build depends on the builders that are configured to run *after* xtext builder. Try to change the .project configuration to move those builders to run before xtext and check if that helps to avoid rebuild requests.
(In reply to Andrey Loskutov from comment #2) > The "outer" build loop behavior was unchanged. I do not doubt you had a good intention. But currently it just does not work like you intended. I guess it's because "rebuildRequested" is set globaly. so after one builder did ran into it all other projects will also quit their build loop early - restarting the outer loop from scratch. (which caused also Bug 577530) > -Dorg.eclipse.core.resources.disallowEarlyInnerBuildLoopExit=false solves all problems (sucess after 4 iterations - otherwise hundrets) It's totaly absurd to change the build order of a million projects just because of that change.
(In reply to Jörg Kubitz from comment #3) > (In reply to Andrey Loskutov from comment #2) > > The "outer" build loop behavior was unchanged. > > I do not doubt you had a good intention. But currently it just does not work > like you intended. May be you should look at the change? > I guess it's because "rebuildRequested" is set globaly. so after one builder > did ran into it all other projects will also quit their build loop early - > restarting the outer loop from scratch. Please check the change once again: https://git.eclipse.org/r/c/platform/eclipse.platform.resources/+/171345/5/bundles/org.eclipse.core.resources/src/org/eclipse/core/internal/events/BuildManager.java The rebuildRequested isn't touched by the change, it is set by builders, and this is what you should understand first - *why* the xtext builder does that? > > -Dorg.eclipse.core.resources.disallowEarlyInnerBuildLoopExit=false > > solves all problems (sucess after 4 iterations - otherwise hundrets) Interesting. *Do* you have builders defined after xtext builder? > It's totaly absurd to change the build order of a million projects just > because of that change. If the config is wrong, it is not absurd to fix it.
New Gerrit change created: https://git.eclipse.org/r/c/platform/eclipse.platform.resources/+/191487
(In reply to Eclipse Genie from comment #5) > New Gerrit change created: > https://git.eclipse.org/r/c/platform/eclipse.platform.resources/+/191487 Why ever xtext did want a rebuild - with that change i do not get a single retry in the outer loop but only single rebuilds per project. And it keeps the early exit.
(In reply to Jörg Kubitz from comment #6) > (In reply to Eclipse Genie from comment #5) > > New Gerrit change created: > > https://git.eclipse.org/r/c/platform/eclipse.platform.resources/+/191487 > > Why ever xtext did want a rebuild - with that change i do not get a single > retry in the outer loop but only single rebuilds per project. > And it keeps the early exit. See my comment on gerrit. We aren't only to have solve xtext rebuild errors (which you don't even want to investigate), it is also about build for project dependencies chains where the patch as proposed would break the build. Please before trying to patch the platform, try to understand why xtext isn't working properly.
i still dont get the point about "constantly" require rebuilds. xtext will always settle. unfortunately there is no reproducer for rebuilds > 1 (or 2)
I did not state xtext "constantly" requires rebuild (that was andrey interpretation). I only see every xtext builder request a rebuild - but after andreys change that causes enough trouble. Why xtext calls rebuild? I do not know why - i can see only when. I saw two different scenarios: 1. when the xtext builder writes a .java file into the src-gen folder: Thread [Worker-9: Building] (Suspended (breakpoint at line 1199 in org.eclipse.core.internal.events.BuildManager)) org.eclipse.core.internal.events.BuildManager.requestRebuild() line: 1199 org.eclipse.xtext.builder.impl.XtextBuilder(org.eclipse.core.internal.events.InternalBuilder).needRebuild() line: 181 org.eclipse.xtext.builder.impl.XtextBuilder(org.eclipse.core.resources.IncrementalProjectBuilder).needRebuild() line: 363 org.eclipse.xtext.builder.impl.BuildContext.needRebuild() line: 60 org.eclipse.xtext.builder.BuilderParticipant$1.handleFileAccess(org.eclipse.core.resources.IFile) line: 412 org.eclipse.xtext.builder.BuilderParticipant$1.afterFileUpdate(org.eclipse.core.resources.IFile) line: 400 org.eclipse.xtext.builder.EclipseResourceFileSystemAccess2.generateFile(org.eclipse.core.resources.IFile, java.io.InputStream, org.eclipse.core.resources.IFile, java.lang.CharSequence, org.eclipse.xtext.generator.OutputConfiguration) line: 270 org.eclipse.xtext.builder.EclipseResourceFileSystemAccess2.generateFile(java.lang.String, java.lang.String, java.lang.CharSequence) line: 218 org.eclipse.xtext.builder.EclipseResourceFileSystemAccess2(org.eclipse.xtext.generator.AbstractFileSystemAccess).generateFile(java.lang.String, java.lang.CharSequence) line: 100 com/wamas/wpl/generator/artifacts/meta/MetaProcessGroupGenerator.xtend line: 28 com/wamas/wpl/generator/artifacts/meta/ExternalMetaGenerator.xtend line: 76 com.wamas.wpl.generator.CompoundFactoryAndMetaGenerator.doGenerate(org.eclipse.xtext.builder.IXtextBuilderParticipant$IBuildContext, org.eclipse.emf.ecore.resource.ResourceSet, org.eclipse.xtext.generator.IFileSystemAccess) line: 29 com/wamas/wpl/generator/BundleModelBuilderParticipant.xtend line: 145 com.wamas.wpl.generator.BundleModelBuilderParticipant(org.eclipse.xtext.builder.BuilderParticipant).doGenerate(org.eclipse.xtext.resource.IResourceDescription$Delta, org.eclipse.xtext.builder.IXtextBuilderParticipant$IBuildContext, org.eclipse.xtext.generator.IFileSystemAccess) line: 563 com.wamas.wpl.generator.BundleModelBuilderParticipant(org.eclipse.xtext.builder.BuilderParticipant).doBuild(java.util.List<org.eclipse.xtext.resource.IResourceDescription.Delta>, java.util.Map<java.lang.String,org.eclipse.xtext.generator.OutputConfiguration>, java.util.Map<org.eclipse.xtext.generator.OutputConfiguration,java.lang.Iterable<org.eclipse.core.resources.IMarker>>, org.eclipse.xtext.builder.IXtextBuilderParticipant.IBuildContext, org.eclipse.xtext.builder.EclipseResourceFileSystemAccess2, org.eclipse.core.runtime.IProgressMonitor) line: 303 com.wamas.wpl.generator.BundleModelBuilderParticipant(org.eclipse.xtext.builder.BuilderParticipant).build(org.eclipse.xtext.builder.IXtextBuilderParticipant$IBuildContext, org.eclipse.core.runtime.IProgressMonitor) line: 265 com/wamas/wpl/generator/BundleModelBuilderParticipant.xtend line: 47 org.eclipse.xtext.builder.impl.RegistryBuilderParticipant$DeferredBuilderParticipant.build(org.eclipse.xtext.builder.IXtextBuilderParticipant$IBuildContext, org.eclipse.core.runtime.IProgressMonitor) line: 164 org.eclipse.xtext.builder.impl.RegistryBuilderParticipant.build(org.eclipse.xtext.builder.IXtextBuilderParticipant$IBuildContext, org.eclipse.core.runtime.IProgressMonitor) line: 70 org.eclipse.xtext.builder.impl.XtextBuilder.doBuild(org.eclipse.xtext.builder.impl.ToBeBuilt, java.util.Set<java.lang.String>, org.eclipse.core.runtime.IProgressMonitor, org.eclipse.xtext.builder.IXtextBuilderParticipant.BuildType) line: 392 org.eclipse.xtext.builder.impl.XtextBuilder.addInfosFromTaskAndBuild(org.eclipse.xtext.builder.impl.ClosedProjectsQueue$Task, org.eclipse.xtext.builder.impl.ToBeBuilt, org.eclipse.xtext.builder.IXtextBuilderParticipant$BuildType, org.eclipse.core.runtime.IProgressMonitor) line: 322 org.eclipse.xtext.builder.impl.XtextBuilder.fullBuild(org.eclipse.core.runtime.IProgressMonitor, boolean) line: 429 org.eclipse.xtext.builder.impl.XtextBuilder.build(int, java.util.Map, org.eclipse.core.runtime.IProgressMonitor) line: 202 2. when "javaBuilderState" has a "unconfirmedDelta" (as far as i understand: the builder in a following project sees the changes in the src-gen folder that the xtext builder in scenario 1 generated) Thread [Worker-9: Building] (Suspended (breakpoint at line 110 in org.eclipse.xtext.builder.impl.javasupport.JdtQueuedBuildData)) owns: org.eclipse.xtext.builder.impl.QueuedBuildData (id=141) org.eclipse.xtext.builder.impl.javasupport.JdtQueuedBuildData.doNeedRebuild(org.eclipse.xtext.common.types.ui.notification.JavaBuilderState, org.eclipse.xtext.xbase.lib.Procedures.Procedure1<? super org.eclipse.xtext.builder.impl.javasupport.UnconfirmedStructuralChangesDelta>) line: 110 org.eclipse.xtext.builder.impl.javasupport.JdtQueuedBuildData.needsRebuild(org.eclipse.core.resources.IProject, java.util.Collection<org.eclipse.xtext.resource.IResourceDescription.Delta>) line: 95 org.eclipse.xtext.builder.impl.QueuedBuildData$CompositeContribution.needsRebuild(org.eclipse.core.resources.IProject, java.util.Collection<org.eclipse.xtext.resource.IResourceDescription.Delta>) line: 110 org.eclipse.xtext.builder.impl.QueuedBuildData.needRebuild(org.eclipse.core.resources.IProject) line: 250 org.eclipse.xtext.builder.impl.XtextBuilder.pollQueuedBuildData(org.eclipse.core.resources.IProject) line: 546 org.eclipse.xtext.builder.impl.XtextBuilder.pollQueuedBuildData() line: 527 org.eclipse.xtext.builder.impl.XtextBuilder.fullBuild(org.eclipse.core.runtime.IProgressMonitor, boolean) line: 418 org.eclipse.xtext.builder.impl.XtextBuilder.build(int, java.util.Map, org.eclipse.core.runtime.IProgressMonitor) line: 202 org.eclipse.core.internal.events.BuildManager$2.run() line: 868 The build command order is set to <name>org.eclipse.xtext.ui.shared.xtextBuilder</name> <name>org.eclipse.jdt.core.javabuilder</name> <name>org.eclipse.pde.ManifestBuilder</name> <name>org.eclipse.pde.SchemaBuilder</name> <name>org.eclipse.pde.ds.core.builder</name> I don't think that buildorder is wrong: The xtext is supposed to generate .java code in the src-gen. And the following java builder should compile those.
but this should happen once per peroject only?!? or do you generate even on empty delta?
(In reply to Christian Dietrich from comment #10) > but this should happen once per peroject only?!? yea, probably. But that is enough to now restart the whole build loop from scratch n times! > or do you generate even on empty delta? i am not into xtext, i do not know how to answer that. does it even matter?
i fear nobody knows why xtext does this besides maybe sebastian. its all 10 years old. so maybe the 2x is too unsave and a 5x would be better.
Created attachment 288171 [details] Sampling Autobuild after single file change Screenshot.png I took some measurements how much we are affected by autobuild. There are two different scenarios: 1. Full build after touching all Projects (bug 578968) [working set WAMAS Development] 2. Touching a single file [com.wamas.platform.jdbc.ConnectionInfo] | disallowEarlyInnerBuildLoopExit| EarlyExit 1. full build | ~300 sec | ~130 sec 2. single file | 2-3 sec | 15-40 sec Surprisingly the full build is faster with change of Bug 568299 but 10 times slower after single file change. Touching a single file is the normal usecase. I attached sampling of that with EarlyExit.
(In reply to Christian Dietrich from comment #12) > i fear nobody knows why xtext does this besides maybe sebastian. > its all 10 years old. I assume this could be for the case where xtext builder is set to run *after* java (or other) builders, so they can "pick up" files generated during the build. If I remember right, the JDT doesn't monitor changes on file system *during* the build (or deltas aren't sent?), so that this was probably a "hack". Requesting rebuild makes no sense if the xtext builder is set up to run before java/other builders, because they will see generated files anyway. (In reply to Jörg Kubitz from comment #9) > The build command order is set to > <name>org.eclipse.xtext.ui.shared.xtextBuilder</name> > <name>org.eclipse.jdt.core.javabuilder</name> > <name>org.eclipse.pde.ManifestBuilder</name> > <name>org.eclipse.pde.SchemaBuilder</name> > <name>org.eclipse.pde.ds.core.builder</name> > > I don't think that buildorder is wrong: The xtext is supposed to generate > .java code in the src-gen. And the following java builder should compile > those. I believe (I don't know xtext code) the right fix for xtext would be to check xtext builder position in IProject.getBuildConfigs() and if it is on the first place, do not request rebuild just because it has generated something.
i did some experiments jdt build first xtext builder second. when i turn of needs rebuild, jdt wont see java file xtext generates, you wont get class files as jdt wont be called xtext builder first, jdt builder 2nd, xtext using stuff from jdt xtext builder wont be called after jdt builder ran @Andrey: how are these two usecases supposed to work without needsRebuild call.
(In reply to Christian Dietrich from comment #15) > i did some experiments > > jdt build first > xtext builder second. > > when i turn of needs rebuild, jdt wont see java file xtext generates, you > wont get class files as jdt wont be called This is what I've explained why xtext wants a rebuild. > xtext builder first, jdt builder 2nd, xtext using stuff from jdt > xtext builder wont be called after jdt builder ran If Xtext uses jdt stuff, shouldn't it run *after* JDT? This is what I've mentioned before with wrong builder order. The real issue is if *both* xtext and jdt needs output of each other. JDT can't know that there is something missing, but xtext knows about dependencies / not yet executed generation. > @Andrey: how are these two usecases supposed to work without needsRebuild > call. May be xtext should request *another* autobuild round in a different way, not interrupting current run. I believe requestRebuild was implemented to unblock build cycles problem, not for builders generating sources into the project that is currently building. I wonder if we should spend new API for builders doing exactly that - notify about *new* input for *current* project build only. The intent would be to *not* touch rebuildRequested flag by builders that generate something *as input* for the current prokect build, so if xtext does that, it can say "newInputGenerated" and the build for current project only will restart from the first builder.
it is even worse we have usecases (xtend active annotations) where we want to run before AND after jdt
Talking about extreme cases: And we also have 1 project with only xtext builder - not producing .java code - and it still requests rebuild.
yes. Xtext does not know that you dont have a XYZ builder that needs to see its generates.
New Gerrit change created: https://git.eclipse.org/r/c/platform/eclipse.platform.resources/+/191538
(In reply to Christian Dietrich from comment #17) > it is even worse > we have usecases (xtend active annotations) > where we want to run before AND after jdt Cool. But you only have one builder - how you do that? Or is there a second one? (In reply to Eclipse Genie from comment #20) > New Gerrit change created: > https://git.eclipse.org/r/c/platform/eclipse.platform.resources/+/191538 Christian: this would need to be used by Xtext builder *instead* of using needRebuild(). I assume following need to be touched: org.eclipse.xtext.builder.impl.BuildContext.needRebuild() org.eclipse.xtext.builder.impl.XtextBuilder.doBuild(ToBeBuilt, Set<String>, IProgressMonitor, BuildType) org.eclipse.xtext.builder.impl.XtextBuilder.pollQueuedBuildData() WDYT?
there is only one builder. assume we rely on 2nd round
org.eclipse.xtext.builder.impl.BuildContext.needRebuild() org.eclipse.xtext.builder.impl.XtextBuilder.doBuild(ToBeBuilt, Set<String>, IProgressMonitor, BuildType) org.eclipse.xtext.builder.impl.XtextBuilder.pollQueuedBuildData() yes, but i assume we need to use reflection to find out which platform api is available
(In reply to Christian Dietrich from comment #23) > yes, but i assume we need to use reflection to find out which platform api > is available That or a resources bundle version check, but I think there is no other "clean" solution for that. "rebuildRequested" was invented to fix inter-project build cycle issues, it shouldn't be used for internal project rebuild requests. I also see that in our internal product code we also "misuse" needsBuild() just to say that we've changed something that affects *current* project build.
(In reply to Eclipse Genie from comment #20) > New Gerrit change created: > https://git.eclipse.org/r/c/platform/eclipse.platform.resources/+/191538 I generally like to have distinct APIs for distinct needs. However that change does neither fix the progress nor reverts the high max retries from bug 568299+. That changes had been made for the case that a rebuild inside the project should be done but now show negative impact on other use cases of many independent projects.
(In reply to Jörg Kubitz from comment #25) > (In reply to Eclipse Genie from comment #20) > > New Gerrit change created: > > https://git.eclipse.org/r/c/platform/eclipse.platform.resources/+/191538 > > I generally like to have distinct APIs for distinct needs. However that > change does neither fix the progress Progress is the least important issue (compared with build performance), for that I would really like a dedicated change / bug report. Let us concentrate here on "rebuild requested" problem we see with xtext projects. > That changes had been made for the case that a rebuild inside the project > should be done but now show negative impact on other use cases of many > independent projects. This is the reason why there *is* a possibility to switch to the original behavior via system property. If you like, you can propose UI change for that property (so affected users can decide what they would like to see), but I would rather spend time fixing the actual root cause. There is no "silver bullet" today to solve all issues we have with the build, especially with such a "long living" API like resources. Either you have slow full build or slow auto build - reason is the misuse of the original solution for project build cycles. The only solution I see to make everyone happy without extra settings is to separate concerns and provide builders *right* API to notify about generated code.
(In reply to Andrey Loskutov from comment #26) > Progress is the least important issue (compared with build performance), for > that I would really like a dedicated change / bug report. Let us concentrate > here on "rebuild requested" problem we see with xtext projects. I got reports from even bigger workspaces which now have autobuild times of 8-13 min. They do need a progress info. Its no that important but its important and it has the same root cause (Bug 568299). You should just fix that root cause. Full build is not a relevant usecase for autobuild. Developers have much more incremental builds. If they want a full build they do would do a clean build. I took another measurement: For us the vast majority of rebuild request come from "unconfirmedDelta" and as far as i understand we can not solve that with a project specific retry. Well xtext does know which projects it want's to have being rebuild, but rebuilding all over and over with O(n^2) is just bad. > This is the reason why there *is* a possibility to switch to the original So lets revert the default setting to O(n) to make a lot developers happy again.
in case you want to fix andreys patch locally, here is Xtext 2.27.0 snapshots that picks it up. https://github.com/eclipse/xtext-eclipse/compare/cd_needsRebuildRefactoring https://ci.eclipse.org/xtext/job/xtext-umbrella/job/cd_needsRebuildRefactoring/lastSuccessfulBuild/artifact/build/p2-repository/
(In reply to Jörg Kubitz from comment #27) > I took another measurement: For us the vast majority of rebuild request come > from "unconfirmedDelta" What is that and who requests rebuilds in this case? > and as far as i understand we can not solve that > with a project specific retry. Why?
(In reply to Andrey Loskutov from comment #29) > (In reply to Jörg Kubitz from comment #27) > > I took another measurement: For us the vast majority of rebuild request come > > from "unconfirmedDelta" > > What is that and who requests rebuilds in this case? see https://bugs.eclipse.org/bugs/show_bug.cgi?id=579082#c9 case 2. > > and as far as i understand we can not solve that > > with a project specific retry. > > Why? Because xtext detects changes there in projects that may have already build and influence the current build. At least it would require further refactoring in xtext to separate the cases where those deltas are really from another project (which was not the case in the case i debugged).
(In reply to Jörg Kubitz from comment #30) > (In reply to Andrey Loskutov from comment #29) > > (In reply to Jörg Kubitz from comment #27) > > > I took another measurement: For us the vast majority of rebuild request come > > > from "unconfirmedDelta" > > > > What is that and who requests rebuilds in this case? > > see https://bugs.eclipse.org/bugs/show_bug.cgi?id=579082#c9 case 2. > > > > and as far as i understand we can not solve that > > > with a project specific retry. > > > > Why? > > Because xtext detects changes there in projects that may have already build > and influence the current build. At least it would require further > refactoring in xtext to separate the cases where those deltas are really > from another project (which was not the case in the case i debugged). I believe that's same issue with API misuse here. Xtext builder, if wants to generate something for the current project (again), should only request rebuild for the *current* project, it is not a cycle issue with multiple projects. @Christian: the code in org.eclipse.xtext.builder.impl.javasupport.JdtQueuedBuildData.doNeedRebuild() seem to have an issue: it inspects unconfirmedDeltas that is shared between *all* Xtext projects (if I see it right) and decides if a current project rebuild is needed if after processing deltas for the old build state for current project unconfirmedDeltas is not empty. But in the delta can be deltas unrelated to the current project. Let assume there are 200 projects in the queue, and only one put some delta into unconfirmedDeltas - all 199 will request rebuild because of that? Shouldn't it *only* request buildInputChanged() if there are deltas for *current* project left in the unconfirmedDeltas? I haven't checked the git history, for which use case is this code?
btw org.eclipse.core.resources.IncrementalProjectBuilder.needRebuild() does not indicate that it is global
@andrey. i have zero idea how jdt queued build data works. the real main problem is the wired batching of events in jdt that might be the real reason for the wired code.
(In reply to Christian Dietrich from comment #32) > btw org.eclipse.core.resources.IncrementalProjectBuilder.needRebuild() does > not indicate that it is global Yes, it is not clearly documented, because at that time they mostly tried to fix build cycles issues, that also explains why it was only used in outer build loop. See for example comment in build manager: https://git.eclipse.org/c/platform/eclipse.platform.resources.git/tree/bundles/org.eclipse.core.resources/src/org/eclipse/core/internal/events/BuildManager.java#n142
(In reply to Christian Dietrich from comment #33) > @andrey. i have zero idea how jdt queued build data works. > the real main problem is the wired batching of events in jdt that might be > the real reason for the wired code. @Sebastian, you seem to know how QueuedBuildData.java (predecessor of org.eclipse.xtext.builder.impl.javasupport.JdtQueuedBuildData.doNeedRebuild()) is supposed to work: https://github.com/eclipse/xtext-eclipse/commits/805a16f3c497e07f8e4724fde29a7c215c2c8566/plugins/org.eclipse.xtext.builder/src/org/eclipse/xtext/builder/impl/QueuedBuildData.java Reading bug 407161 comment 7 I can't understand the current implementation which is coming basically from this patch: https://github.com/eclipse/xtext-eclipse/commit/c0d5cdb391393bc3bebb809d5cd6b31924dea978 Could you shed a light in context of comment 31? I believe the rebuild is requested too often (for unrelated projects).
(In reply to Christian Dietrich from comment #28) > in case you want to fix andreys patch locally, > here is Xtext 2.27.0 snapshots that picks it up. > > https://github.com/eclipse/xtext-eclipse/compare/cd_needsRebuildRefactoring > https://ci.eclipse.org/xtext/job/xtext-umbrella/job/ > cd_needsRebuildRefactoring/lastSuccessfulBuild/artifact/build/p2-repository/ @Christian: could you please share the link with related Xtext commit? @Jörg: could you please test your config with that Xtext build & patch below? I would like to merge https://git.eclipse.org/r/c/platform/eclipse.platform.resources/+/191538 so we can see early enough if there are other issues with xtext builder.
https://github.com/eclipse/xtext-eclipse/commit/8700323de51e540c8296e07a51867f9395b3ef1f
(In reply to Andrey Loskutov from comment #36) > @Jörg: could you please test your config with that Xtext build & patch below? As far as i understand the xtext changes now assume that only the current project has to be rebuild regardless where the file was generated. We have at least one DSL that generates output in into other projects, that needs to keep working.
(In reply to Jörg Kubitz from comment #38) > (In reply to Andrey Loskutov from comment #36) > > @Jörg: could you please test your config with that Xtext build & patch below? > > As far as i understand the xtext changes now assume that only the current > project has to be rebuild regardless where the file was generated. We have > at least one DSL that generates output in into other projects, that needs to > keep working. This doesn't mean, the patch shouldn't work. If auto-build is on (which is what you have), new generated files in other projects should trigger autobuild anyway for changed projects. But you haven't answered: does it work now with both platform+xtext patches?
(In reply to Andrey Loskutov from comment #39) > This doesn't mean, the patch shouldn't work. If auto-build is on (which is > what you have), new generated files in other projects should trigger > autobuild anyway for changed projects. But only in a second autobuild. Would be kinda frustrating if after an autobuild the next starts without any user changes. That undergoes the idea of requestRebuild retries. > But you haven't answered: does it work now with both platform+xtext patches? i cant test that today.
A few things that are worthy to mention: Xtext seems to be to broad with it's request to rebuild. It's basically requesting a rebuild whenever a file was written by the code generator. Though the JavaDoc of needsRebuild() states: """ Indicates that this builder made changes that affect a build configuration that precedes this build configuration in the currently executing build order, and thus a rebuild will be necessary. """ So *iff* a Java file is written, Xtext could do some special treatment and check the build configuration, since we know some of the builders by name. If there are no unknown builders and the JavaBuilder follows the Xtext builder, needsRebuild could be skipped. This could also work across project boundaries considering the build order of the workspace or the current build configuration, if that is known to the current build run. JdtQueuedBuildData is tricky. I'm pretty sure that this is an example of "architecture by reverse engineering and reflection". It's based on certain assumptions. JDT lacks some information on affected files when the Java compiler runs in batches. There was / is? no reliable API available to tell which types have been rebuilt / gone missing / were added after a build ran - the JavaElementDeltas are sometimes not sufficient. That's why Xtext introspects the internal build state of JDT to figure which names have been changed in addition to a regular change listener. This is supposed to give the correct deltas at the correct point in time - meaning really only after the build ran. It is not generally possible to only observe a the project with the Xtext files since its dependencies might also contain changes that affect the linking of the Xtext files. Generally speaking this is a tough nut to crack without any knowledge of the installed Xtext languages and the present DSL files in the workspace. E.g. all of this could be skipped if no language is interested in any JDT types, but that's impossible to decide generically, e.g. a language might depend on ecore::EObject would could be everything, or some validation might check against Java types. If there is a Java delta present, it might mean that a file was generated, reconciled but not yet compiled by JDT. So a rebuild is requested since some languages (looking at you, Xtend), might need up-to-date class files in their dependent projects, too.
(In reply to Andrey Loskutov from comment #39) > But you haven't answered: does it work now with both platform+xtext patches? It is not working as i expect it to be: after a clean build ("clean all projects") and subsequent file touches the behaviour is still worse then before Bug 568299. I took a measurement of all basicBuildLoop times. Old basicBuildLoop times: 33sec <-clean 266sec 0sec (don't know why it triggered 2 autobuilds, but 0sec don't harm) 1. file touch: 2sec 2. file touch: 2sec With buildInputChanged(): 35sec <- clean 102sec (faster... but what happens after a file touch?) 1. file touch: 137sec (remaining build time of second loop) 16sec (don't know why it triggered 2 autobuilds) 2. file touch: 26sec (still some cycle missing?) 3. file touch: 3sec (still slower) 4. file touch: 3sec @xtext: it feels totally wrong that buildInputChanged() is called while the xtextbuilder is the first builder in our command list. The xtext builder is just called again and does not report the buildInputChanged() on the second run. Also it feels like we do have a cycle between xtext projects which is not reported anymore.
@jörg. if you have any idea on how to make it nicer. please propose. have no idea how to find out which builder we are.
(In reply to Christian Dietrich from comment #43) > @jörg. if you have any idea on how to make it nicer. please propose. have no > idea how to find out which builder we are. BuildManager could offer a int getCurrentCommandNumber() that could serve that purpose like BuildManager.hasBeenBuilt(IProject) does for java. It's more important though that the former requestRebuild() is used when there is a delta detected from a project that BuildManager.hasBeenBuilt(IProject).
I don’t understand. Can you please elaborate
(In reply to Christian Dietrich from comment #45) > I don’t understand. Can you please elaborate If the variable int "i" from BuildManager.basicBuild(IBuildConfiguration, int, IBuildContext, ICommand[], MultiStatus, IProgressMonitor) is not a local variable but a member of BuildManager, then xtext could ask if i==0 and in that case ommit the call to buildInputChanged(). And in org.eclipse.xtext.builder.impl.javasupport.JdtQueuedBuildData.needsRebuild(IProject, Collection<Delta>) if the delta is from a project that already has been build xtext has to call requestRebuild - like java does for build cycles.
(In reply to Jörg Kubitz from comment #42) > @xtext: it feels totally wrong that buildInputChanged() is called while the > xtextbuilder is the first builder in our command list. The xtext builder is > just called again and does not report the buildInputChanged() on the second > run. Why should it report buildInputChanged() on second run? But yes, we might update the patch and just don't re-start loop if the builder that requests buildInputChanged() is the first one. > Also it feels like we do have a cycle between xtext projects which is not > reported anymore. Do you mean, you had a cycle between xtext projects that was reported? How? Or do you mean, something is not built now because some changes were not detected?
i wont have time and free brain to mess with JdtQueuedBuildData for the next weeks and months :(
(In reply to Jörg Kubitz from comment #42) Jörg, from where in the patched xtext do you see the requestRebuild() calls?
(In reply to Andrey Loskutov from comment #49) > (In reply to Jörg Kubitz from comment #42) > > Jörg, from where in the patched xtext do you see the requestRebuild() calls? none! it always calls buildInputChanged() now - and that is wrong. It does not solve the cycle between the projects anymore. For the old behaviour see stacktraces in comment 9.
(In reply to Andrey Loskutov from comment #47) > Why should it report buildInputChanged() on second run? In your usecase - when the xtext builder is set after the java builder and it wants to run the java builder in the same project again because it had created .java files in that project. > Do you mean, you had a cycle between xtext projects that was reported? How? > Or do you mean, something is not built now because some changes were not > detected? yes. there are so many projects, i don't know when exactly it happens - i have only seen changes in the same project on the samples i took. but it has to be such a cycle otherwise the next auto build should be a idempotent 0sec run.
> none! it always calls buildInputChanged() now - and that is wrong. It does not solve the cycle between the projects anymore. i dont understand. i was told never to call needsrebuild but buildInputChanged instead
(In reply to Jörg Kubitz from comment #51) > (In reply to Andrey Loskutov from comment #47) > > Why should it report buildInputChanged() on second run? > > In your usecase - when the xtext builder is set after the java builder and > it wants to run the java builder in the same project again because it had > created .java files in that project. That is clear, this would be the on *first* run. But once we've re-started the loop, why it should trigger buildInputChanged() on *second* run *again*? > yes. there are so many projects, i don't know when exactly it happens - i > have only seen changes in the same project on the samples i took. but it has > to be such a cycle otherwise the next auto build should be a idempotent 0sec > run. Could you trace the projects being built & find out if there is a cycle or not? (In reply to Christian Dietrich from comment #52) > > none! it always calls buildInputChanged() now - and that is wrong. It does not solve the cycle between the projects anymore. > > i dont understand. > i was told never to call needsrebuild but buildInputChanged instead That is OK, but now it seems that there are inter-dependencies or cycles between projects that aren't picked up by the build if only the current project is rebuilt. Jörg, I will push an updated patch set (not the final one), let see if that is something that could improve the autobuild case for you. Ideally you could find the "cycle" root cause and extract some trivial reproducer.
(In reply to Andrey Loskutov from comment #53) > That is clear, this would be the on *first* run. But once we've re-started > the loop, why it should trigger buildInputChanged() on *second* run *again*? It should not. It's just unclear why it requests a second run which then does probably nothing (not even not calling buildInputChanged() anymore). If it realy needs a second run (who knows?) without other commands running in between it could just do that itself. > Could you trace the projects being built & find out if there is a cycle or > not? I can't spend much time on that but surly i will try to find out. - nevertheless xtext needs to keep detecting such. It may not even be a true cyclic dependency but just an invisible dependency which leads to a wrong build order of projects.
i still dont understand the cycle thing. why does it not settle for you. the needsrebuild will be done only if xtext actually generates something
(In reply to Christian Dietrich from comment #55) > i still dont understand the cycle thing. > why does it not settle for you. it does settle - after a few autobuilds. imaging projects p1 and p2 and only xtext builders. let the build order be p1, p2 if now p2 is build and created something for p1 (either in p1 or just p1 may depends on p2), then p1 needs to be build again i.e. requestRebuild() needs to be called.
(In reply to Andrey Loskutov from comment #53) > Jörg, I will push an updated patch set (not the final one), let see if that > is something that could improve the autobuild case for you. The patch still requires Christian's fix in xtext. It also still (but differently) fixes bug 568299 (because it avoids running following builders after input change is detected and gives previous builders chance to react on the change). From xtext point of view no other extra dependencies tracking should be needed, even if it would be much better if xtext could differentiate if it needs a single project rebuild (== buildInputChanged()) or a global rebuild (== needsRebuild()). @Jörg: what will be interesting now, which build types (clean/auto) will show now which behavior/performance if you switch -Dorg.eclipse.core.resources.allowEarlyInnerBuildLoopExit=true on and off, and compare that with the state before bug 568299.
I have update the Xtext cd_needsRebuildRefactoring branch + update site to work with the renamed method
New Gerrit change created: https://git.eclipse.org/r/c/jdt/eclipse.jdt.core/+/191874
Gerrit change https://git.eclipse.org/r/c/platform/eclipse.platform.resources/+/191538 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.resources.git/commit/?id=4c59c9591f2038e4eb4b468ed4391a63655d2345
New Gerrit change created: https://git.eclipse.org/r/c/platform/eclipse.platform.resources/+/191981
Gerrit change https://git.eclipse.org/r/c/platform/eclipse.platform.resources/+/191981 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.resources.git/commit/?id=ee1b5e8180d6d026e35d5550bf262a0d23f3437c
Gerrit change https://git.eclipse.org/r/c/jdt/eclipse.jdt.core/+/191874 was merged to [master]. Commit: http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=114597d3f0dd066cd2612e084b9f072ae996c01c
@Jörg a new Xtext 2.27.0 nightly that picks up the platform change should be visible soon. please give feedback
Xtext patch has a flaw, will update it.
WITHOUT xtext changes: Times for "inner" basisBuildLoop (it would help to have separate names for the two basisBuildLoops - for example "retryloop" for the outer) 29s <-- clean 112s <-- progress OK 123s <-- 100% progress 13s <-- 100% progress 20s <-- 100% progress 0s 0s 0s 1. file touch: 2s <- OK 0s <- WHY? 0s 2. file touch: 2s <- OK 0s So, in this case the autobuild works - only progress indication during full build retries is wrong.
updated Xtext version is available now
(In reply to Jörg Kubitz from comment #66) > 1. file touch: > 2s <- OK > 0s <- WHY? Please update Xtext, this shouldn't happen then. Xtext almost always requested rebuilds.
This fix may well help with Bug 486083 and perhaps Bug 420710
(In reply to Christian Dietrich from comment #67) > updated Xtext version is available now with xtext master/HEAD: 36s <-clean 143s <-progress from 0 to 100 1. file touch: 150s <-probably not enough cycles detected 21s <-another progress from 0 to 100 HUH? probably not enough cycles detected 2. file touch: 29s <-probably not enough cycles detected 3. file touch: 2s <-OK 4. file touch: 2s only 1 basicBuildLoop per touch - good 5. file touch: 9 2s .. That result shows xtext currently does not ask for enough or the right projects to rebuild since 1 clean build is not enough.
i fear we need some more detailed analysis from your side
(In reply to Christian Dietrich from comment #71) > i fear we need some more detailed analysis from your side I had a little look at the xtext sources. Hard to read due to the reflections. As far a i understand xtext always calls requestProjectRebuild without any specific project as parameter. Am i on a wrong branch? You need to pass the projects which contain the delta or in which the files have been added.
no. but this means: rebuild current project.
p.s. do you generate files to other projects in your code. and if yes how?
(In reply to Christian Dietrich from comment #74) > p.s. do you generate files to other projects in your code. and if yes how? yes we do! i don't know exeactly why or how it is done, but xtext used to detect it - see stacktraces in comment 9.
would be nice if you can find out how you do it and what you did customize to make this work.
maybe we can also move this part of the discussion to https://github.com/eclipse/xtext-eclipse/issues/1820
(In reply to Christian Dietrich from comment #76) > would be nice if you can find out how you do it > and what you did customize to make this work. I don't have the time to dig into that. We just need a version that autobuilds at least as good as 4.20 again. Would be nice if you either fix the obvious flaws or just roll back to the working state.
i fear if you dont help us, we cannot test it.
@jörg please check https://ci.eclipse.org/xtext/job/xtext-umbrella/job/cd_xtext_eclipse_issue1820/1/artifact/build/p2-repository/ https://github.com/eclipse/xtext-eclipse/pull/1821 as i dont know whatr you made to make generation to other projects possible, i also dont know if my change will be picked up in your codebase btw what is your github username?
(In reply to Jörg Kubitz from comment #75) > (In reply to Christian Dietrich from comment #74) > > p.s. do you generate files to other projects in your code. and if yes how? > > yes we do! i don't know exeactly why or how it is done The OCL (and QVTd) projects localize their multiple *.mwe2 scripts in a ...build plugin, from which multiple Xtext editors are generated. This has the advantage of reducing the number of projects with an Xtext nature and avoiding including build-only functionality in the deliverable editor plugins. However the *.mwe2 scripts are only invoked manually from launch configurations so they should only initiate a builder loop. The default setting for MWE2 launches is refresh entire workspace. I see that this could be optimized to mention only the generated plugins at the expense of neglecting to update after adding another editor. The OCL (and QVTd) projects continue to use the original Xtext framework and so there are no auto-generated Xtend files that need auto-building; just Java. I suspect that the diverse tools used within the *.mwe2 scripts use a mix of File and IFile creations/writes.
@ed. mwe is a completely different topic. it is involved in build for validation only, not for generation / building at all. feed free to create enhancement requests on github.com/eclipse/mwe
@Jörg: current master of resources code should provide "comparable" behavior as before changes for bug 568299, so I consider this issue as solved (for platform resources) now. Since you have observed/reported multiple issues in this bug, following is what you need to do to make sure builds in your environment will be improved: 1) Please work with Christian on https://github.com/eclipse/xtext-eclipse/issues/1820 to get better understanding where xtext still need an improvement to avoid too many / too less builds in your concrete environment. If that requires another platform API change, create *new* enhancement request for that. 2) Please create new bug for "100% progress on multiple rebuilds", it shouldn't be mixed with bug 568299 or this bug changes.
i see one issue: a builder may call multiple different rebuild request methods (https://github.com/eclipse/xtext-eclipse/commit/fdf2db57542ba377556541829967cb52495c8e97) it's unclear from documentation what happens then. To avoid endless tries to rebuild the current project the buildmanager should continue if ALSO another project was enqueued for build.
(In reply to Jörg Kubitz from comment #84) > i see one issue: a builder may call multiple different rebuild request > methods > (https://github.com/eclipse/xtext-eclipse/commit/ > fdf2db57542ba377556541829967cb52495c8e97) > it's unclear from documentation what happens then. What is exactly not clear? > To avoid endless tries to rebuild the current project the buildmanager > should continue if ALSO another project was enqueued for build. Not sure I understood that, please provide example code / explain use case.
(In reply to Andrey Loskutov from comment #85) > (In reply to Jörg Kubitz from comment #84) > > i see one issue: a builder may call multiple different rebuild request > > methods > > (https://github.com/eclipse/xtext-eclipse/commit/ > > fdf2db57542ba377556541829967cb52495c8e97) > > it's unclear from documentation what happens then. > > What is exactly not clear? > > > To avoid endless tries to rebuild the current project the buildmanager > > should continue if ALSO another project was enqueued for build. > > Not sure I understood that, please provide example code / explain use case. if you have a builder which calls multiple of thesese calls: requestRebuild() requestRebuildOnNextRound() requestRebuild(project, boolean) requestRebuild(projects) it is not documented what happens - and unclear what should happen. especially if one builder calls for immediate rebuild but also for later rebuild - will it immediate rebuild or in the next outer loop?
(In reply to Jörg Kubitz from comment #86) > > Not sure I understood that, please provide example code / explain use case. > > if you have a builder which calls multiple of thesese calls: > > requestRebuild() > requestRebuildOnNextRound() > requestRebuild(project, boolean) > requestRebuild(projects) The four methods are from BuildManager and are NOT API that should be called by clients, so builders can't call these methods at all directly. If you start code complete *in the builder code* there is only two choices: requestProjectRebuild(boolean) requestProjectsRebuild(Collection<IProject>) and both are well documented. > it is not documented what happens - and unclear what should happen. Please be concrete, for me the javadoc for API explains what will be rebuilt and when, so please give a hint what exactly is not clear in the javadoc. If you are about BuildManager non-API, feel free to improve documentation especially if you know *where* is something unclear. > especially if one builder calls for immediate rebuild but also for later > rebuild - will it immediate rebuild or in the next outer loop? The API javadocs for requestProjectRebuild(boolean) and requestProjectsRebuild(Collection<IProject>) should give you an answer. If you call both methods, both rebuilds will happen, there is no mention that one API call is mutually exclusive to some other.
I've been trying the latest builds and using the Xtext Build Console things look pretty good. I haven't experienced any mege-outages, but perhaps I haven't been trying for long enough. The Xtext Build Console shows one trivial anomally. Every Xtext build session in which the indexer does something seems to be followed by a very fast build session that does nothing. The latter seems redundant. The main residual problem is that after I check out a different branch from the OCL GIT the Xtext build takes nearly a minute as all the dependent (unchanged) projects such as UML are re-indexed. The redundant re-indexing without any 'clean' provocation should be avoided. There really should be some way of avoiding indexing files that are expensive and redundant to index.
(In reply to Ed Willink from comment #88) > I've been trying the latest builds and using the Xtext Build Console things > look pretty good. Thanks for monitoring. > The Xtext Build Console shows one trivial anomally. Every Xtext build > session in which the indexer does something seems to be followed by a very > fast build session that does nothing. The latter seems redundant. Sounds like a needRebuild() call somewhere that walks over empty deltas, but also could be something caused by the indexer itself. If that can be reproduced, please report a separated bug. > The main residual problem is that after I check out a different branch from > the OCL GIT the Xtext build takes nearly a minute as all the dependent > (unchanged) projects such as UML are re-indexed. The redundant re-indexing > without any 'clean' provocation should be avoided. There really should be > some way of avoiding indexing files that are expensive and redundant to > index. The indexing is implemented in completely different module. Builder code doesn't even know what index is. So if you can reproduce and the extra indexing seem to be unnecessary, please open a separated bug for JDT core.
(In reply to Andrey Loskutov from comment #89) > Sounds like a needRebuild() call somewhere that walks over empty deltas, but > also could be something caused by the indexer itself. If that can be > reproduced, please report a separated bug. It happens every time. My comments are just a report back of what I currently perceive. I leave it to you and Christian to cherry pick the observations as new / duplicate platform / Xtext issues.
New Gerrit change created: https://git.eclipse.org/r/c/jdt/eclipse.jdt.core/+/192441
Gerrit change https://git.eclipse.org/r/c/jdt/eclipse.jdt.core/+/192441 was merged to [master]. Commit: http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=a47425cbeecfd12f7e2a34a854245ab662311f27
