Bug 550265 - UI freeze in Installed Software tab
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>
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 7fd3dbb..291e66a 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.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 @@
/**
* 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 @@
// 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 @@
// 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 6154802..3fb7146 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 @@
@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 4bca06e..e729c58 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 @@
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;
}
