Ticket #23097: 23097.patch

File 23097.patch, 6.3 KB (added by taylor.smock, 16 months ago)
  • src/org/openstreetmap/josm/plugins/PluginListParser.java

    Subject: [PATCH] Fix #23097: Improve CPU usage and memory allocations during startup
    
    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).
    
    Note: I'm also including a change in `PluginListParser` to avoid compiling a pattern over and over again. That reduces the cost of `PluginListParser.parse` from 25.5 mb to 12.1 mb and 217ms to 162ms. Most of the remaining cost is stuff we cannot do anything about.
    
    Additional note: The PluginListParser numbers included the cost of interning the strings. I ended up removing that since code analysis indicated that the strings were not kept long-term.
    ---
    IDEA additional info:
    Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
    <+>UTF-8
    diff --git a/src/org/openstreetmap/josm/plugins/PluginListParser.java b/src/org/openstreetmap/josm/plugins/PluginListParser.java
    a b  
    1111import java.util.LinkedList;
    1212import java.util.List;
    1313import java.util.jar.Attributes;
     14import java.util.regex.Matcher;
     15import java.util.regex.Pattern;
    1416
    1517import org.openstreetmap.josm.tools.Logging;
    1618
     
    6264            String name = null;
    6365            String url = null;
    6466            Attributes manifest = new Attributes();
     67            final Pattern spaceColonSpace = Pattern.compile("\\s*:\\s*", Pattern.UNICODE_CHARACTER_CLASS);
     68            final Matcher matcher = spaceColonSpace.matcher("");
    6569            for (String line = r.readLine(); line != null; line = r.readLine()) {
    6670                if (line.startsWith("\t")) {
    67                     final String[] keyValue = line.split("\\s*:\\s*", 2);
    68                     if (keyValue.length >= 2)
    69                         manifest.put(new Attributes.Name(keyValue[0].substring(1)), keyValue[1]);
     71                    matcher.reset(line);
     72                    if (matcher.find() && matcher.start() > 0 && matcher.end() < line.length()) {
     73                        final String key = line.substring(1, matcher.start());
     74                        final String value = line.substring(matcher.end());
     75                        manifest.put(new Attributes.Name(key), value);
     76                    }
    7077                    continue;
    7178                }
    7279                addPluginInformation(ret, name, url, manifest);
  • src/org/openstreetmap/josm/tools/XmlObjectParser.java

    IDEA additional info:
    Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
    <+>UTF-8
    diff --git a/src/org/openstreetmap/josm/tools/XmlObjectParser.java b/src/org/openstreetmap/josm/tools/XmlObjectParser.java
    a b  
    99import java.lang.reflect.Field;
    1010import java.lang.reflect.Method;
    1111import java.lang.reflect.Modifier;
    12 import java.util.Arrays;
    1312import java.util.HashMap;
    1413import java.util.Iterator;
    1514import java.util.LinkedList;
     
    189188        private final boolean both;
    190189        private final Map<String, Field> fields = new HashMap<>();
    191190        private final Map<String, Method> methods = new HashMap<>();
     191        /** This is used to avoid array copies in {@link #getUncachedMethod(String)}. Do not modify. */
     192        private Method[] cachedKlassMethods;
     193        /** This is used to avoid array copies in {@link #getUncachedField(String)}. Do not modify. */
     194        private Field[] cachedKlassFields;
    192195
    193196        Entry(Class<?> klass, boolean onStart, boolean both) {
    194197            this.klass = klass;
     
    197200        }
    198201
    199202        Field getField(String s) {
    200             return fields.computeIfAbsent(s, ignore -> Arrays.stream(klass.getFields())
    201                     .filter(f -> f.getName().equals(s))
    202                     .findFirst()
    203                     .orElse(null));
     203            return fields.computeIfAbsent(s, this::getUncachedField);
     204        }
     205
     206        /**
     207         * Get a field (uncached in {@link #fields})
     208         * @implNote Please profile startup when changing
     209         * @param s The field to get
     210         * @return The field, or {@code null}.
     211         */
     212        private Field getUncachedField(String s) {
     213            if (this.cachedKlassFields == null) {
     214                this.cachedKlassFields = klass.getFields();
     215            }
     216            for (Field field : this.cachedKlassFields) {
     217                if (field.getName().equals(s)) {
     218                    return field;
     219                }
     220            }
     221            return null;
    204222        }
    205223
    206224        Method getMethod(String s) {
    207             return methods.computeIfAbsent(s, ignore -> Arrays.stream(klass.getMethods())
    208                     .filter(m -> m.getName().equals(s) && m.getParameterTypes().length == 1)
    209                     .findFirst()
    210                     .orElse(null));
     225            return methods.computeIfAbsent(s, this::getUncachedMethod);
     226        }
     227
     228        /**
     229         * Get an uncached method (in {@link #methods})
     230         * @implNote Please profile startup when changing
     231         * @param s The method to find
     232         * @return The method or {@code null}.
     233         */
     234        private Method getUncachedMethod(String s) {
     235            if (cachedKlassMethods == null) {
     236                cachedKlassMethods = klass.getMethods();
     237            }
     238            for (Method method : cachedKlassMethods) {
     239                if (method.getParameterCount() == 1 && method.getName().equals(s)) {
     240                    return method;
     241                }
     242            }
     243            return null;
    211244        }
    212245    }
    213246