Comment 1 Thomas Wolf 2022-03-02 10:54:59 EST

Yes, I noticed that a few days ago, too. Though in may case it was 1000s of commits :-). Somehow drawing that result tree takes ages if many commits were fetched. I'll look into it during 6.2. Probably the same performance hacks as elsewhere are needed: switching off redrawing; perhaps clearing the selection and re-setting it afterwards; perhaps other things. In any case it looks like the "normal" performance problems with Trees when there are many items.

Comment 2 Thomas Wolf 2022-03-02 17:18:49 EST

(In reply to Thomas Wolf from comment #1)
> In any case it looks like the "normal" performance problems with Trees when
> there are many items.

Actually, it's worse. This tree is set to auto-expand to level 2, which means it reads all the commits from the repository on the UI thread. Really fixing this will need switching off auto-expanding, expanding the first node only, and using deferred loading for children (the commits, or their children, the changed files). Deferred loading is quite a bit of work.

Comment 3 Eclipse Genie 2022-03-13 17:43:40 EDT

New Gerrit change created: https://git.eclipse.org/r/c/egit/egit/+/191826

Comment 4 Thomas Wolf 2022-03-13 18:22:30 EDT

(In reply to Eclipse Genie from comment #3)
> New Gerrit change created: https://git.eclipse.org/r/c/egit/egit/+/191826

Pierre-Yves, can you try if this change improves things for you? It should, as it moves all commit parsing and other expensive things into background jobs. I don't have a test case that results in minute-long UI blocking currently...

Download https://ci.eclipse.org/egit/job/egit.gerrit/2578/artifact/org.eclipse.egit.repository/target/repository/ as ZIP file and install EGit and JGit from that. (This will give you an EGit/JGit 6.2, which currently is the same as the 6.1 that will be released on March 16, plus the above change.)

Comment 5 Pierre-Yves Bigourdan 2022-03-14 05:10:08 EDT

Hello Thomas, thanks for looking into this! I've tried this out on two big repositories pulling ~4000 new commits each, things are *much* smoother. Great work!

There were still a couple of small UI hiccups, I've attached three samples below. But again, things are much better than they were before, so feel free to consider this resolved if you can't come up with some easy wins based on these new samples. :)

Sample at 09:52:03.382 (+1.342s)
Stack Trace
	at org.eclipse.swt.internal.cocoa.OS.objc_msgSend(Native Method)
	at org.eclipse.swt.internal.cocoa.NSOutlineView.levelForItem(NSOutlineView.java:66)
	at org.eclipse.swt.widgets.TreeItem.calculateWidth(TreeItem.java:271)
	at org.eclipse.swt.widgets.Tree.calculateWidth(Tree.java:273)
	at org.eclipse.swt.widgets.Tree.setScrollWidth(Tree.java:3216)
	at org.eclipse.swt.widgets.Tree.setScrollWidth(Tree.java:3208)
	at org.eclipse.swt.widgets.Tree.setRedraw(Tree.java:3203)
	at org.eclipse.jface.viewers.AbstractTreeViewer.lambda$1(AbstractTreeViewer.java:1584)
	at org.eclipse.jface.viewers.AbstractTreeViewer$$Lambda$602/0x000000080071f9f0.run(Unknown Source)
	at org.eclipse.jface.viewers.StructuredViewer.preservingSelection(StructuredViewer.java:1398)
	at org.eclipse.jface.viewers.TreeViewer.preservingSelection(TreeViewer.java:365)
	at org.eclipse.jface.viewers.StructuredViewer.preservingSelection(StructuredViewer.java:1359)
	at org.eclipse.jface.viewers.AbstractTreeViewer.inputChanged(AbstractTreeViewer.java:1576)
	at org.eclipse.jface.viewers.ContentViewer.setInput(ContentViewer.java:282)
	at org.eclipse.jface.viewers.StructuredViewer.setInput(StructuredViewer.java:1632)
	at org.eclipse.egit.ui.internal.fetch.FetchResultTable.setData(FetchResultTable.java:617)
	at org.eclipse.egit.ui.internal.fetch.FetchResultDialog.createFetchResultTable(FetchResultDialog.java:136)
	at org.eclipse.egit.ui.internal.pull.PullResultDialog.createDialogArea(PullResultDialog.java:107)
	at org.eclipse.jface.dialogs.Dialog.createContents(Dialog.java:767)
	at org.eclipse.jface.window.Window.create(Window.java:431)
	at org.eclipse.jface.dialogs.Dialog.create(Dialog.java:1094)
	at org.eclipse.jface.window.Window.open(Window.java:788)
	at org.eclipse.egit.ui.internal.pull.PullOperationUI.showResults(PullOperationUI.java:281)
	at org.eclipse.egit.ui.internal.pull.PullOperationUI.handlePullResults(PullOperationUI.java:223)
	at org.eclipse.egit.ui.internal.pull.PullOperationUI$2.run(PullOperationUI.java:194)
	at org.eclipse.swt.widgets.RunnableLock.run(RunnableLock.java:40)
	at org.eclipse.swt.widgets.Synchronizer.runAsyncMessages(Synchronizer.java:185)
	at org.eclipse.swt.widgets.Display.runAsyncMessages(Display.java:4306)
	at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3929)
	at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine$5.run(PartRenderingEngine.java:1154)
	at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:338)
	at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.run(PartRenderingEngine.java:1045)
	at org.eclipse.e4.ui.internal.workbench.E4Workbench.createAndRunUI(E4Workbench.java:155)
	at org.eclipse.ui.internal.Workbench.lambda$3(Workbench.java:644)
	at org.eclipse.ui.internal.Workbench$$Lambda$232/0x000000080028fd78.run(Unknown Source)
	at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:338)
	at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:551)
	at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:156)
	at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:152)
	at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:203)
	at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:136)
	at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:104)
	at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:401)
	at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:255)
	at java.base@17.0.1/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base@17.0.1/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
	at java.base@17.0.1/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base@17.0.1/java.lang.reflect.Method.invoke(Method.java:568)
	at app//org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:659)
	at app//org.eclipse.equinox.launcher.Main.basicRun(Main.java:596)
	at app//org.eclipse.equinox.launcher.Main.run(Main.java:1467)


Sample at 09:52:05.537 (+0.692s)
Stack Trace
	at org.eclipse.swt.internal.cocoa.OS.objc_msgSend_stret(Native Method)
	at org.eclipse.swt.internal.cocoa.NSLayoutManager.glyphRangeForTextContainer(NSLayoutManager.java:88)
	at org.eclipse.swt.graphics.TextLayout.computeRuns(TextLayout.java:334)
	at org.eclipse.swt.graphics.TextLayout.getBounds(TextLayout.java:749)
	at org.eclipse.jface.viewers.StyledCellLabelProvider.updateTextLayout(StyledCellLabelProvider.java:296)
	at org.eclipse.jface.viewers.StyledCellLabelProvider.measure(StyledCellLabelProvider.java:278)
	at org.eclipse.jface.viewers.OwnerDrawLabelProvider$OwnerDrawListener.handleEvent(OwnerDrawLabelProvider.java:59)
	at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:89)
	at org.eclipse.swt.widgets.Display.sendEvent(Display.java:4565)
	at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1522)
	at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1545)
	at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1530)
	at org.eclipse.swt.widgets.TreeItem.calculateWidth(TreeItem.java:262)
	at org.eclipse.swt.widgets.Tree.calculateWidth(Tree.java:273)
	at org.eclipse.swt.widgets.Tree.computeSize(Tree.java:505)
	at org.eclipse.swt.layout.GridData.computeSize(GridData.java:493)
	at org.eclipse.swt.layout.GridLayout.layout(GridLayout.java:225)
	at org.eclipse.swt.layout.GridLayout.computeSize(GridLayout.java:168)
	at org.eclipse.swt.widgets.Composite.computeSize(Composite.java:232)
	at org.eclipse.swt.layout.GridData.computeSize(GridData.java:493)
	at org.eclipse.swt.layout.GridLayout.layout(GridLayout.java:225)
	at org.eclipse.swt.layout.GridLayout.computeSize(GridLayout.java:168)
	at org.eclipse.swt.widgets.Composite.computeSize(Composite.java:232)
	at org.eclipse.swt.layout.GridData.computeSize(GridData.java:493)
	at org.eclipse.swt.layout.GridLayout.layout(GridLayout.java:225)
	at org.eclipse.swt.layout.GridLayout.computeSize(GridLayout.java:168)
	at org.eclipse.swt.widgets.Composite.computeSize(Composite.java:232)
	at org.eclipse.swt.layout.GridData.computeSize(GridData.java:493)
	at org.eclipse.swt.layout.GridLayout.layout(GridLayout.java:225)
	at org.eclipse.swt.layout.GridLayout.computeSize(GridLayout.java:168)
	at org.eclipse.swt.widgets.Composite.computeSize(Composite.java:232)
	at org.eclipse.swt.layout.GridData.computeSize(GridData.java:493)
	at org.eclipse.swt.layout.GridLayout.layout(GridLayout.java:225)
	at org.eclipse.swt.layout.GridLayout.computeSize(GridLayout.java:168)
	at org.eclipse.swt.widgets.Composite.computeSize(Composite.java:232)
	at org.eclipse.swt.widgets.Shell.computeSize(Shell.java:630)
	at org.eclipse.jface.window.Window.getInitialSize(Window.java:590)
	at org.eclipse.jface.dialogs.Dialog.getInitialSize(Dialog.java:1202)
	at org.eclipse.egit.ui.internal.pull.PullResultDialog.getInitialSize(PullResultDialog.java:189)
	at org.eclipse.jface.window.Window.initializeBounds(Window.java:762)
	at org.eclipse.jface.dialogs.Dialog.initializeBounds(Dialog.java:721)
	at org.eclipse.jface.window.Window.create(Window.java:435)
	at org.eclipse.jface.dialogs.Dialog.create(Dialog.java:1094)
	at org.eclipse.jface.window.Window.open(Window.java:788)
	at org.eclipse.egit.ui.internal.pull.PullOperationUI.showResults(PullOperationUI.java:281)
	at org.eclipse.egit.ui.internal.pull.PullOperationUI.handlePullResults(PullOperationUI.java:223)
	at org.eclipse.egit.ui.internal.pull.PullOperationUI$2.run(PullOperationUI.java:194)
	at org.eclipse.swt.widgets.RunnableLock.run(RunnableLock.java:40)
	at org.eclipse.swt.widgets.Synchronizer.runAsyncMessages(Synchronizer.java:185)
	at org.eclipse.swt.widgets.Display.runAsyncMessages(Display.java:4306)
	at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3929)
	at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine$5.run(PartRenderingEngine.java:1154)
	at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:338)
	at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.run(PartRenderingEngine.java:1045)
	at org.eclipse.e4.ui.internal.workbench.E4Workbench.createAndRunUI(E4Workbench.java:155)
	at org.eclipse.ui.internal.Workbench.lambda$3(Workbench.java:644)
	at org.eclipse.ui.internal.Workbench$$Lambda$232/0x000000080028fd78.run(Unknown Source)
	at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:338)
	at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:551)
	at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:156)
	at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:152)
	at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:203)
	at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:136)
	at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:104)
	at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:401)
	at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:255)
	at java.base@17.0.1/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base@17.0.1/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
	at java.base@17.0.1/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base@17.0.1/java.lang.reflect.Method.invoke(Method.java:568)
	at app//org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:659)
	at app//org.eclipse.equinox.launcher.Main.basicRun(Main.java:596)
	at app//org.eclipse.equinox.launcher.Main.run(Main.java:1467)


Sample at 09:53:13.431 (+0.667s)
Stack Trace
	at org.eclipse.swt.internal.cocoa.OS.objc_msgSendSuper_stret(Native Method)
	at org.eclipse.swt.widgets.TreeItem.calculateWidth(TreeItem.java:240)
	at org.eclipse.swt.widgets.Tree.calculateWidth(Tree.java:273)
	at org.eclipse.swt.widgets.Tree.calculateWidth(Tree.java:276)
	at org.eclipse.swt.widgets.Tree.setScrollWidth(Tree.java:3216)
	at org.eclipse.swt.widgets.Tree.setScrollWidth(Tree.java:3208)
	at org.eclipse.swt.widgets.Tree.setRedraw(Tree.java:3203)
	at org.eclipse.jface.viewers.AbstractTreeViewer.expandToLevel(AbstractTreeViewer.java:1135)
	at org.eclipse.egit.ui.internal.fetch.FetchResultTable$4$1$1.runInUIThread(FetchResultTable.java:495)
	at org.eclipse.ui.progress.UIJob.lambda$0(UIJob.java:95)
	at org.eclipse.ui.progress.UIJob$$Lambda$851/0x0000000800b775b8.run(Unknown Source)
	at org.eclipse.swt.widgets.RunnableLock.run(RunnableLock.java:40)
	at org.eclipse.swt.widgets.Synchronizer.runAsyncMessages(Synchronizer.java:185)
	at org.eclipse.swt.widgets.Display.runAsyncMessages(Display.java:4306)
	at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3929)
	at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine$5.run(PartRenderingEngine.java:1154)
	at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:338)
	at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.run(PartRenderingEngine.java:1045)
	at org.eclipse.e4.ui.internal.workbench.E4Workbench.createAndRunUI(E4Workbench.java:155)
	at org.eclipse.ui.internal.Workbench.lambda$3(Workbench.java:644)
	at org.eclipse.ui.internal.Workbench$$Lambda$232/0x000000080028fd78.run(Unknown Source)
	at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:338)
	at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:551)
	at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:156)
	at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:152)
	at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:203)
	at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:136)
	at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:104)
	at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:401)
	at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:255)
	at java.base@17.0.1/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base@17.0.1/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
	at java.base@17.0.1/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base@17.0.1/java.lang.reflect.Method.invoke(Method.java:568)
	at app//org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:659)
	at app//org.eclipse.equinox.launcher.Main.basicRun(Main.java:596)
	at app//org.eclipse.equinox.launcher.Main.run(Main.java:1467)

Comment 6 Thomas Wolf 2022-03-14 07:58:30 EDT

Great to hear!

Your samples are interesting, but I'm not sure I can do much about them.

The first one appears to be the initial setInput(), which somehow takes a long time computing the scrollbar settings. Not sure why it takes so long. How many branches/tags were fetched?

The second one again tries to compute the size of the tree. Perhaps something can be done by giving that tree some default size so that the layout doesn't try to determine the true size of the tree. And perhaps there's something we could do to avoid this double size computation...

The third one apparently is the expansion of of the top node in the tree to show the commits fetched for that branch. If that is thousands of commits, then the tree is just slow. Unlikely I'll be able to improve that. (Except some kind of paging, but the standard deferred loading mechanism isn't made for that. I'd have to invent something.)

("Expand all" -- the "+" button on the right -- is also still slow and leads to a UI block (spinning cursor). It really expands everything, down to the individual files. That is just going to take quite a bit of time with many rows. Nothing I could do about that.)

Comment 7 Eclipse Genie 2022-03-14 17:23:09 EDT

Gerrit change https://git.eclipse.org/r/c/egit/egit/+/191826 was merged to [master].
Commit: http://git.eclipse.org/c/egit/egit.git/commit/?id=9f1cc0bba3d678f1130bd6bbd5c1f26b9f9b12e6

Comment 8 Thomas Wolf 2022-03-14 17:27:57 EDT

Merged with a minor tweak that hopefully avoids this duplicate size computation.

Comment 9 Pierre-Yves Bigourdan 2022-03-15 05:22:41 EDT

> How many branches/tags were fetched?

Several hundred branches, and several thousand tags (basically each commit is tagged with the corresponding CI build version number).

> Merged with a minor tweak that hopefully avoids this duplicate size computation.

I've not observed any recurrence of the second stack trace sample, so that seems to have had the desired effect.

Thanks again for looking into this and providing detailed explanations!