Changeset 18691 in josm

Timestamp:

2023-03-14T14:37:10+01:00 (21 months ago)

Author:

taylor.smock

Message:

Significantly reduce cost of large presets like Name Suggestion Index

This reduces the startup CPU of TaggingPresets#initialize by ~29% and the memory
by ~55%. This was done by adding a batch addActiveLayerChangeListeners method
in MainLayerManager.

This additionally reduces the cost of
TaggingPresetSelect$PresetClassification.simplifyString (used by the preset
search dialog) by ~28% CPU cycles and ~58% memory allocations. This was done by
having a single static Pattern which was reused -- most of the remaining
potentially reducible cost for simplifyString is from Matcher initialization.
This would require thread local variables, so may not be worthwhile for now.

Location:

trunk

Files:

• src/org/openstreetmap/josm/gui/layer/MainLayerManager.java (modified) (2 diffs)
• src/org/openstreetmap/josm/gui/tagging/presets/TaggingPresetSelector.java (modified) (3 diffs)
• src/org/openstreetmap/josm/gui/tagging/presets/TaggingPresets.java (modified) (2 diffs)
• test/functional/org/openstreetmap/josm/io/MultiFetchServerObjectReaderTest.java (modified) (1 diff)
• test/performance/org/openstreetmap/josm/io/OsmReaderPerformanceTest.java (modified) (1 diff)
• test/unit/org/openstreetmap/josm/data/validation/routines/DomainValidatorTestIT.java (modified) (1 diff)
• test/unit/org/openstreetmap/josm/gui/datatransfer/ClipboardUtilsTest.java (modified) (1 diff)
• test/unit/org/openstreetmap/josm/gui/datatransfer/LayerTransferableTest.java (modified) (1 diff)

trunk/src/org/openstreetmap/josm/gui/layer/MainLayerManager.java

@@ -169 +169 @@
 
     /**
-     * Adds a active/edit layer change listener
+     * Adds an active/edit layer change listener
      *
      * @param listener the listener.
      */
     public synchronized void addActiveLayerChangeListener(ActiveLayerChangeListener listener) {
-        for (int i = 0; i < activeLayerChangeListeners.size(); i++) {
-            if (activeLayerChangeListeners.get(i) == listener) {
+        for (ActiveLayerChangeListener activeLayerChangeListener : activeLayerChangeListeners) {
+            if (activeLayerChangeListener == listener) {
                 Logging.error("");
                 Logging.error("Attempted to add listener that was already in list: " + listener);
@@ -183 +183 @@
         }
         activeLayerChangeListeners.add(listener);
+    }
+
+    /**
+     * Adds multiple active/edit layer change listeners. Either all listeners are added or none are added.
+     *
+     * @param listeners the listeners.
+     * @return {@code false} if the listener list did not change
+     * @since 18691
+     */
+    public synchronized boolean addActiveLayerChangeListeners(Collection<? extends ActiveLayerChangeListener> listeners) {
+        for (ActiveLayerChangeListener activeLayerChangeListener : activeLayerChangeListeners) {
+            if (listeners.contains(activeLayerChangeListener)) {
+                Logging.error("");
+                Logging.error("Attempted to add listener that was already in list: " + listeners);
+                showStackTrace(Thread.currentThread().getStackTrace());
+                return false;
+            }
+        }
+        return activeLayerChangeListeners.addAll(listeners);
     }
 

trunk/src/org/openstreetmap/josm/gui/tagging/presets/TaggingPresetSelector.java

@@ -18 +18 @@
 import java.util.Objects;
 import java.util.Set;
+import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 
@@ -62 +63 @@
     private static final int CLASSIFICATION_TAGS_MATCH = 100;
 
+    private static final Pattern PATTERN_PUNCTUATION = Pattern.compile("\\p{Punct}", Pattern.UNICODE_CHARACTER_CLASS);
+
     private static final BooleanProperty SEARCH_IN_TAGS = new BooleanProperty("taggingpreset.dialog.search-in-tags", true);
     private static final BooleanProperty ONLY_APPLICABLE = new BooleanProperty("taggingpreset.dialog.only-applicable-to-selection", true);
@@ -136 +139 @@
 
         private static String simplifyString(String s) {
-            return Utils.deAccent(s).toLowerCase(Locale.ENGLISH).replaceAll("\\p{Punct}", "");
+            return PATTERN_PUNCTUATION.matcher(Utils.deAccent(s).toLowerCase(Locale.ENGLISH)).replaceAll("");
         }
 

trunk/src/org/openstreetmap/josm/gui/tagging/presets/TaggingPresets.java

@@ -9 +9 @@
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -88 +89 @@
 
         readFromPreferences();
+        final List<TaggingPreset> activeLayerChangeListeners = new ArrayList<>(taggingPresets.size());
         for (TaggingPreset tp: taggingPresets) {
             if (!(tp instanceof TaggingPresetSeparator)) {
                 MainApplication.getToolbar().register(tp);
-                MainApplication.getLayerManager().addActiveLayerChangeListener(tp);
-            }
-        }
+                activeLayerChangeListeners.add(tp);
+            }
+        }
+        MainApplication.getLayerManager().addActiveLayerChangeListeners(activeLayerChangeListeners);
         if (taggingPresets.isEmpty()) {
             presetsMenu.setVisible(false);

trunk/test/functional/org/openstreetmap/josm/io/MultiFetchServerObjectReaderTest.java

@@ -11 +11 @@
 import java.io.FileInputStream;
 import java.io.FileNotFoundException;
-import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.OutputStreamWriter;

trunk/test/performance/org/openstreetmap/josm/io/OsmReaderPerformanceTest.java

@@ -7 +7 @@
 import java.io.ByteArrayOutputStream;
 import java.io.File;
-import java.io.FileInputStream;
 import java.io.IOException;
 import java.io.InputStream;

trunk/test/unit/org/openstreetmap/josm/data/validation/routines/DomainValidatorTestIT.java

@@ -23 +23 @@
 import java.io.Closeable;
 import java.io.File;
-import java.io.FileInputStream;
 import java.io.FileOutputStream;
 import java.io.IOException;

trunk/test/unit/org/openstreetmap/josm/gui/datatransfer/ClipboardUtilsTest.java

@@ -15 +15 @@
 import java.awt.datatransfer.Transferable;
 import java.awt.datatransfer.UnsupportedFlavorException;
-import java.io.IOException;
 
 import net.trajano.commons.testing.UtilityClassTestUtil;

trunk/test/unit/org/openstreetmap/josm/gui/datatransfer/LayerTransferableTest.java

@@ -16 +16 @@
 import org.junit.jupiter.api.Test;
 import org.openstreetmap.josm.gui.datatransfer.LayerTransferable.Data;
-import org.openstreetmap.josm.gui.layer.Layer;
 import org.openstreetmap.josm.gui.layer.LayerManagerTest;
 import org.openstreetmap.josm.gui.layer.LayerManagerTest.TestLayer;