Community
Participate
Working Groups
I have huge legacy repository converted from Clearcase, where old Clearcase "baselines" (now commits) contained hundreds or thousands of files. Selecting those commits in the history view takes literally minutes and Eclipse UI freezes completely because for each file type in the commit FileDiff.getImageDescriptor() calls to EditorRegistry.getImageDescriptor() and this one calls for *unknown* file types to Program.findProgram() and to the native GTK, trying to find icons in the system. This is OK for small commits with few files but doesn't scale for huge commits. The call to Program.findProgram() MUST be in UI thread, there is no way to workaround this and therefore cannot run in a background job - the code is OK from this point of view. It means, we should either cache the results for editor types (still huge penalty for the very fist huge commit) or allow users to configure something like "History view -> Don't query external icons for unknown file types". Therefore I would like to introduce such kind of option for egit (question: where does it belongs to in the preferences - to the "Git->History"?), default to "false" (== query everything, to be compatible) and allow users to switch this option. Given that option, I think we can avoid the heavy getSystemExternalEditorImageDescriptor() call: we can check upfront if the content type is known by Eclipse and for all external content types simply supply "text" - it is just icon at the end... Stack trace of frozen Eclipse (I'm using Eclipse 3.8.2, EGit 4.0 (but also older EGit versions)): "main" prio=10 tid=0x00007f35d800b800 nid=0xbe7 runnable [0x00007f35df8b5000] java.lang.Thread.State: RUNNABLE at org.eclipse.swt.internal.gtk.OS._g_app_info_get_default_for_type(Native Method) at org.eclipse.swt.internal.gtk.OS.g_app_info_get_default_for_type(OS.java:2006) at org.eclipse.swt.program.Program.gio_getProgram(Program.java:861) at org.eclipse.swt.program.Program.findProgram(Program.java:635) at org.eclipse.swt.program.Program.findProgram(Program.java:613) at org.eclipse.ui.internal.registry.EditorRegistry.getSystemExternalEditorImageDescriptor(EditorRegistry.java:1272) at org.eclipse.ui.internal.registry.EditorRegistry.getImageDescriptor(EditorRegistry.java:1480) at org.eclipse.ui.internal.registry.EditorRegistry.getImageDescriptor(EditorRegistry.java:414) at org.eclipse.egit.ui.UIUtils.getEditorImage(UIUtils.java:556) at org.eclipse.egit.ui.internal.history.FileDiff.getImageDescriptor(FileDiff.java:420) at org.eclipse.egit.ui.internal.history.FileDiffLabelProvider.getImage(FileDiffLabelProvider.java:47) at org.eclipse.jface.viewers.ColumnLabelProvider.update(ColumnLabelProvider.java:37) at org.eclipse.jface.viewers.ViewerColumn.refresh(ViewerColumn.java:152) at org.eclipse.jface.viewers.AbstractTableViewer.doUpdateItem(AbstractTableViewer.java:399) at org.eclipse.jface.viewers.StructuredViewer$UpdateItemSafeRunnable.run(StructuredViewer.java:485) at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:42) at org.eclipse.ui.internal.JFaceUtil$1.run(JFaceUtil.java:49) at org.eclipse.jface.util.SafeRunnable.run(SafeRunnable.java:175) at org.eclipse.jface.viewers.StructuredViewer.updateItem(StructuredViewer.java:2167) at org.eclipse.jface.viewers.AbstractTableViewer.internalRefreshAll(AbstractTableViewer.java:752) at org.eclipse.jface.viewers.AbstractTableViewer.internalRefresh(AbstractTableViewer.java:649) at org.eclipse.jface.viewers.AbstractTableViewer.internalRefresh(AbstractTableViewer.java:636) at org.eclipse.jface.viewers.AbstractTableViewer$2.run(AbstractTableViewer.java:592) at org.eclipse.jface.viewers.StructuredViewer.preservingSelection(StructuredViewer.java:1443) at org.eclipse.jface.viewers.StructuredViewer.preservingSelection(StructuredViewer.java:1404) at org.eclipse.jface.viewers.AbstractTableViewer.inputChanged(AbstractTableViewer.java:590) at org.eclipse.egit.ui.internal.history.CommitFileDiffViewer.inputChanged(CommitFileDiffViewer.java:422) I will came later to this with a patch, if it is OK with the proposed option.
It seems wrong that findProgram() needs to run on the UI thread, it doesn't seem to be related to UI. It's calling OS.LSGetApplicationForInfo() which is a native call. how about limiting number of these calls to a reasonable number per commit (e.g. 50) ? or could this be run in a background thread ? If that's possible we could first use a default icon and switch to the correct one lazily
(In reply to Matthias Sohn from comment #1) > It seems wrong that findProgram() needs to run on the UI thread, it doesn't > seem to be related to UI. Unfortunately this is really implemented in the way that any access from non UI thread will fail. > how about limiting number of these calls to a reasonable number per commit > (e.g. 50) ? The problem is that API tries to use file *names* to match editors so that after first 50 "unknown" files the rest will be shown differently. > or could this be run in a background thread ? No, this is not possible, the code must be in UI thread, therefore the best way to do it is not to do it at all :-) > If that's possible we could > first use a default icon and switch to the correct one lazily To retrieve real icon via findProgram() we need access to the UI thread. As said, this cannot be done in any other "lazy" way. What we can probably do is to switch from "show all icons" to "show only icons for known files" automatically if the number of files is bigger as 50-100. We also can try to avoid asking for files with their real name if the content type is "unknown" (Platform.getContentTypeManager().findContentTypeFor(filename) == null), and ask for <empty_string>.<extension> only. This way the first match will be cached and reused later. I haven't checked how well this works yet. We also can try to use (I haven't checked if this works and if it is similar slow!) EditorRegistry.getDefaultEditor(filename) and if it is not null, use it's image descriptor, and in other case just provide the icon of the default editor (WorkbenchImages.getImageDescriptor(ISharedImages.IMG_OBJ_FILE)).
I've added some references which shows why it is like it is... BTW I believe I've also observed freezes while merging huge number of files - in this case the both staged/unstaged lists in the Staging view were involved - I believe because of the same reason. So the solution should also not only fix FileDiff but consider the code in StagingViewLabelProvider too.
Created attachment 252506 [details] Example git repo with single project and some big commits I've started to look at the code and I guess we can't just add limit to the decorators as they are stateless. Try to select different commits in the attached (small) example...
Created attachment 252509 [details] now really with git commits
I think I have an acceptable solution without extra preference etc: https://git.eclipse.org/r/46016
it doesn't make a big difference on Mac, with the sample repository it takes around 2sec to switch history view from jgit to the sample repository. Though it takes 8-9sec to switch it back to jgit' history.
submitted your change since it looks good. Did it make a big difference on Linux ? can we close this bug ?
(In reply to Matthias Sohn from comment #8) > submitted your change since it looks good. Thanks! > Did it make a big difference on > Linux ? The difference like day and night, at least on our RHEL7/KDE setup. Now I can browse the history again, before UI was frozen for minutes. > can we close this bug ? Yes, works now flawlessly. The most time is now spent in the background job trying to get branch info for the comment (we have lot of branches), but this one is not blocking UI, so I can now do diffs etc.
