Author: tfmorris Date: 2008-08-28 21:31:44-0700 New Revision: 15641 Modified: trunk/src/argouml-app/src/org/argouml/uml/diagram/ui/PropPanelDiagram.java trunk/src/argouml-app/src/org/argouml/uml/ui/UMLComboBoxModel2.java trunk/src/argouml-app/src/org/argouml/uml/ui/foundation/core/UMLGeneralizationPowertypeComboBoxModel.java trunk/src/argouml-app/src/org/argouml/uml/ui/foundation/core/UMLStructuralFeatureTypeComboBoxModel.java
Log: RESOLVED - task 5182: Target selection incredibly slow with large models http://argouml.tigris.org/issues/show_bug.cgi?id=5182 Modified: trunk/src/argouml-app/src/org/argouml/uml/diagram/ui/PropPanelDiagram.java Url: http://argouml.tigris.org/source/browse/argouml/trunk/src/argouml-app/src/org/argouml/uml/diagram/ui/PropPanelDiagram.java?view=diff&rev=15641&p1=trunk/src/argouml-app/src/org/argouml/uml/diagram/ui/PropPanelDiagram.java&p2=trunk/src/argouml-app/src/org/argouml/uml/diagram/ui/PropPanelDiagram.java&r1=15640&r2=15641 ============================================================================== --- trunk/src/argouml-app/src/org/argouml/uml/diagram/ui/PropPanelDiagram.java (original) +++ trunk/src/argouml-app/src/org/argouml/uml/diagram/ui/PropPanelDiagram.java 2008-08-28 21:31:44-0700 @@ -28,6 +28,10 @@ import java.awt.event.ActionEvent; import java.beans.PropertyChangeEvent; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.List; import javax.swing.ImageIcon; import javax.swing.JComboBox; @@ -46,6 +50,7 @@ import org.argouml.uml.ui.UMLComboBoxModel2; import org.argouml.uml.ui.UMLComboBoxNavigator; import org.argouml.uml.ui.UMLSearchableComboBox; +import org.argouml.uml.util.PathComparator; /** * This class represents the properties panel for a Diagram. @@ -112,20 +117,34 @@ @Override protected void buildModelList() { - Object t = getTarget(); - removeAllElements(); - if (t instanceof Relocatable) { - Relocatable diagram = (Relocatable) t; + Object target = getTarget(); + List list = new ArrayList(); + if (target instanceof Relocatable) { + Relocatable diagram = (Relocatable) target; for (Object obj : diagram.getRelocationCandidates( getModelManagementFactory().getRootModel())) { if (diagram.isRelocationAllowed(obj)) { - addElement(obj); + list.add(obj); } } } /* This should not be needed if the above is correct, * but let's be sure: */ - addElement(getSelectedModelElement()); + list.add(getSelectedModelElement()); + Collections.sort(list, new PathComparator()); + setElements(list); + } + + @Override + protected void buildMinimalModelList() { + Collection list = new ArrayList(1); + list.add(getSelectedModelElement()); + setElements(list); + } + + @Override + protected boolean isLazy() { + return true; } @Override Modified: trunk/src/argouml-app/src/org/argouml/uml/ui/UMLComboBoxModel2.java Url: http://argouml.tigris.org/source/browse/argouml/trunk/src/argouml-app/src/org/argouml/uml/ui/UMLComboBoxModel2.java?view=diff&rev=15641&p1=trunk/src/argouml-app/src/org/argouml/uml/ui/UMLComboBoxModel2.java&p2=trunk/src/argouml-app/src/org/argouml/uml/ui/UMLComboBoxModel2.java&r1=15640&r2=15641 ============================================================================== --- trunk/src/argouml-app/src/org/argouml/uml/ui/UMLComboBoxModel2.java (original) +++ trunk/src/argouml-app/src/org/argouml/uml/ui/UMLComboBoxModel2.java 2008-08-28 21:31:44-0700 @@ -28,6 +28,7 @@ import java.beans.PropertyChangeListener; import java.util.ArrayList; import java.util.Collection; +import java.util.LinkedList; import java.util.List; import javax.swing.AbstractListModel; @@ -71,8 +72,10 @@ /** * The list with objects that should be shown in the combobox. + * TODO: Using a list here forces a linear search when we're trying to add + * a new element to the model which can be very slow for large models. */ - private List objects = new ArrayList(); + private List objects = new LinkedList(); /** * The selected object. @@ -110,6 +113,8 @@ */ private boolean processingWillBecomeVisible = false; + private boolean modelValid; + /** * Constructs a model for a combobox. The container given is used @@ -245,8 +250,7 @@ protected void setElements(Collection elements) { if (elements != null) { ArrayList toBeRemoved = new ArrayList(); - for (int i = 0; i < objects.size(); i++) { - Object o = objects.get(i); + for (Object o : objects) { if (!elements.contains(o) && !(isClearable && "".equals(o))) { toBeRemoved.add(o); } @@ -360,6 +364,7 @@ LOG.debug("Ignoring duplicate setTarget request " + theNewTarget); return; } + modelValid = false; LOG.debug("setTarget target : " + theNewTarget); theNewTarget = theNewTarget instanceof Fig ? ((Fig) theNewTarget).getOwner() : theNewTarget; @@ -386,19 +391,12 @@ addOtherModelEventListeners(comboBoxTarget); buildingModel = true; - try { - LOG.debug("Building the combo box model for " + this); - buildMinimalModelList(); - // Do not set buildingModel = false here, - // otherwise the action for selection is performed. - setSelectedItem(getSelectedModelElement()); - } catch (InvalidElementException e) { - LOG.warn("buildModelList attempted to operate on " - + "deleted element"); - } finally { - buildingModel = false; - } - + buildMinimalModelList(); + // Do not set buildingModel = false here, + // otherwise the action for selection is performed. + setSelectedItem(getSelectedModelElement()); + buildingModel = false; + if (getSize() > 0) { fireIntervalAdded(this, 0, getSize() - 1); } @@ -408,8 +406,7 @@ diagram.addPropertyChangeListener( ArgoDiagram.NAMESPACE_KEY, this); buildingModel = true; - LOG.debug("Building the combo box model for " + this); - buildModelList(); + buildMinimalModelList(); setSelectedItem(getSelectedModelElement()); buildingModel = false; if (getSize() > 0) { @@ -432,7 +429,20 @@ * till the list is displayed. */ protected void buildMinimalModelList() { - buildModelList(); + buildModelListTimed(); + } + + private void buildModelListTimed() { + long startTime = System.currentTimeMillis(); + try { + buildModelList(); + long endTime = System.currentTimeMillis(); + LOG.debug("buildModelList took " + (endTime - startTime) + + " msec. for " + this.getClass().getName()); + } catch (InvalidElementException e) { + LOG.warn("buildModelList attempted to operate on " + + "deleted element"); + } } /** @@ -723,8 +733,9 @@ } public void popupMenuWillBecomeVisible(PopupMenuEvent ev) { - if (isLazy() && !processingWillBecomeVisible) { - buildModelList(); + if (isLazy() && !modelValid && !processingWillBecomeVisible) { + buildModelListTimed(); + modelValid = true; // We should be able to just do the above, but Swing has already // computed the size of the popup menu. The rest of this is // a workaround for Swing bug Modified: trunk/src/argouml-app/src/org/argouml/uml/ui/foundation/core/UMLGeneralizationPowertypeComboBoxModel.java Url: http://argouml.tigris.org/source/browse/argouml/trunk/src/argouml-app/src/org/argouml/uml/ui/foundation/core/UMLGeneralizationPowertypeComboBoxModel.java?view=diff&rev=15641&p1=trunk/src/argouml-app/src/org/argouml/uml/ui/foundation/core/UMLGeneralizationPowertypeComboBoxModel.java&p2=trunk/src/argouml-app/src/org/argouml/uml/ui/foundation/core/UMLGeneralizationPowertypeComboBoxModel.java&r1=15640&r2=15641 ============================================================================== --- trunk/src/argouml-app/src/org/argouml/uml/ui/foundation/core/UMLGeneralizationPowertypeComboBoxModel.java (original) +++ trunk/src/argouml-app/src/org/argouml/uml/ui/foundation/core/UMLGeneralizationPowertypeComboBoxModel.java 2008-08-28 21:31:44-0700 @@ -25,13 +25,16 @@ // $header$ package org.argouml.uml.ui.foundation.core; -import java.util.HashSet; +import java.util.ArrayList; +import java.util.Collection; import java.util.Set; +import java.util.TreeSet; import org.argouml.kernel.Project; import org.argouml.kernel.ProjectManager; import org.argouml.model.Model; import org.argouml.uml.ui.UMLComboBoxModel2; +import org.argouml.uml.util.PathComparator; /** * @since Nov 3, 2002 @@ -63,7 +66,7 @@ * @see org.argouml.uml.ui.UMLComboBoxModel2#buildModelList() */ protected void buildModelList() { - Set elements = new HashSet(); + Set<Object> elements = new TreeSet<Object>(new PathComparator()); Project p = ProjectManager.getManager().getCurrentProject(); for (Object model : p.getUserDefinedModelList()) { elements.addAll(Model.getModelManagementHelper() @@ -73,7 +76,24 @@ elements.addAll(p.getProfileConfiguration().findByMetaType( Model.getMetaTypes().getClassifier())); - setElements(elements); + removeAllElements(); + addAll(elements); + } + + @Override + protected void buildMinimalModelList() { + Collection list = new ArrayList(1); + Object element = getSelectedModelElement(); + if (element == null) { + element = " "; + } + list.add(element); + setElements(list); + } + + @Override + protected boolean isLazy() { + return true; } /* Modified: trunk/src/argouml-app/src/org/argouml/uml/ui/foundation/core/UMLStructuralFeatureTypeComboBoxModel.java Url: http://argouml.tigris.org/source/browse/argouml/trunk/src/argouml-app/src/org/argouml/uml/ui/foundation/core/UMLStructuralFeatureTypeComboBoxModel.java?view=diff&rev=15641&p1=trunk/src/argouml-app/src/org/argouml/uml/ui/foundation/core/UMLStructuralFeatureTypeComboBoxModel.java&p2=trunk/src/argouml-app/src/org/argouml/uml/ui/foundation/core/UMLStructuralFeatureTypeComboBoxModel.java&r1=15640&r2=15641 ============================================================================== --- trunk/src/argouml-app/src/org/argouml/uml/ui/foundation/core/UMLStructuralFeatureTypeComboBoxModel.java (original) +++ trunk/src/argouml-app/src/org/argouml/uml/ui/foundation/core/UMLStructuralFeatureTypeComboBoxModel.java 2008-08-28 21:31:44-0700 @@ -1,5 +1,5 @@ // $Id$ -// Copyright (c) 1996-2007 The Regents of the University of California. All +// Copyright (c) 1996-2008 The Regents of the University of California. All // Rights Reserved. Permission to use, copy, modify, and distribute this // software and its documentation without fee, and without a written // agreement is hereby granted, provided that the above copyright notice @@ -25,6 +25,8 @@ package org.argouml.uml.ui.foundation.core; import java.beans.PropertyChangeEvent; +import java.util.ArrayList; +import java.util.Collection; import java.util.Set; import java.util.TreeSet; @@ -91,11 +93,22 @@ elements.addAll(p.getProfileConfiguration().findByMetaType( Model.getMetaTypes().getClassifier())); - // Our comparator will throw an InvalidElementException if the old - // list contains deleted elements (eg after a new project is loaded) - // so remove all the old contents first - removeAllElements(); - addAll(elements); + setElements(elements); + } + + @Override + protected void buildMinimalModelList() { + Collection list = new ArrayList(1); + Object element = getSelectedModelElement(); + if (element != null) { + list.add(element); + } + setElements(list); + } + + @Override + protected boolean isLazy() { + return true; } /* --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
