diff options
author | Ed Merks | 2020-03-22 08:43:26 +0000 |
---|---|---|
committer | Ed Merks | 2020-03-23 06:18:05 +0000 |
commit | 604228dc96fb6a94813ddf850dec2ed79b1924a8 (patch) | |
tree | 4003af3b72de8edb52603d921657e6ca0a2d6921 | |
parent | 94c6971552563e1889f13b2aebee354794aa4c74 (diff) | |
download | rt.equinox.p2-604228dc96fb6a94813ddf850dec2ed79b1924a8.tar.gz rt.equinox.p2-604228dc96fb6a94813ddf850dec2ed79b1924a8.tar.xz rt.equinox.p2-604228dc96fb6a94813ddf850dec2ed79b1924a8.zip |
Bug 550265 - UI freeze in Installed Software tabI20200405-0600I20200404-1800I20200404-1220I20200404-0600I20200404-0330I20200403-1800I20200402-2250I20200402-1800I20200402-0850I20200402-0410I20200401-1800I20200331-1800I20200330-1800I20200329-1800I20200328-1800I20200327-1800I20200326-1800I20200325-1800I20200324-1800
Add logic to eliminate cycles and to optimize performance by caching
children and avoiding expensive duplicate queries.
Change-Id: Id317e90c99442a79d77dbce48f6f91313d16d70a
Signed-off-by: Ed Merks <ed.merks@gmail.com>
3 files changed, 139 insertions, 19 deletions
diff --git a/bundles/org.eclipse.equinox.p2.ui/src/org/eclipse/equinox/internal/p2/ui/dialogs/InstalledIUGroup.java b/bundles/org.eclipse.equinox.p2.ui/src/org/eclipse/equinox/internal/p2/ui/dialogs/InstalledIUGroup.java index 7fd3dbbb0..291e66aed 100644 --- a/bundles/org.eclipse.equinox.p2.ui/src/org/eclipse/equinox/internal/p2/ui/dialogs/InstalledIUGroup.java +++ b/bundles/org.eclipse.equinox.p2.ui/src/org/eclipse/equinox/internal/p2/ui/dialogs/InstalledIUGroup.java @@ -13,12 +13,15 @@ *******************************************************************************/ package org.eclipse.equinox.internal.p2.ui.dialogs; +import java.util.*; import org.eclipse.e4.ui.dialogs.filteredtree.FilteredTree; import org.eclipse.e4.ui.dialogs.filteredtree.PatternFilter; import org.eclipse.equinox.internal.p2.ui.ProvUI; import org.eclipse.equinox.internal.p2.ui.ProvUIProvisioningListener; +import org.eclipse.equinox.internal.p2.ui.model.InstalledIUElement; import org.eclipse.equinox.internal.p2.ui.model.ProfileElement; import org.eclipse.equinox.internal.p2.ui.viewers.*; +import org.eclipse.equinox.p2.metadata.IInstallableUnit; import org.eclipse.equinox.p2.ui.ProvisioningUI; import org.eclipse.jface.viewers.StructuredViewer; import org.eclipse.jface.viewers.TreeViewer; @@ -27,8 +30,8 @@ import org.eclipse.swt.graphics.Font; import org.eclipse.swt.widgets.*; /** - * An InstalledIUGroup is a reusable UI component that displays the - * IU's in a given profile. + * An InstalledIUGroup is a reusable UI component that displays the IU's in a + * given profile. * * @since 3.4 */ @@ -39,13 +42,14 @@ public class InstalledIUGroup extends StructuredIUGroup { /** * Create a group that represents the installed IU's. * - * @param parent the parent composite for the group - * @param font The font to use for calculating pixel sizes. This font is - * not managed by the receiver. - * @param profileId the id of the profile whose content is being shown. + * @param parent the parent composite for the group + * @param font The font to use for calculating pixel sizes. This font is + * not managed by the receiver. + * @param profileId the id of the profile whose content is being shown. * @param columnConfig the columns to be shown */ - public InstalledIUGroup(ProvisioningUI ui, final Composite parent, Font font, String profileId, IUColumnConfig[] columnConfig) { + public InstalledIUGroup(ProvisioningUI ui, final Composite parent, Font font, String profileId, + IUColumnConfig[] columnConfig) { super(ui, parent, font, columnConfig); if (profileId == null) this.profileId = ui.getProfileId(); @@ -59,17 +63,97 @@ public class InstalledIUGroup extends StructuredIUGroup { // Table of installed IU's FilteredTree filteredTree = new FilteredTree(parent, SWT.MULTI | SWT.FULL_SELECTION | SWT.H_SCROLL | SWT.V_SCROLL | SWT.BORDER, new PatternFilter()); - filteredTree.getFilterControl().setFocus(); // Steal focus, consistent with org.eclipse.ui.internal.about.AboutPluginsPage + filteredTree.getFilterControl().setFocus(); // Steal focus, consistent with + // org.eclipse.ui.internal.about.AboutPluginsPage TreeViewer installedIUViewer = filteredTree.getViewer(); - // Filters and sorters before establishing content, so we don't refresh unnecessarily. + // Filters and sorters before establishing content, so we don't refresh + // unnecessarily. IUComparator comparator = new IUComparator(IUComparator.IU_NAME); comparator.useColumnConfig(getColumnConfig()); installedIUViewer.setComparator(comparator); installedIUViewer.setComparer(new ProvElementComparer()); // Now the content. - installedIUViewer.setContentProvider(new DeferredQueryContentProvider()); + // Specialize the content provider for performance optimization. + // See https://bugs.eclipse.org/bugs/show_bug.cgi?id=550265 + installedIUViewer.setContentProvider(new DeferredQueryContentProvider() { + /** + * A cache for mapping each element to its children. + */ + private final Map<Object, Object[]> elementChildren = new HashMap<>(); + + /** + * A cache for mapping each IU to that IU's children. + */ + private final Map<IInstallableUnit, Set<IInstallableUnit>> iuChildren = new HashMap<>(); + + @Override + public boolean hasChildren(Object element) { + // Delegate to getChildren so that all children are always cached. + return getChildren(element).length != 0; + } + + @Override + public Object[] getChildren(Object element) { + Object[] objects = elementChildren.get(element); + if (objects == null) { + if (element instanceof InstalledIUElement) { + // Special case handling. + InstalledIUElement installedIUElement = (InstalledIUElement) element; + if (!installedIUElement.shouldShowChildren()) { + // If the element shouldn't show children because that would create a cycle, + // don't show any. + objects = new Object[0]; + elementChildren.put(element, objects); + return objects; + } + + // If the children for this IU have already been computed. + String localProfileId = installedIUElement.getProfileId(); + IInstallableUnit iu = installedIUElement.getIU(); + Set<IInstallableUnit> children = iuChildren.get(iu); + if (children != null) { + // Bypass the expensive query computation because we already know the IU + // children from a previous expensive computation. + Set<InstalledIUElement> elementIUChildren = new LinkedHashSet<>(); + for (IInstallableUnit childIU : children) { + elementIUChildren.add(new InstalledIUElement(element, localProfileId, childIU)); + } + + objects = elementIUChildren.toArray(); + elementChildren.put(element, objects); + return objects; + } + } + + // Get the children the normal ways and cache them. + objects = super.getChildren(element); + elementChildren.put(element, objects); + + // If there are children and this is an InstalledIUElement, cache the mapping + // from the IU to the child IUs for reuse above. + if (objects.length != 0 && element instanceof InstalledIUElement) { + InstalledIUElement installedIUElement = (InstalledIUElement) element; + IInstallableUnit iu = installedIUElement.getIU(); + Set<IInstallableUnit> children = new LinkedHashSet<>(); + for (Object object : objects) { + if (object instanceof InstalledIUElement) { + InstalledIUElement childInstalledIUElement = (InstalledIUElement) object; + IInstallableUnit childIU = childInstalledIUElement.getIU(); + children.add(childIU); + } else { + // In case all the children are not also InstalledIUElement, which shouldn't + // happen. + return objects; + } + } + iuChildren.put(iu, children); + } + } + return objects; + } + }); // Now the visuals, columns before labels. setTreeColumns(installedIUViewer.getTree()); @@ -78,9 +162,13 @@ public class InstalledIUGroup extends StructuredIUGroup { // Input last. installedIUViewer.setInput(getInput()); - final StructuredViewerProvisioningListener listener = new StructuredViewerProvisioningListener(getClass().getName(), installedIUViewer, ProvUIProvisioningListener.PROV_EVENT_IU | ProvUIProvisioningListener.PROV_EVENT_PROFILE, getProvisioningUI().getOperationRunner()); + final StructuredViewerProvisioningListener listener = new StructuredViewerProvisioningListener( + getClass().getName(), installedIUViewer, + ProvUIProvisioningListener.PROV_EVENT_IU | ProvUIProvisioningListener.PROV_EVENT_PROFILE, + getProvisioningUI().getOperationRunner()); ProvUI.getProvisioningEventBus(getProvisioningUI().getSession()).addListener(listener); - installedIUViewer.getControl().addDisposeListener(e -> ProvUI.getProvisioningEventBus(getProvisioningUI().getSession()).removeListener(listener)); + installedIUViewer.getControl().addDisposeListener( + e -> ProvUI.getProvisioningEventBus(getProvisioningUI().getSession()).removeListener(listener)); return installedIUViewer; } diff --git a/bundles/org.eclipse.equinox.p2.ui/src/org/eclipse/equinox/internal/p2/ui/model/InstalledIUElement.java b/bundles/org.eclipse.equinox.p2.ui/src/org/eclipse/equinox/internal/p2/ui/model/InstalledIUElement.java index 615480202..3fb7146f8 100644 --- a/bundles/org.eclipse.equinox.p2.ui/src/org/eclipse/equinox/internal/p2/ui/model/InstalledIUElement.java +++ b/bundles/org.eclipse.equinox.p2.ui/src/org/eclipse/equinox/internal/p2/ui/model/InstalledIUElement.java @@ -15,6 +15,7 @@ package org.eclipse.equinox.internal.p2.ui.model; import java.util.Collection; +import java.util.Objects; import org.eclipse.core.runtime.IProgressMonitor; import org.eclipse.equinox.internal.p2.ui.*; import org.eclipse.equinox.p2.metadata.IInstallableUnit; @@ -101,10 +102,37 @@ public class InstalledIUElement extends QueriedElement implements IIUElement { @Override public boolean shouldShowChildren() { + // Check that no parent has the same IU as this parent. + // That would lead to a cycle and induce an infinite tree. + // See https://bugs.eclipse.org/bugs/show_bug.cgi?id=550265 + for (Object parent = getParent(this); parent instanceof InstalledIUElement;) { + InstalledIUElement installedIUElement = (InstalledIUElement) parent; + if (Objects.equals(iu, installedIUElement.getIU())) { + return false; + } + parent = installedIUElement.getParent(installedIUElement); + } return true; } @Override + public Object[] getChildren(Object o) { + if (shouldShowChildren()) { + // Only show children if that would not induce a cycle. + return super.getChildren(o); + } + + return new Object[0]; + } + + @Override + protected Object[] getFilteredChildren(Collection<?> results) { + // Given the equality definition, a child cannot be equal to a sibling of this + // because the child has a different parent. + return results.toArray(); + } + + @Override public boolean equals(Object obj) { if (this == obj) return true; diff --git a/bundles/org.eclipse.equinox.p2.ui/src/org/eclipse/equinox/internal/p2/ui/model/QueriedElement.java b/bundles/org.eclipse.equinox.p2.ui/src/org/eclipse/equinox/internal/p2/ui/model/QueriedElement.java index 4bca06e9a..e729c58ad 100644 --- a/bundles/org.eclipse.equinox.p2.ui/src/org/eclipse/equinox/internal/p2/ui/model/QueriedElement.java +++ b/bundles/org.eclipse.equinox.p2.ui/src/org/eclipse/equinox/internal/p2/ui/model/QueriedElement.java @@ -107,17 +107,21 @@ public abstract class QueriedElement extends ProvElement { Collection<?> results = queryDescriptor.performQuery(monitor); cachedChildren = Collections.unmodifiableCollection(results); if (results.size() > 0) { - Collection<Object> returnedChildren = new HashSet<>(); - returnedChildren.addAll(results); - Object[] siblings = getSiblings(); - for (Object sibling : siblings) { - returnedChildren.remove(sibling); - } - return returnedChildren.toArray(); + return getFilteredChildren(results); } return new Object[0]; } + protected Object[] getFilteredChildren(Collection<?> results) { + Collection<Object> returnedChildren = new HashSet<>(); + returnedChildren.addAll(results); + Object[] siblings = getSiblings(); + for (Object sibling : siblings) { + returnedChildren.remove(sibling); + } + return returnedChildren.toArray(); + } + public void setQueryable(IQueryable<?> queryable) { this.queryable = queryable; } |