Changeset 13121 in josm for trunk/src

Timestamp:

2017-11-13T00:59:15+01:00 (7 years ago)

Author:

Don-vip

Message:

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

Location:

trunk/src/org/openstreetmap/josm

Files:

• data/tagging/ac/AutoCompletionItem.java (modified) (2 diffs)
• data/tagging/ac/AutoCompletionSet.java (modified) (4 diffs)
• gui/tagging/ac/AutoCompletionListItem.java (modified) (1 diff)
• gui/tagging/ac/AutoCompletionManager.java (modified) (1 diff)

trunk/src/org/openstreetmap/josm/data/tagging/ac/AutoCompletionItem.java

@@ -22 +22 @@
     private AutoCompletionPriority priority;
     /** the value of this item */
-    private String value;
+    private final String value;
 
     /**
@@ -79 +79 @@
      * @param value the value; must not be null
      * @throws IllegalArgumentException if value if null
+     * @deprecated value is now final, set it when constructing the object
      */
+    @Deprecated
     public void setValue(String value) {
         CheckParameterUtil.ensureParameterNotNull(value, "value");
-        this.value = value;
+        throw new UnsupportedOperationException("setValue() is no longer supported");
     }
 

trunk/src/org/openstreetmap/josm/data/tagging/ac/AutoCompletionSet.java

@@ -5 +5 @@
 import java.util.Objects;
 import java.util.Optional;
+import java.util.Set;
 import java.util.TreeSet;
 import java.util.stream.Collectors;
@@ -18 +19 @@
 public class AutoCompletionSet extends TreeSet<AutoCompletionItem> {
 
+    // Keep a separate tree set of values for determining fast if a value is present
+    private final Set<String> values = new TreeSet<>();
+
     @Override
     public boolean add(AutoCompletionItem e) {
         // Is there already an item for the value?
-        Optional<AutoCompletionItem> result = stream().filter(i -> i.getValue().equals(e.getValue())).findFirst();
-        if (result.isPresent()) {
-            AutoCompletionItem item = result.get();
-            // yes: merge priorities
-            AutoCompletionPriority newPriority = item.getPriority().mergeWith(e.getPriority());
-            // if needed, remove/re-add the updated item to maintain set ordering
-            if (!item.getPriority().equals(newPriority)) {
-                remove(item);
-                item.setPriority(newPriority);
-                return add(item);
+        String value = e.getValue();
+        if (contains(value)) { // Fast
+            Optional<AutoCompletionItem> result = stream().filter(i -> i.getValue().equals(e.getValue())).findFirst(); // Slow
+            if (result.isPresent()) {
+                AutoCompletionItem item = result.get();
+                // yes: merge priorities
+                AutoCompletionPriority newPriority = item.getPriority().mergeWith(e.getPriority());
+                // if needed, remove/re-add the updated item to maintain set ordering
+                if (!item.getPriority().equals(newPriority)) {
+                    super.remove(item);
+                    item.setPriority(newPriority);
+                    return super.add(item);
+                } else {
+                    return false;
+                }
             } else {
-                return false;
+                // Should never happen if values is correctly synchronized with this set
+                throw new IllegalStateException(value);
             }
         } else {
+            values.add(value);
             return super.add(e);
         }
+    }
+
+    @Override
+    public boolean remove(Object o) {
+        if (o instanceof AutoCompletionItem) {
+            values.remove(((AutoCompletionItem) o).getValue());
+        }
+        return super.remove(o);
+    }
+
+    @Override
+    public void clear() {
+        values.clear();
+        super.clear();
     }
 
@@ -74 +99 @@
      */
     public boolean contains(String value) {
-        return stream().anyMatch(i -> i.getValue().equals(value));
+        return values.contains(value);
     }
 
@@ -83 +108 @@
      */
     public boolean remove(String key) {
-        return removeIf(i -> i.getValue().equals(key));
+        return values.remove(key) && removeIf(i -> i.getValue().equals(key));
     }
 }

trunk/src/org/openstreetmap/josm/gui/tagging/ac/AutoCompletionListItem.java

@@ -86 +86 @@
      */
     public void setValue(String value) {
-        item.setValue(value);
+        throw new UnsupportedOperationException("setValue() is no longer supported");
     }
 

trunk/src/org/openstreetmap/josm/gui/tagging/ac/AutoCompletionManager.java

@@ -332 +332 @@
 
     /**
-     * Populates the an {@link AutoCompletionList} with the currently cached values for some given tags
+     * Populates the {@link AutoCompletionList} with the currently cached values for some given tags
      *
      * @param list the list to populate