Changeset 18796 in josm for trunk

Timestamp:

2023-08-07T22:04:55+02:00 (11 months ago)

Author:

taylor.smock

Message:

Fix #23097: Significantly reduce allocations during startup in XmlObjectParser, PluginListParser, and ReadLocalPluginInformationTask

With the Name Suggestion Index preset added to JOSM, the following methods are
relatively expensive during startup (mem old -> mem new, cpu old -> cpu new):

• XmlObjectParser$Entry.getField (124 MB -> 8.1 MB, 501ms -> 99ms)
• XmlObjectParser$Entry.getMethod (126 MB -> 452 kB, 292ms -> 45ms)

The gains are almost entirely from getting rid of copy calls to Method and Field
(done when calling Class.getMethods() and Class.getFields()). There are
further gains in JVM methods (like GC), but those can be a bit ticklish to
profile correctly. It does look like a 20% improvement there though
(32,653ms -> 26,075ms).

This additionally reduces pattern compilation from PluginListParser.parse and
ReadLocalPluginInformationTask.listFiles.

Location:

trunk/src/org/openstreetmap/josm

Files:

• plugins/PluginListParser.java (modified) (2 diffs)
• plugins/ReadLocalPluginInformationTask.java (modified) (2 diffs)
• tools/XmlObjectParser.java (modified) (3 diffs)

trunk/src/org/openstreetmap/josm/plugins/PluginListParser.java

@@ -12 +12 @@
 import java.util.List;
 import java.util.jar.Attributes;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 
 import org.openstreetmap.josm.tools.Logging;
@@ -63 +65 @@
             String url = null;
             Attributes manifest = new Attributes();
+            final Pattern spaceColonSpace = Pattern.compile("\\s*:\\s*", Pattern.UNICODE_CHARACTER_CLASS);
+            final Matcher matcher = spaceColonSpace.matcher("");
             for (String line = r.readLine(); line != null; line = r.readLine()) {
                 if (line.startsWith("\t")) {
-                    final String[] keyValue = line.split("\\s*:\\s*", 2);
-                    if (keyValue.length >= 2)
-                        manifest.put(new Attributes.Name(keyValue[0].substring(1)), keyValue[1]);
+                    matcher.reset(line);
+                    if (matcher.find() && matcher.start() > 0 && matcher.end() < line.length()) {
+                        final String key = line.substring(1, matcher.start());
+                        final String value = line.substring(matcher.end());
+                        manifest.put(new Attributes.Name(key), value);
+                    }
                     continue;
                 }

trunk/src/org/openstreetmap/josm/plugins/ReadLocalPluginInformationTask.java

@@ -14 +14 @@
 import java.util.List;
 import java.util.Map;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 
 import org.openstreetmap.josm.data.Preferences;
@@ -83 +85 @@
 
     private static File[] listFiles(File pluginsDirectory, final String regex) {
-        return pluginsDirectory.listFiles((FilenameFilter) (dir, name) -> name.matches(regex));
+        final Matcher matcher = Pattern.compile(regex).matcher("");
+        return pluginsDirectory.listFiles((dir, name) -> matcher.reset(name).matches());
     }
 

trunk/src/org/openstreetmap/josm/tools/XmlObjectParser.java

@@ -10 +10 @@
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
-import java.util.Arrays;
 import java.util.HashMap;
 import java.util.Iterator;
@@ -190 +189 @@
         private final Map<String, Field> fields = new HashMap<>();
         private final Map<String, Method> methods = new HashMap<>();
+        /** This is used to avoid array copies in {@link #getUncachedMethod(String)}. Do not modify. */
+        private Method[] cachedKlassMethods;
+        /** This is used to avoid array copies in {@link #getUncachedField(String)}. Do not modify. */
+        private Field[] cachedKlassFields;
 
         Entry(Class<?> klass, boolean onStart, boolean both) {
@@ -198 +201 @@
 
         Field getField(String s) {
-            return fields.computeIfAbsent(s, ignore -> Arrays.stream(klass.getFields())
-                    .filter(f -> f.getName().equals(s))
-                    .findFirst()
-                    .orElse(null));
+            return fields.computeIfAbsent(s, this::getUncachedField);
+        }
+
+        /**
+         * Get a field (uncached in {@link #fields})
+         * @implNote Please profile startup when changing
+         * @param s The field to get
+         * @return The field, or {@code null}.
+         */
+        private Field getUncachedField(String s) {
+            if (this.cachedKlassFields == null) {
+                this.cachedKlassFields = klass.getFields();
+            }
+            for (Field field : this.cachedKlassFields) {
+                if (field.getName().equals(s)) {
+                    return field;
+                }
+            }
+            return null;
         }
 
         Method getMethod(String s) {
-            return methods.computeIfAbsent(s, ignore -> Arrays.stream(klass.getMethods())
-                    .filter(m -> m.getName().equals(s) && m.getParameterTypes().length == 1)
-                    .findFirst()
-                    .orElse(null));
+            return methods.computeIfAbsent(s, this::getUncachedMethod);
+        }
+
+        /**
+         * Get an uncached method (in {@link #methods})
+         * @implNote Please profile startup when changing
+         * @param s The method to find
+         * @return The method or {@code null}.
+         */
+        private Method getUncachedMethod(String s) {
+            if (cachedKlassMethods == null) {
+                cachedKlassMethods = klass.getMethods();
+            }
+            for (Method method : cachedKlassMethods) {
+                if (method.getParameterCount() == 1 && method.getName().equals(s)) {
+                    return method;
+                }
+            }
+            return null;
         }
     }