Bug 542597 - staging view does not update if build automatically is off and workspace is built
Summary: staging view does not update if build automatically is off and workspace is b...
Status: REOPENED
Alias: None
Product: EGit
Classification: Technology
Component: UI (show other bugs)
Version: 5.2   Edit
Hardware: PC Windows 10
: P3 normal (vote)
Target Milestone: 5.13   Edit
Assignee: Project Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-12-10 05:08 EST by Michael Keppler CLA
Modified: 2021-07-14 10:53 EDT (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Keppler CLA 2018-12-10 05:08:27 EST
If you have an oomph based setup, during the startup it automatically disables "Build automatically" while doing its tasks. During the time where "build automatically" is off and the workspace build is running (e.g. due to reloading the target from the oomph setup), the staging view never updates itself. This can be really cumbersome if you want to commit changes during that time. After clicking the plus button you have to refresh the view explicitly from the statusbar, otherwise the changes are not shown as staged. Similarly after committing you have to refresh again, otherwise the committed files don't go away.

Only when Oomph has finished building the workspace and re-enables automatic building, the view reacts to changes again. At work I have a setup where this can easily take 10 minutes, and you easily forget that Oomph is still doing some tasks...

Do we eventually have a general problem if "build automatically" is disabled? (I'm still not sure whether it is just that flag, or both that flag and a full rebuild running, which causes the issue)
Comment 1 Thomas Wolf CLA 2018-12-10 15:59:21 EST
The staging view reacts to Index Diff Changed events. The IndexDiffCacheEntry used to wait for the workspace lock, and since the parallel pull implementation still waits for some project locks. See IndexDiffCacheEntry.waitForWorkspaceLock().

This is independent of autobuild or an explicit full build running. If a full build is running and has the workspace locked, the staging view will update only once that build is done.

Compare bug 541115.

I guess the wait is needed in case a build changes some non-ignored resources. But the index diff update then itself runs without workspace lock, so a subsequent build is free to start and change the resources anyway.

At the end of the day I'm just not sure if the wait is needed. On Mac and Linux at least, I think the worst that could happen is that we compute an index diff that's not quite correct because some build changes files while we're computing it, but we'd get a ResourceChangedEvent once the build's done and then would update our index diff and eventually we'd get a correct state. But might we get into trouble on Windows because two different threads might try to access the same file concurrently? Moreover, the build would access the file via the Eclipse resource tree (IResource), while JGit would use java.io.File.

And, as mentioned in bug 541115, we'd have to rewrite quite a few tests.
Comment 2 Eclipse Genie CLA 2021-06-26 10:16:07 EDT
New Gerrit change created: https://git.eclipse.org/r/c/egit/egit/+/182486
Comment 4 Andrey Loskutov CLA 2021-07-05 05:25:18 EDT
(In reply to Eclipse Genie from comment #3)
> Gerrit change https://git.eclipse.org/r/c/egit/egit/+/182486 was merged to
> [master].
> Commit:
> http://git.eclipse.org/c/egit/egit.git/commit/
> ?id=06f40a8aee6f7a3dc2319bc5c84830ba842cc71d

Please consider to revert this change or to do something else.

I see now that in a multi-project build with ~200 PDE projects located in same git repository, "Updating index" runs multiple times in parallel to the main build task, stealing CPU/IO resources from build.
Comment 5 Thomas Wolf CLA 2021-07-05 06:35:13 EDT
(In reply to Andrey Loskutov from comment #4)
> (In reply to Eclipse Genie from comment #3)
> > Gerrit change https://git.eclipse.org/r/c/egit/egit/+/182486 was merged to
> > [master].
> > Commit:
> > http://git.eclipse.org/c/egit/egit.git/commit/
> > ?id=06f40a8aee6f7a3dc2319bc5c84830ba842cc71d
> 
> Please consider to revert this change or to do something else.
> 
> I see now that in a multi-project build with ~200 PDE projects located in
> same git repository, "Updating index" runs multiple times in parallel to the
> main build task, stealing CPU/IO resources from build.

Any idea why? I suppose the build produces resource change events, but unless that build produces file changes in non-ignored directories, it should not cause an index diff computation to run. If it does, maybe there's a problem in GitResourceDeltaVisitor.

Or... if "automatically ignore derived resources" is on, a .gitignore might change, in which case we'd recompute an index diff anyway. Perhaps that case needs to be handled better.

Given that we're still early in a release cycle and I have other uses cases in mind than just won't work if we wait for builds all the time I'd prefer to first invest some time to try to figure out what exactly is happening and what we could improve to avoid the problems you see. We can still revert before the next release if we can't find a solution.
Comment 6 Andrey Loskutov CLA 2021-07-05 07:49:38 EDT
(In reply to Thomas Wolf from comment #5)
> Any idea why? I suppose the build produces resource change events, but
> unless that build produces file changes in non-ignored directories, it
> should not cause an index diff computation to run. If it does, maybe there's
> a problem in GitResourceDeltaVisitor.

Haven't analyzed yet. We have many bundles that call ant tasks that call xtend compiler/ecore/xcore/m2e etc generators that touch lot of files and generate java sources (should be git ignored). I guess the problem is somewhere along those "external" build events that might trigger workspace refresh tasks that might trigger egit index jobs.

> Or... if "automatically ignore derived resources" is on

It is off.
Comment 7 Andrey Loskutov CLA 2021-07-05 08:19:44 EDT
BTW, I know there are few scripts in SDK that re-build files checked into git, so not git ignored on purpose. I'm not sure if we do similar things in our product, but that could be the case, especially I know we have lot of generated artifacts.
Comment 8 Thomas Wolf CLA 2021-07-05 10:23:41 EDT
(In reply to Andrey Loskutov from comment #7)
> BTW, I know there are few scripts in SDK that re-build files checked into
> git, so not git ignored on purpose. I'm not sure if we do similar things in
> our product, but that could be the case, especially I know we have lot of
> generated artifacts.

Normally the SDK build produces only a few such generated artifacts. Updating the index diff for only a few files should be fast and not really hurt build performance.

I can see that updating the index diff might be costly if a full reload is done if many files changed.

Do you have any numbers how much slower your build is because of index diff updates "stealing CPU/IO resources from the build"?
Comment 9 Andrey Loskutov CLA 2021-07-05 10:39:12 EDT
(In reply to Thomas Wolf from comment #8)
> Normally the SDK build produces only a few such generated artifacts.
> Updating the index diff for only a few files should be fast and not really
> hurt build performance.

For "tiny" SDK repositories, yes. I just wanted to point out a possible use case where not git ignored files are updated during the build.

> I can see that updating the index diff might be costly if a full reload is
> done if many files changed.

No, in my case I had zero changed files.

> Do you have any numbers how much slower your build is because of index diff
> updates "stealing CPU/IO resources from the build"?

Not yet. But manually updating git status in IDE on our repository via Staging view "Refresh" on idling file system/CPU takes ~ 8 - 10 seconds with full CPU load. I see many times this job triggered during full build, so I assume that sums up to at least minute extra time.
Comment 10 Thomas Wolf CLA 2021-07-05 10:49:11 EDT
(In reply to Andrey Loskutov from comment #9)
> Not yet. But manually updating git status in IDE on our repository via
> Staging view "Refresh" on idling file system/CPU takes ~ 8 - 10 seconds with
> full CPU load. I see many times this job triggered during full build, so I
> assume that sums up to at least minute extra time.

Staging view "Refresh" refreshes all projects from the repository and then waits for a full index diff reload. So it's not really representative.