Changeset 13121 in josm


Ignore:
Timestamp:
2017-11-13T00:59:15+01:00 (7 days ago)
Author:
Don-vip
Message:

fix #15547 - fix major performance drawback of AutoCompletionSet (regression from r12859)

Location:
trunk/src/org/openstreetmap/josm
Files:
4 edited

Legend:

Unmodified
Added
Removed
  • trunk/src/org/openstreetmap/josm/data/tagging/ac/AutoCompletionItem.java

    r12859 r13121  
    2222    private AutoCompletionPriority priority;
    2323    /** the value of this item */
    24     private String value;
     24    private final String value;
    2525
    2626    /**
     
    7979     * @param value the value; must not be null
    8080     * @throws IllegalArgumentException if value if null
     81     * @deprecated value is now final, set it when constructing the object
    8182     */
     83    @Deprecated
    8284    public void setValue(String value) {
    8385        CheckParameterUtil.ensureParameterNotNull(value, "value");
    84         this.value = value;
     86        throw new UnsupportedOperationException("setValue() is no longer supported");
    8587    }
    8688
  • trunk/src/org/openstreetmap/josm/data/tagging/ac/AutoCompletionSet.java

    r12901 r13121  
    55import java.util.Objects;
    66import java.util.Optional;
     7import java.util.Set;
    78import java.util.TreeSet;
    89import java.util.stream.Collectors;
     
    1819public class AutoCompletionSet extends TreeSet<AutoCompletionItem> {
    1920
     21    // Keep a separate tree set of values for determining fast if a value is present
     22    private final Set<String> values = new TreeSet<>();
     23
    2024    @Override
    2125    public boolean add(AutoCompletionItem e) {
    2226        // Is there already an item for the value?
    23         Optional<AutoCompletionItem> result = stream().filter(i -> i.getValue().equals(e.getValue())).findFirst();
    24         if (result.isPresent()) {
    25             AutoCompletionItem item = result.get();
    26             // yes: merge priorities
    27             AutoCompletionPriority newPriority = item.getPriority().mergeWith(e.getPriority());
    28             // if needed, remove/re-add the updated item to maintain set ordering
    29             if (!item.getPriority().equals(newPriority)) {
    30                 remove(item);
    31                 item.setPriority(newPriority);
    32                 return add(item);
     27        String value = e.getValue();
     28        if (contains(value)) { // Fast
     29            Optional<AutoCompletionItem> result = stream().filter(i -> i.getValue().equals(e.getValue())).findFirst(); // Slow
     30            if (result.isPresent()) {
     31                AutoCompletionItem item = result.get();
     32                // yes: merge priorities
     33                AutoCompletionPriority newPriority = item.getPriority().mergeWith(e.getPriority());
     34                // if needed, remove/re-add the updated item to maintain set ordering
     35                if (!item.getPriority().equals(newPriority)) {
     36                    super.remove(item);
     37                    item.setPriority(newPriority);
     38                    return super.add(item);
     39                } else {
     40                    return false;
     41                }
    3342            } else {
    34                 return false;
     43                // Should never happen if values is correctly synchronized with this set
     44                throw new IllegalStateException(value);
    3545            }
    3646        } else {
     47            values.add(value);
    3748            return super.add(e);
    3849        }
     50    }
     51
     52    @Override
     53    public boolean remove(Object o) {
     54        if (o instanceof AutoCompletionItem) {
     55            values.remove(((AutoCompletionItem) o).getValue());
     56        }
     57        return super.remove(o);
     58    }
     59
     60    @Override
     61    public void clear() {
     62        values.clear();
     63        super.clear();
    3964    }
    4065
     
    7499     */
    75100    public boolean contains(String value) {
    76         return stream().anyMatch(i -> i.getValue().equals(value));
     101        return values.contains(value);
    77102    }
    78103
     
    83108     */
    84109    public boolean remove(String key) {
    85         return removeIf(i -> i.getValue().equals(key));
     110        return values.remove(key) && removeIf(i -> i.getValue().equals(key));
    86111    }
    87112}
  • trunk/src/org/openstreetmap/josm/gui/tagging/ac/AutoCompletionListItem.java

    r12859 r13121  
    8686     */
    8787    public void setValue(String value) {
    88         item.setValue(value);
     88        throw new UnsupportedOperationException("setValue() is no longer supported");
    8989    }
    9090
  • trunk/src/org/openstreetmap/josm/gui/tagging/ac/AutoCompletionManager.java

    r12865 r13121  
    332332
    333333    /**
    334      * Populates the an {@link AutoCompletionList} with the currently cached values for some given tags
     334     * Populates the {@link AutoCompletionList} with the currently cached values for some given tags
    335335     *
    336336     * @param list the list to populate
Note: See TracChangeset for help on using the changeset viewer.