Community
Participate
Working Groups
For example staging a file via drag and drop can freeze UI on big repositories because the AddCommand is directly executed by Staging view from UI thread, same for unstage, delete, ignore, replace and rebase operations. The operations should be wrapped to jobs (note: some of them initially require UI thread for prompts) and scheduled via IWorkbenchSiteProgressService, see for example StagingView.commit(boolean) or GitHistoryPage.schedule(Job). I think those jobs must be using same scheduling rule to prevent different repo operations running in parallel (e.g. multiple stage/unstage operations executed fast one after each other). Ideally staging view will detect that such operations are running and also postpone the index update if one is queued (see myIndexDiffListener). BTW not sure if it was a good idea to run reload(Repository) entirely in UI thread. Questions: 1) Should the jobs can have same family? Yes, if we need to know if one of them is running. 2) Cancel pending jobs on closing the view? Most likely no - user assumes we did all the work for him already. Example of the stack trace of Eclipse frozen at "drag-and-drop" of a file to the staged area: "main" prio=10 tid=0x0000000012518800 nid=0x76be runnable [0x000000004132a000] java.lang.Thread.State: RUNNABLE at org.eclipse.jgit.dircache.DirCacheTree.peq(DirCacheTree.java:525) at org.eclipse.jgit.dircache.DirCacheTree.validate(DirCacheTree.java:446) at org.eclipse.jgit.dircache.DirCacheTree.validate(DirCacheTree.java:481) at org.eclipse.jgit.dircache.DirCacheTree.validate(DirCacheTree.java:481) at org.eclipse.jgit.dircache.DirCacheTree.validate(DirCacheTree.java:481) at org.eclipse.jgit.dircache.DirCacheTree.validate(DirCacheTree.java:481) at org.eclipse.jgit.dircache.DirCacheTree.validate(DirCacheTree.java:481) at org.eclipse.jgit.dircache.DirCacheTree.validate(DirCacheTree.java:481) at org.eclipse.jgit.dircache.DirCache.getCacheTree(DirCache.java:906) at org.eclipse.jgit.dircache.DirCacheIterator.<init>(DirCacheIterator.java:107) at org.eclipse.egit.core.internal.indexdiff.IndexDiffCacheEntry.refreshIndexDelta(IndexDiffCacheEntry.java:223) at org.eclipse.egit.core.internal.indexdiff.IndexDiffCacheEntry.access$0(IndexDiffCacheEntry.java:203) at org.eclipse.egit.core.internal.indexdiff.IndexDiffCacheEntry$1.onIndexChanged(IndexDiffCacheEntry.java:102) at org.eclipse.jgit.events.IndexChangedEvent.dispatch(IndexChangedEvent.java:55) at org.eclipse.jgit.events.IndexChangedEvent.dispatch(IndexChangedEvent.java:47) at org.eclipse.jgit.events.ListenerList.dispatch(ListenerList.java:120) at org.eclipse.jgit.lib.Repository.fireEvent(Repository.java:155) at org.eclipse.jgit.internal.storage.file.FileRepository.notifyIndexChanged(FileRepository.java:447) at org.eclipse.jgit.lib.Repository$1.onIndexChanged(Repository.java:1049) at org.eclipse.jgit.dircache.DirCache.commit(DirCache.java:707) at org.eclipse.jgit.dircache.BaseDirCacheEditor.commit(BaseDirCacheEditor.java:199) at org.eclipse.jgit.dircache.DirCacheBuilder.commit(DirCacheBuilder.java:72) at org.eclipse.jgit.api.AddCommand.call(AddCommand.java:209) at org.eclipse.egit.ui.internal.staging.StagingView.stage(StagingView.java:1909) at org.eclipse.egit.ui.internal.staging.StagingView.access$17(StagingView.java:1867) at org.eclipse.egit.ui.internal.staging.StagingView$16.drop(StagingView.java:786) at org.eclipse.swt.dnd.DNDListener.handleEvent(DNDListener.java:90)
Could we reuse the corresponding operations in org.eclipse.egit.core.op which already know the needed scheduling rule ?
(In reply to Matthias Sohn from comment #1) > Could we reuse the corresponding operations in org.eclipse.egit.core.op > which already know the needed scheduling rule ? Thanks for the hint, good idea, but ... I've just tried to use AddToIndexOperation - the problem is that it is IResource based (and calculates the repositories out of resources for scheduling rule), while StagingView uses String paths because it can see non-IResource elements. So one can try to refactor the operation first, but looking at the code I think we could get it easier by using RuleUtil and wrapping existing StagingView code. We can see later how similar the resulting jobs will be and if we can extract some common functionality. But re-using existing operations makes lot of sense because I've just seen that StagingView doesn't call RepositoryMapping.fireRepositoryChanged() at the end (because it doesn't use RepositoryMapping at all because RepositoryMapping is IResource based)... OK, there seem to be no listener in EGit for this but it is at least inconsistent and in worst case breaks 3rd party clients. Not all operations are IResource based, so if there are suitable existing we should definitely reuse them. I will see if I can came with a patch for that.
First proposal for stage/unstage: https://git.eclipse.org/r/39586
(In reply to Andrey Loskutov from comment #3) > First proposal for stage/unstage: https://git.eclipse.org/r/39586 merged as e091484329031a4049f98931b9de57bff046932a
