Community
Participate
Working Groups
Build ID: I20070608-1718 Currently builder actions are run with the workspace root as the scheduling rule. This locks the entire workspace while one project is being built. At a minimum it seems like the rule should be the project being built so you can still work on other projects. But even this it's not ideal. It may make sense for Java, but for other builder types it may not make sense. Our builder, for example, is a make based C++ builder. We're happy to allow users to make changes to project resources even while a build is running in the background. Currently we have no work around for this issue. User builds may take an hour, and right now they have to just go drink a lot of coffee, or switch to another IDE. :) I don't see any way to override the scheduling rule in Eclipse 3.3. org.eclipse.ui.actions.BuildAction just sets it to the default workspace build rule.
*** Bug 203054 has been marked as a duplicate of this bug. ***
This is a complete blocker. I can see that it works really well in Eclipse's Java PDE -- the users can save during a build, cancel the build if they wish and the system will rebuild after the save in the background as appropriate. Furthermore incremental build is so fast that this problem is never really encountered during normal usage However as Warren put it, this can't be appropriate in the case where the build is occurring externally to Eclipse (as in the CDT standard make) and it take more than a few seconds. As Eclipse has no concept of which files are being needed by the build, to simply have the workspace locked is draconian. Just as now there is the ability to Launch while build are ongoing (Window -> Preferences -> Run/Debug -> Launching), the same should be true for editing. The alternative is to use Eclipse purely for editing/debugging and call make from a shell.
Adding John as he is the scheduling rule expert. Could you attach a patch with your suggested changes?
I've made some progress in allowing builders to specify the resources which should be locked during the build (e.g. the derived resources in CDT projects) and plumbed this in the team IResourceRuleFactory. Unfortunately I've run into some problems with the notification mechanism. As the build is currently run with the workspace scheduling rule, changes made to the workspace during PRE & POST_BUILD work correctly as the workspace rule contains all other resource rules. However if the rule for the build operation is changed this is no longer true. Presumably the PRE and POST_BUILD notifications should occur within the build Operation so that the resource deltas continue to be fired as described in: http://www.eclipse.org/articles/Article-Resource-deltas/resource-deltas.html? If anyone has any bright ideas, I'm all ears!
Created attachment 93601 [details] Patch to core.resources for finer grained workspace build Proposed patch against the 3.3 branch to add the ability for the builder to specify the set of resources it would like locked. Defaults to the previous behaviour of locking the workspace. Any feedback greatly appreciated.
Created attachment 93603 [details] Patch to cdt build Patch to cdt standard make to allow standard make targets to only lock derived resources.
Hi James, First of all, thanks for your patches! I applied your patches (93601 and 93603) to my Eclipse 3.3 and CDT 4.0. I was able to save modified files while a target in the "Make Target" view is building and the resource can be from the same project of the build as well. That is really nice! However, if I am doing "Project->Build Project", Eclipse still behaves the old way, i.e. attempt to save a modified resources is queued after the "Build Project" task. Is this what you intended? I also noticed while one target is building, double click on other targets in the "Make Target" has not effect and the target will not be even be enqueued. Is this also an expected behavior? Thanks Maggie
> However, if I am doing "Project->Build Project", Eclipse still behaves the old > way, i.e. attempt to save a modified resources is queued after the "Build > Project" task. Is this what you intended? The main point of the patches was to flesh out what might be the interface for builders to narrow the set of resources locked during the build. As an example, the implementation for the CDT make targets was provided. The default, for all other builders (/ other repository providers) is to return the Workspae scheduling rule as before. I'll need to spend more time looking at the internalBuild code, as one of our developers experienced a deadlock when starting the build while simultaneously attempting to save a file. The problem (I believe) is that: the PRE_BUILD and POST_BUILD handlers are allowed to modify the Workspace, and the build no longer necessarily holds the workspace root (see internalBuild). The workspace tree is locked during the PRE_BUILD notification (though no scheduling ruls is held). The build then attempts to acquire its own scheduling rule which may block if another job is conflicting leading to deadlock. > I also noticed while one target is building, double click on other targets in > the "Make Target" has not effect and the target will not be even be enqueued. > Is this also an expected behavior? This is no doubt a bug. I'll have a look at it when I get a few free cycles. If any of the platfrom guys have any bright ideas, I'd be very interested to hear :)
You can't relax the scheduling rules held during PRE_BUILD and POST_BUILD since this is breaking for build listener implementations. You would need to hold the workspace root scheduling rule during pre-build and post-build events, and then release it during the actual builds.
Created attachment 115244 [details] Work in progress patch 1. getRule added to builders API 2. Project#internalBuild modified to respect the fact that notifications have to be performed within the workspace root rule 3. IModyfyingResourceChangeListener with getRule method added. I think that listeners could also use rules. But this could be a separate bug. The questions is what about Workspace#build. Workspace#build builds all projects sequentially in one thread. I'm not sure if starting a separate job for each build in this case is fine. I'm investigating further.
Created attachment 115421 [details] Work in progress patch 2
(In reply to comment #11) > Created an attachment (id=115421) [details] > Work in progress patch 2 It's nice to get some attention on this :) Presuambly you still need to keep the ResourceRuleFactory involved as well? I made some further progress as per john's comment 9. Going from workspace lock to fewer locks is striaght-forward. However at the end of the build re-acquiring the workspace lock for the POST_BUILD event appears to be impossible in the general case without releasing all the currently held locks... (Although, if you ignore other builders, you may be able to acquire the workspace lock without releasing the currently held locks due to Eclipse's requirement that all other consumers must always grap outer locks first...) If you release currently held locks then you let in a bunch of other changes to the tree, although maybe this isn't such a big deal?
(In reply to comment #12) > It's nice to get some attention on this :) Presuambly you still need to keep > the ResourceRuleFactory involved as well? PRE_BUILD and POST_BUILD notifications require worksapce root rule, so I don;t think that we should modify ResourceRuleFactory. This could be done if the rule for whole build operation is narrowed. To achieve this we would have to introduce rules for notifications as well. > you release currently held locks then you let in a bunch of other changes to > the tree, although maybe this isn't such a big deal? Because we allow for concurrent modifications and some changes can be done concurrently with a build, POST_BUILD delta can carry other changes done during the build. This will have to be added to javadoc.
Created attachment 115566 [details] Work in progress patch 3 (to release)
I'm going to release the latest patch with the next N-build. Two issues two investigate and address: - BuildAction should not use build rule - BuildManager#build(int, IProgressMonitor) should not run builds sequentially, but concurrently
For the Java Builder to allow projects to be built concurrently or even to allow changes during a build loop, we need to ensure that prereq projects remain stable & consistent while each dependent project is built. The best way to think of it is: we see the workspace as layers of projects. Layer 0 has a few projects with no dependents (ex. Kernel) & Layer 1 has some projects which only depend on layer 0 projects, and so on. We need a rule that ensures that Kernel is built before any other project/layer, and when layer 1 projects are built concurrently we need to ensure that no .class/jar changes happen to Kernel. Its ok if source level changes happen to some of Kernel's classes since the next build loop will handle those changes. I'm not convinced that simple project level locks, or even read locks on prereq projects can capture the real dependencies between projects well enough for us to make use of this change. In other words... when a user changes a source or .class file in a project, can a rule know that the source change can happen now, but the .class or jar file change must wait for the build to finish?
(In reply to comment #16 > The best way to think of it is: we see the workspace as layers of projects. > Layer 0 has a few projects with no dependents (ex. Kernel) & Layer 1 has some > projects which only depend on layer 0 projects, and so on. So the rule for projects from the kernel could lock only kernel projects. The rule for projects from the level 1 would lock the project and kernel and so on. > I'm not convinced that simple project level locks, or even read locks on prereq > projects can capture the real dependencies between projects well enough for us > to make use of this change. Of course we can always lock whole workspace for java builders. Or the rule for java builders can be "all java projects". > In other words... when a user changes a source or .class file in a project, can > a rule know that the source change can happen now, but the .class or jar file > change must wait for the build to finish? I'm not following this. I would explain how the rules are going to be used during builds: - workspace build - the build loop is run within workspace rule and rules would help us to run particular builds concurrently, not sequentially (the current build order) - project build - a user wants to build just one project. So the rule will lock all needed projects. In java builder the rule could be all prereqs and the project.
Try this again - Say my workspace has 3 projects. Projects B & C depend on project A. Now what happens when builders for projects B & C are running concurrently and a user tries to change a source file in project A ? To build projects B & C concurrently, project A must remain consistent until both B & C are finished (C could start 1 second later but finish 2 minutes after B). For some languages & project contents, its possible that certain files can be updated without impacting another project's build result (in the case of Java, source files in project A can be changed & project B or C will not notice/care since they only use A's .class files & jars). So if project B and C both acquire a read lock on project A to ensure it doesn't change & then release it when they're done, how do we allow source file changes to project A when B and/or C hold read locks ? Can the build rule be smart enough (and specific to each builder/language) to allow some file changes to proceed, but suspend others until the build is finished ?
(In reply to comment #18) > Try this again - Say my workspace has 3 projects. Projects B & C depend on > project A. > > Now what happens when builders for projects B & C are running concurrently and > a user tries to change a source file in project A ? > > To build projects B & C concurrently, project A must remain consistent until > both B & C are finished (C could start 1 second later but finish 2 minutes > after B). > > For some languages & project contents, its possible that certain files can be > updated without impacting another project's build result (in the case of Java, > source files in project A can be changed & project B or C will not notice/care > since they only use A's .class files & jars). The rule doesn't have to lock whole projects. For this particular case, the rule used to build B would be "the project B and .class files and jars", not "the project B and A". So source files could be modified by other operations, however the build for project A would be delayed since its rule would probably require whole project A. > > Can the build rule be smart enough (and specific to each builder/language) to > allow some file changes to proceed, but suspend others until the build is > finished ? > As in my example above, the scheduling rule can lock not only projects, but folders and files too. So for the java builder this would be the project being built + folders with binaries. What do you think?
Kent has a good point that a single scheduling rule isn't rich enough to maximize concurrency. The case of two projects B & C depending on a third project A is quite common. In this case the builders on B and C could theoretically run at the same time, but they both require that A not be modified (I'm simplifying a bit from what Kent said). What we need for this case is for the builder to give two scheduling rules: "the stuff I want to modify" (write lock), and "the stuff I don't want to be changed while I'm building" (read lock). Overlapping read locks would be allowed, but write locks would not. The above example would work as follows: Builder on B would have write lock on B and read lock on A Builder on C would have write lock on C and read lock on A -> Builders on B and C can run at the same time This could be implemented by having the thread running the BuildManager hold all the read locks, and each builder run in a separate job with its own write lock. Kent: I realize you could go a step further and only lock the class files from project A in this case, but I think the implementation of that would be quite complex. In the case of output folders overlapping the input folder, you would have to traverse and gather all the individual class files to lock. I suspect the complexity of doing this could negate the performance improvement from the concurrency.
Some other notes: - What do we do when there is an explicit build order set? One option to maintain compatibility is that we would resort to a sequential build if there is an explicit build order. The most common case is that there is no explicit build order so this may be good enough. - Builders don't receive deltas for changes that occurred while they were running. This means they won't receive deltas for changes made concurrently by other builders. This is likely ok, since they presumably don't care about changes in resources that they aren't explicitly "locking", but will need to be specified.
We (Douglas Pollock + Zorzella) have been trying to work on a similar patch (https://bugs.eclipse.org/bugs/show_bug.cgi?id=234744), with Java as our primary target. Our rationale was that there is no need to grab a resource lock in the builder at all. We created a parallel lock, called a build lock. This makes it so that only one build job can run at any given time. Then when writes have to occur (say the build needs to write a .class, or a generated .java file), they are communicated back to the workspace via asynchronous worker jobs that possess a workspace lock. These writes happen "immediately" (are not blocked), since the build job doesn't hold the workspace lock. As for the consistency of the Java build, our argument was that the currently running Java build need not be consistent. If a write occurs during the build, then a new build will be queued. It cannot start running until the build lock is released. At the end of the first build, the second build will start. This continues until the builds catch up with the user. Basically, a user editing while a large build is in progress does not expect a consistent workspace until all builds are finished. The thing we're considering now (and the reason this bug here was brought to our attention) is the PRE and POST BUILD listeners; but we'd love to know if there's anything wrong with our rationale, etc.
*** Bug 234744 has been marked as a duplicate of this bug. ***
It seems like "patch 3" has been committed. Is this bug now considered fixed or is there more work ongoing?
(In reply to comment #24) > It seems like "patch 3" has been committed. Is this bug now considered fixed > or is there more work ongoing? Yes, it has. However the work is not over. The patch 3 works for very simple cases, but for builders like java builders we have to go a bit further (see comments 16 - 21). (In reply to comment #22) > The thing we're considering now (and the reason this bug here was brought to > our attention) is the PRE and POST BUILD listeners; but we'd love to know if > there's anything wrong with our rationale, etc. Right. PRE and POST_BUILD notifications lock the workspace regardless of whether there are any listeners, or the listeners need the workspace locked. However the time spent in listeners is usually very short, so I'm not sure if any changes here will improve the user experience. We could open a bug for listeners, if you want to work on that.
I noticed while debugging something else that the currently committed code isn't working as expected. Since it wraps the entire build in a call to IWorkspace#run, it has the workspace root scheduling rule for the entire duration, regardless of what the builders specify as their scheduling rule. I'm not sure what this outer IWorkspaceRunnable is for.
(In reply to comment #26) > I noticed while debugging something else that the currently committed code > isn't working as expected. Since it wraps the entire build in a call to > IWorkspace#run, it has the workspace root scheduling rule for the entire > duration, regardless of what the builders specify as their scheduling rule. I'm > not sure what this outer IWorkspaceRunnable is for. > I think that this could be ui BuildAction. It just uses the workspace root rule. I haven't changed that yet. I'll do when I continue the work.
I am referring to Project.internalBuild: protected void internalBuild(final int trigger, final String builderName, final Map args, IProgressMonitor monitor) throws CoreException { workspace.run(new IWorkspaceRunnable() { ... }, monitor); }
Does this mean we have to audit the call hierarchy to make sure there are no Workspace.run() calls?
What about a different approach to the problem? How command line build works "traditionally" --------------------------------------------- When the build takes long, many users run the make on the command line and don't care about the eclipse build. This is how development with make based systems worked for a long time. You build on the command line and you can continue editing with your editor. In case you changed a file during the build, you might cancel the build and restart it or you rerun the build after it finished. You can work this way with eclipse, all you have to do is a refresh after the build. Could we somehow integrate this approach into eclipse? Observation ----------- During the 1h build, the first 59:50 no eclipse resource changes happen. Only at the end a refresh happens and eclipse finds the changed resources. Most files make (re)generates are binary files or in general files the user is not supposed to edit. And the workspace lock is basically needed to ensure integrity of the eclipse resource system. Food for thought ---------------- Here is the basic idea: Suppose, the make file build would be split in two eclipse builds. When the build starts, make (=the external build command) is launched and the eclipse build finishes immediately. That is: it "fakes" that is is done very quickly. When the make command finished, another eclipse build (or rebuild) is triggered. It again does nothing (and finishes very quickly). But this time eclipse will find all the changed resources and do the refresh. ==> instead of holding the lock during the entire build, we hold the lock at the beginning and the end of the build for a short time. This is not the full solution (I have not thought about all details and implications of this approach). But I wonder if something along those lines could help....
Michael, the idea that, at the end of a build, another one might be needed to address changes that occurred to files while the first build was taking place is at the heart of our patch, found at: https://bugs.eclipse.org/bugs/show_bug.cgi?id=234744 What that patch does is to have the build rule not hold a workspace lock, but a separate "build" lock. This way, a "save" is not blocked, but it triggers a second build. Since that second build also needs to acquire a "build" lock, it is blocked until the first build finishes. In other words, it is queued, which is just what will (in my mind) guarantee that, eventually, the final compilation will be consistent with the final state of the source files. Does that make sense?
(In reply to comment #31) > What that patch does is to have the build rule not hold a workspace lock, but a > separate "build" lock. This way, a "save" is not blocked, but it triggers a > second build. Since that second build also needs to acquire a "build" lock, it > is blocked until the first build finishes. In other words, it is queued, which > is just what will (in my mind) guarantee that, eventually, the final > compilation will be consistent with the final state of the source files. This of course fits with how resource locking works in general, see: http://www.eclipse.org/articles/Article-Concurrency/jobs-api.html Example 3 - Read-access to Resources An alternative approach is that the builder listens for the post build resource delta and schedules an incremental build of the changed source files. Either way, you're right, locking the build output directory will have the desired effect of preventing further builds from occurring concurrently.
(In reply to comment #28) > I am referring to Project.internalBuild: John is right. There doesn't appear to be any additional concurrency in the currently checked-in Project.internalBuild(). I'm still a bit confused / concerned that that the PRE_BUILD/POST_BUILD resource deltas are no longer well defined. Maybe this doesn't matter, but previously, as a result of running with the workspace rule, PRE_BUILD was always followed by a POST_BUILD. Now we can presumably have POST_CHANGE notifications happening during the build. It also appears possible that multiple PRE_BUILD notifications are fired before the first POST_BUILD notification. The actual contents of the PRE_BUILD & POST_BUILD resource deltas may therefore be pretty meaningless (as the resource deltas seem to be calculated from the last POST_CHANGE fired). This seems largely different to how the events are defined in the article: http://www.eclipse.org/articles/Article-Resource-deltas/resource-deltas.html Am I wrong on this; is this important?
I think there may be some additional issues in the current implementation: - IncrementalBuilder.getDelta(...) may no longer do the right thing. The delta from the last build is calculated in BuildManager.getDelta() using the workspace tree (currentTree) set when basicBuild(...) was called. BuildManager is currently singleton as far as the workspace is concerned, so currentTree may well be updated by a subsequent build before the current build has done getDelta(). [ BuildManager has the following temporary variables: //the following four fields only apply for the lifetime of a single builder invocation. protected InternalBuilder currentBuilder; private DeltaDataTree currentDelta; private ElementTree currentLastBuiltTree; private ElementTree currentTree; ] - Project.internalBuild() uses a MultiRule of all the builder commands set on the Project. If any of the builders on the project use the default WorkspaceRoot rule, there is no benefit. - There's a misleading comment in the BuildManager.getRule() - presumably left over from my patch partially applied: "// Returns the derived resources for the specified builderName" It also seems that getDelta() isn't valid when getRule() is called. It's probably desirable that this works given that the scheduling rule returned by the builder must contain resource rules for resources the builder intends to modify.
Regarding the issues mentioned here: (In reply to comment #28) > I am referring to Project.internalBuild: > > protected void internalBuild(final int trigger, final String builderName, final > Map args, IProgressMonitor monitor) throws CoreException { > > workspace.run(new IWorkspaceRunnable() { > ... > }, monitor); > } > I've tried to get rid of the workspace.run, as the defaultRule of workspace.run would also be IWorkspaceRoot, e.g. the whole ws would be locked. After this I encountered that the build job, created from the according Build Action (my own code, but similar to the platform), needs to set the rule accordingly. This means that either no rule needs to be specified or a special rule needs to be created, that allows the occuring locks. This is that in the internalBuild method, some PRE and POST build operations are done with the default build rule. If the outer rule set in the build-job is not aware of this an according Error is shown. I then did some testing with no rule in the job itself and a simple project lock returned by my builder, e.g. only the C-Project built was locked. With this I was able to modify other projects and save files there. I think this is the main issue to be solved in this request, but I additionally found that if I try to save a file from the locked project, the dialog showing the blocking jobs is popped up and this one is modal, so unless you either cancel the save or the blocking build all other actions in the ui are blocked. In general I think the proposal to let the builder specify the lock is the apropriate way to go, because then the implementing developer has the control, but there's much more to prepare for correct function.
(In reply to comment #35) > After this I encountered that the build job, created from the according Build > Action (my own code, but similar to the platform), needs to set the rule > accordingly. This means that either no rule needs to be specified or a special > rule needs to be created, that allows the occuring locks. This is that in the > internalBuild method, some PRE and POST build operations are done with the > default build rule. If the outer rule set in the build-job is not aware of this > an according Error is shown. The BuildAction should use a null rule as Szymon pointed out in Comment 27. > I then did some testing with no rule in the job itself and a simple project > lock returned by my builder, e.g. only the C-Project built was locked. With > this I was able to modify other projects and save files there. > I think this is the main issue to be solved in this request, but I additionally > found that if I try to save a file from the locked project, the dialog showing > the blocking jobs is popped up and this one is modal, so unless you either > cancel the save or the blocking build all other actions in the ui are blocked. You should return a rule to lock the resources your builder is going to modify. Resources only read by your builder (such as source files) don't need to be locked (a la IFile#getContents()). I think there are still some issues with the builder resource delta that need to be resolved (comment 34).
(In reply to comment #36) > The BuildAction should use a null rule as Szymon pointed out in Comment 27. This is what I did, but If someone wants to trigger only one build at a time, there needs to be some "singleton" rule, thats aware of the happening locks. > You should return a rule to lock the resources your builder is going to modify. > Resources only read by your builder (such as source files) don't need to be > locked (a la IFile#getContents()). > Hmm, I'd say that during a build I wouldn't like to allow modification of the source code, although the files are not modified by the build, but read. But once I encounter any blocking state the according dialog is modal and unless I either wait for completion or cancel one of those blocking jobs, I'm not able to work on any other stuff. If the dialog wasn't modal, I'd be able to continue work and let the locking stuff continue and finish accordingly. > I think there are still some issues with the builder resource delta that need > to be resolved (comment 34). > I just wanted to point out that there could even be some more issues.
(In reply to comment #37) > Hmm, I'd say that during a build I wouldn't like to allow modification of the > source code, although the files are not modified by the build, but read. Why not? Yes the compile might fail, but with resource delta notifications we know to rebuild the particular modified source file in a subsequent build Job. Fixing this offers the possibility that builders like CDT's managed make, are similar in apparent performance to the Java Builder, even if invoking the compiler takes seconds not milliseconds. If we lock the source tree in CDT there's no way auto-build can ever work responsively. Clearly what's locked is builder implementation specific. But in the CDT managed make world, we should aim to approach the interactive performance of Java.
> Why not? Yes the compile might fail, but with resource delta notifications we > know to rebuild the particular modified source file in a subsequent build Job. Builders don't receive resource deltas for changes that occurred while they were running (otherwise the build cycle would loop).
(In reply to comment #39) > > Why not? Yes the compile might fail, but with resource delta notifications we > > know to rebuild the particular modified source file in a subsequent build Job. > > Builders don't receive resource deltas for changes that occurred while they > were running (otherwise the build cycle would loop). Indeed. But other parts of the system do receive post change events. One would hope that IncrementalProjectBuilder#getDelta(IProject) would continue to provide resources changed since the last time the builder was run. Given that the builder has locked the resources it's changing (i.e. its output) one would hope it's possible for core.resources to keep the builder delta separate from deltas created by other Jobs running concurrently. The JavaDoc for the IncrementalProjectBuilder#getDelta says: * Returns the resource delta recording the changes in the given project * since the last time this builder was run. <code>null</code> is returned * if no such delta is available. An empty delta is returned if no changes * have occurred, or if deltas are not applicable for the current build kind. * If <code>null</code> is returned, clients should assume * that unspecified changes have occurred and take the appropriate action. As far as I can see it's this additional state that makes the builder more than just a common WorkspaceJob which the integrator schedules when appropriate. After all it would be possible for integrators to re-implement the functionality provided by the builder using resource listeners and their own scheduling rules... Does this make sense?
(In reply to comment #40) > (In reply to comment #39) > > > Why not? Yes the compile might fail, but with resource delta notifications we > > > know to rebuild the particular modified source file in a subsequent build Job. > > > > Builders don't receive resource deltas for changes that occurred while they > > were running (otherwise the build cycle would loop). To respond to this point directly, when the user has auto-build switched on, saving a file causes a build to be scheduled. Presumably this is the indirect result of a resource delta. You might imagine that this would continue to work in the new world. There could be a mechanism to ensure that only one autobuild was queued for the Project to prevent multiple saves from triggering multiple builds. Given that you wrote this stuff and the documenation explaining it (http://www.eclipse.org/articles/Article-Builders/builders.html) please do correct me if I'm wrong on any of the above!
> one would > hope it's possible for core.resources to keep the builder delta separate from > deltas created by other Jobs running concurrently. You can hope, but that won't make it true ;) The delta mechanism (for either builders or change events) currently doesn't record what thread each change occurred in. I'm not even sure that would be enough - for example the Java builder is implemented as several threads operating in tandem.
I didn't follow the entire thread here, but looking at the target milestone ... and the fact that API freeze has passed now... is it still realistic to get this request into 3.5 ?
Some part of the work was released. We added IncrementalProjectBuilder#getRule that is respected during a project build. I plan to work on that during M7, however I think that there is more to discuss right now that to work or implement. All reasonable changes made during M7 which are not API changes will be released to 3.5. Other changes will be postponed to 3.6.
No further work planned for 3.5. Will be continued in 3.6.
*** Bug 290922 has been marked as a duplicate of this bug. ***
I just stumbled over this again... and would like to mention that this issue is one of the biggest adoption problems of Eclipse in the C/C++ world, where builds often take hours (as mentioned in the description and comment 2). Even the simplest editors and IDEs (Emacs, Slickedit, Devstudio, ...) all support asynchronous builds to proceed while still browsing / editing files. Getting this issue resolved in 3.6 would be a huge step forward for Eclipse in the C/C++ space, along with the flexible project improvements contributed by Serge via e4. I'm wondering whether this could be promoted to a plan item? To reiterate on the original description, the goal of this is allowing the following acceptance criteria: (1) an asynchronous build to run in background while still editing files (2) all methods of invoking a build to respect the sync/async property (autobuild, kbd shortcut, menu item, toolbar, launch, dependency) (3) build-before-launch to wait for the build (although that is in background) (4) workspace-build to wait and sequentially build projects (potentially in background without locking the ws) While (1) is easy to achieve with an "external tool" kind of build, achieving the other criteria seems impossible for Eclipse extenders without having the Platform changed. One idea I had was that a builder could report two different kinds of "done" events: (a) "done preparing" in the sense that the workspace may be unlocked, (b) "done building" in the sense that launches / dependent builds can proceed.
(In reply to comment #47) > Getting this issue resolved in 3.6 would be a huge step forward for Eclipse in > the C/C++ space, along with the flexible project improvements contributed by > Serge via e4. I'm wondering whether this could be promoted to a plan item? There is a lot of effort still to do about Serge stuff. Regarding this and current plan items, there is not too much time left for other items. We could discuss this item right when Bug 284148 is fixed along with related issues. Serge help with e4 stuff would probably speed it up.
(In reply to comment #26) > I noticed while debugging something else that the currently committed code > isn't working as expected. Since it wraps the entire build in a call to > IWorkspace#run, it has the workspace root scheduling rule for the entire > duration, regardless of what the builders specify as their scheduling rule. I'm > not sure what this outer IWorkspaceRunnable is for. May I request a fix for the above and for BuildAction to not enforce a workspace rule when running a build? I think this is the bare minimum to make the new IncrementalBuilder.getRule() API of any use for clients.
Hi Toni, (In reply to comment #49) > May I request a fix for the above and for BuildAction to not enforce a > workspace rule when running a build? > I think this is the bare minimum to make the new IncrementalBuilder.getRule() > API of any use for clients. This isn't quite sufficient: Project#internalBuild(...) wraps the build in a workspace.run(...) which uses the workspace rule. I think there are other issues which need to be tackled before these changes should be exposed. The major issue is that IncrementalProjectBuilder#getDelta(IProject) is no longer well defined. Previously it returned the delta since last build. As changes can be made concurrently this will be unreliable for builders. This could be fixed in a couple ways: 1) Snapshot the workspace tree before build. #getDelta() would return the resource delta'd from this snapshot excluding any resources covered by the builder's scheduling rule 2) Change the semantics of this to return the delta from start of last build. Leave it up to the builder to ignore derived resources that it operates on.
(In reply to comment #50) > This isn't quite sufficient: Project#internalBuild(...) wraps the build in a > workspace.run(...) which uses the workspace rule. This is exactly what I meant by referring to comment 49. The IWorkspaceRunnable should be removed or it should use a null scheduling rule. > I think there are other issues which need to be tackled before these changes > should be exposed. The major issue is that > IncrementalProjectBuilder#getDelta(IProject) is no longer well defined. > Previously it returned the delta since last build. As changes can be made > concurrently this will be unreliable for builders. This should be noted in the Javadoc, but allowing a different scheduling rule would still make sense for builders which don't depend on the correctness of the delta. Or am I missing something?
(In reply to comment #51) > This is exactly what I meant by referring to comment 49. Sorry, I misread your comment. > This should be noted in the Javadoc, but allowing a different scheduling rule > would still make sense for builders which don't depend on the correctness of > the delta. Or am I missing something? In my mind a builder is a Job with benefits. One of the main reasons for using it rather than a Job+scheduling rule is resource deltas. You also get automatic scheduling (if you want it), as well as callbacks from build actions and build before launch. If the resource delta doesn't continue to work, to me that's the major builder functionality removed. The platform also only schedules incremental builds if there's been a resource delta. If the change happened concurrently with the previous build then a subsequent build won't be triggered. I guess I'd turn the question on its head: Is there much the platform build support gives you if you don't need resource deltas? In CDT, for example, it seems that the make target building is a good place where simple Jobs should be used rather than wrapping the arguments, calling the project build, unwrapping them and building. For make targets I don't think we use any of the platform builder added value...
Created attachment 153578 [details] Patch to enable custom builder rule This patch contains the suggested changes to Project#internalBuild() and BuildAction#run(). My assumption is that this does not break any builder which uses the workspace root as its scheduling rule.
To make it very clear: 1. The small patch as just attached by Toni enables our commercial product to provide functionality which is very important for us (allow editing files while a slow external Makefile build is ongoing -- even if that Makefile build is initiated through autobuild or build-before-launch). 2. The patch does not break any existing functionality, since all builders known today use the Workspace root as scheduling rule. The new API IncrementalProjectBuilder#getRule() has only been added in 3.5 and fact is that it does not work properly as of today, so no clients are expected to use it yet. 3. Documenting the limitations of IncrementalProjectBuilder#getRule() in its Javadocs should be easy, and thus "complete" the work. I cannot think of any arguments against integrating Toni's patch into Eclipse HEAD, and if nobody speaks up mentioning any problems I'm going to commit the "core" part of Toni's patch to HEAD tomorrow (Thursday). I cannot commit the "UI" part of the patch, but that part only fixes the main menu build action in the UI, in the worst case we could live without that, and Toni's patch does not break anything if only the core part is applied. Toni, could you please draft a 2nd patch which updates the API docs for IncrementalProjectBuilder#getRule() such that James and other committers can review the new docs and suggest changes. Thanks!
I think I want this as much as you do. I'm just worried that this introduces bugs in the build lifecycle... (In reply to comment #54) > 1. ... > Makefile build is initiated through autobuild or build-before-launch). Is that going to work if core.resources thinks there's no delta since end of last build? > I cannot commit the "UI" part of the patch, but that part only fixes the main > menu build action in the UI, in the worst case we could live without that, and > Toni's patch does not break anything if only the core part is applied. As it happens we (CDT) register our own BuildAction to handle building of referenced project configurations correctly. So this should work in the CDT perspective even without platform support (just override run() in BuildGroup.java: CDTBuildAction).
(In reply to comment #55) > Is that going to work if core.resources thinks there's no delta since end of > last build? Hi James, can you be a little bit more specific about potential end user cases that might fail? - This would probably help us come up with test cases for this. Right now, I interpret your concern as: "If autobuild is ON and I change a file of project P while build is ongoing, today I cannot save the file (due to workspace lock). With the change, I will be allowed to save the file but it is not clearly determined whether the ongoing build will use the old or the new version of the file. Therefore, the builder must return all deltas since BEGIN of the build, such that autobuild re-runs once the current build is done" Does that cover your usage scenario / concern? Are there any other potential test cases that you could dream up?
(In reply to comment #54) > 1. The small patch as just attached by Toni enables our commercial product > to provide functionality which is very important for us (allow editing > files while a slow external Makefile build is ongoing -- even if that > Makefile build is initiated through autobuild or build-before-launch). Autobuild (and any workspace-wide build) still aquires a lock on the workspace root, but I think that's acceptable for our use case.
Thanks Martin. (In reply to comment #56) > (In reply to comment #55) > Hi James, can you be a little bit more specific about potential end user cases > that might fail? - This would probably help us come up with test cases for > this. Right now, I interpret your concern as: > > "If autobuild is ON and I change a file of project P while build is ongoing, > today I cannot save the file (due to workspace lock). With the change, I will > be allowed to save the file but it is not clearly determined whether the > ongoing build will use the old or the new version of the file. Therefore, the > builder must return all deltas since BEGIN of the build, such that autobuild > re-runs once the current build is done" > > Does that cover your usage scenario / concern? > Are there any other potential test cases that you could dream up? That's one of them, yes. In code terms: BuildManager#needsBuild(...) is called during basicBuild to work out whether build should proceed. This returns false if there's no change between the workspace tree and the last build tree. 2) Up till now singleton BuildManager has assumed that only build is taking place at once. With relaxed scheduling rules, once the workspace lock is released (when the build is run) other builds with non-conflicting scheduling rules can start. To me it looks like BuildManager will just break: protected InternalBuilder currentBuilder; private DeltaDataTree currentDelta; private ElementTree currentLastBuiltTree; private ElementTree currentTree; If more than one build enter basicBuild, it looks like the build tree state will be lost, and deltas may be way out of whack, perhaps so much so that some post change deltas are lost altogether... I think resolving this bug requires more than just changing the locking. Relaxing this without enhancing BuildManager will quickly break when there's more than one builder supporting reduced scheduling rules. Won't it?
(In reply to comment #58) > That's one of them, yes. In code terms: > BuildManager#needsBuild(...) is called during basicBuild to work out whether > build should proceed. This returns false if there's no change between the > workspace tree and the last build tree. In this case the builder should set the "callOnEmptyDelta" flag. This can be added to the Javadoc of IncrementalProjectBuilder#getRule(). > 2) Up till now singleton BuildManager has assumed that only build is taking > place at once. With relaxed scheduling rules, once the workspace lock is > released (when the build is run) other builds with non-conflicting scheduling > rules can start. I agree that there must not be more than one build at a time. But is this possible? - BuildManager protects itself by testing and setting the "building" flag. If a build is ongoing any other build request is ignored. This might not be the best solution but it prevents data corruption. - Every build must go through an at least temporary lock of workspace root. And the builder scheduling rule must be contained in the workspace root rule (and thus conflict with it) anyway. Thus any other build is blocked while a build is ongoing. There might be a slight chance of a race condition, though, when switching the scheduling rule.
Thanks Toni! (In reply to comment #59) > In this case the builder should set the "callOnEmptyDelta" flag. This can be > added to the Javadoc of IncrementalProjectBuilder#getRule(). Sounds reasonable. > - BuildManager protects itself by testing and setting the "building" flag. Ah, missed that, you're right. That address my major concern. There's a slight oddity here though, if a caller requests a build using the API and one is already taking place with non-conflicting scheduling rule. The build will return immediately Status.OK_STATUS rather than blocking on the in progress build (as currently happens): if (!canRun(trigger)) return Status.OK_STATUS; > - Every build must go through an at least temporary lock of workspace root. Yep. I think it's safe given that this flag is updated while the workspace lock is held (the ws lock is released just before calling the builder). Given the above, I'm happy with your patch.
Created attachment 153717 [details] Improve javadoc of InrementalProjectBuilder#getRule() This is my attempt at describing the existing caveats when overriding getRule(). Suggestions are welcome!
(In reply to comment #61) Thanks Toni! - Here are my suggestions: * line 231 -- clarify like this: If {@link #getRule()} is overridden to return a scheduling rule other than the workspace root, the delta returned by this method may be incomplete since the builder cannot control changes outside its scheduling rule. Note: I assume the reason for this limitation is that builders with a "small" schedulingRule cannot control what changes are applied outside their schedulingRule while the build is ongoing. Would it make sense to recommend builders with non-wsroot-schedulingRule to return null here? Since a potentially incomplete delta is not of much value to anyone anyways, right? * line 355 ff -- I agree with the content, but I suggest being a bit more verbose. For instance: The scheduling rule determines which resources in the workspace are protected from being modified while the builder is running. Up until Eclipse 3.5, the entire workspace was always locked during a build; since Eclipse 3.6, builders can allow resources outside their scheduling rule to be modified. If this method returns any rule other than the workspace root, resources can be modified concurrently to the build. Therefore, the delta returned by {@link #getDelta(IProject)} might be incomplete. The builder should be configured with the "callOnEmptyDelta" option enabled in this case. This will ensure that the builder is called even if the delta appears to be empty. Aside my proposed changes, a native English speaker should review the text (James?). For tracking and IP logging purposes, can you create one combined patch that contains your modifications (the one-line code change plus the change to API docs) and have that one obsolete the other two patches?
(In reply to comment #60) > Given the above, I'm happy with your patch. Thanks James. I just completed the core.resources "AutomatedTests" based on I20091201-1600 with Toni's patch applied, on Windows XP and Linux x86_64 (1011 Unittests). I found 5 test failures (all in IContentTypeManagerTest) related to bundle resolving -- I don't think that any of these are related to Toni's change but I am investigating. I think the important point to note is that there are 2 independent work items: (a) Prepare the Eclipse Framework to *allow* builders run with a scheduling rule they provide. This is what this bug is about -- most of the work has been done by Szymon, and Toni is adding the last missing piece. (b) Understand the constraints under which a builder which leverages the new feature from (a) can operate reliably, teach clients how to write such reliable code and potentially improve the framework to make it easier for clients to write reliable code that leverages the new feature. For us at Wind River, it currently looks like we can live with the known constraints of (b) e.g. Autobuild always locking the workspace (BTW that would be another interesting item to add to the Javadocs). The current changes do not break existing functionality as long as the feature is not exercised, and the changes do allow clients to leverage / experiment with the new feature. In this sense, I would suggest declaring this bug as FIXED with Toni's changes and follow-up on potential improvements for (b) in a separate bug.
Created attachment 153723 [details] Combined patch for core.resources This incorporates the proposed Javadoc changes and also adds a section about the limitation in case of autobuild or "Build All".
(In reply to comment #64) > This incorporates the proposed Javadoc changes and also adds a section about Thanks Toni. For completeness, could you please add a Contributor line to the copyright comments of the 2 affected files, and attach the patch again -- then the patch will match what we have in CVS. I have found 2 more test failures on Linux 32-bit (RHEL4u8 / Sun 1.5.0_20), in org.eclipse.core.tests.internal.localstore.HistoryStoreTest but I verified that these 2 failures as well as the 5 failures in org.eclipse.core.tests.resources.content.IContentTypeManagerTest also happen without the patch applied. Also, from code review, these do not seem to be related to any changes here. I am ready to commit the patch into HEAD and mark this bug as RESOLVED as per comment 63, since the original request of *allowing* builders to specify scheduling rules is satisfied. I have entered bug 296800 against Platform UI/IDE to fix the build action, and existing bug 289986 can be used to further fine-tune the system and constraints around builders with non-workspace scheduling rules. Szymon as the main contributor and component lead will still have a chance to review changes when releasing the changes into the mapfile.
Created attachment 153724 [details] Combined patch for core.resources with contributor line
Comment on attachment 153724 [details] Combined patch for core.resources with contributor line I have committed the patch to HEAD - since Toni is only committer on the Tools TLP but not on Eclipse, I understand that he qualifies for an iplog+.
Marking fixed since the original request to make the Builder determine the scheduling rule is satisfied. Following bugs currently exist for follow-up work: * Bug 296800 - UI actions for build should not lock the workspace * Bug 289986 - Allow modification of project B while project A is building More bugs for follow-up work may be added, such as addressing the potential minor race condition that Toni has mentioned in the BuildManager. The important point is that having this in Eclipse M4 will enable us to move forward defining builders such that concrete problem cases can be detected, documented and addressed. I also filed bug 296805 and bug 296811 for the test failures I found when verifying against the core/resources unit tests before checkin. Szymon, can you please review and release the change into M4 and set the target milestone accordingly?
I didn't have a chance to look at the patch and the discussion. I'm on other M4 things right now. I will look at it later so we could have it in M4.
There is a bit of over-specification added here in IncrementalProjectBuilder#getRule(): 1) * The rule returned here has no effect in case of an automatic build * or any other workspace-wide build, since the builder is still invoked * inside a workspace root rule in this case. This is an assumption based on the current implementation of workspace build. It is conceivable that we could parallelize full builds, or that someone could implement a parallel workspace build. I think we should just state the generalized form of this: * The rule returned here may have no effect if the build is invoked within the * scope of another operation that locks the entire workspace. 2) * If this method returns any rule other than the workspace root, * resources can be modified concurrently to the build. Therefore, the * delta returned by {@link #getDelta(IProject)} might be incomplete. * The builder should be configured with the "callOnEmptyDelta" option * enabled in this case. This will ensure that the builder is called * even if the delta appears to be empty. This are several strategies for dealing with this, and "callOnEmptyDelta" is only one option. Far more likely in my mind is that builders would lock only the projects for which they require deltas, in which case this won't be a problem. In discussions with Kent Johnson from JDT is the likely approach the Java builder would take here. In any case I think we should just state the behaviour and let the builder designer decide how to handle it. I propose changing this to: * If this method returns any rule other than the workspace root, * resources can be modified concurrently with the build. Therefore, the * delta returned by {@link #getDelta(IProject)} may not contain changes * that occurred concurrently with the build.
Created attachment 153780 [details] Patch of all my wording changes
