Changeset 19079 in josm for trunk

Timestamp:

2024-05-14T19:29:53+02:00 (7 weeks ago)

Author:

taylor.smock

Message:

See #23671: Significantly improve performance of Utils.isBlank and Utils.isStripEmpty along with reducing intern cost when loading files

From Utils, isBlank and isStripEmpty are functionally similar; both take a
string and check for the same whitespace characters. Specifically, the code flow
for isBlank calls strip, which uses DEFAULT_STRIP as additional characters
to remove. strip eventually calls isStrippedChar to determine whether or not
to keep a character. isStrippedChar checks DEFAULT_STRIP always, so the
returns from Utils.isBlank and Utils.isStripEmpty should always be the same.

Since the two functions are the same, we change isBlank to point at
isStripEmpty, and isStripEmpty now calls String.isBlank prior to checking
every character. String.isBlank is a built-in method, and is thus more likely
to be well-optimized by the JVM. In the event that it returns false, we want to
avoid any additional work, so we use a traditional for loop instead of a stream;
we will usually know within a character or two whether or not the string is empty.
With a stream, we first have to construct the stream, then iterate through it.

In AbstractReader, we create a map of strings to interned strings to avoid
calling String.intern too often -- despite being something we want to do fairly
frequently. Part of the (current) problem with String.intern is that it uses a
ConcurrentHashTable, so it "costs" more to use. The HashMap (even with the
cost of the lambdas) is ~25% of the CPU cost of interning, given sufficiently
duplicated data.

Location:

trunk/src/org/openstreetmap/josm

Files:

• io/AbstractReader.java (modified) (3 diffs)
• tools/Utils.java (modified) (2 diffs)

trunk/src/org/openstreetmap/josm/io/AbstractReader.java

@@ -88 +88 @@
 
     /**
+     * A lookup table to avoid calling {@link String#intern()} unnecessarily.
+     */
+    private final Map<String, String> tagMap = new HashMap<>();
+
+    /**
      * The dataset to add parsed objects to.
      */
@@ -370 +375 @@
                 }
             }
+            this.tagMap.clear();
             progressMonitor.finishTask();
             progressMonitor.removeCancelListener(cancelListener);
@@ -605 +611 @@
             ((AbstractPrimitive) t).setModified(true);
         } else {
-            t.put(key.intern(), value.intern());
+            t.put(this.tagMap.computeIfAbsent(key, Utils::intern), this.tagMap.computeIfAbsent(value, Utils::intern));
         }
     }

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

@@ -739 +739 @@
      */
     public static boolean isBlank(String string) {
-        return string == null || strip(string).isEmpty();
+        return isStripEmpty(string);
     }
 
@@ -763 +763 @@
      */
     public static boolean isStripEmpty(String str) {
-        return str == null || IntStream.range(0, str.length()).allMatch(i -> isStrippedChar(str.charAt(i), null));
+        if (str != null && !str.isBlank()) {
+            for (int i = 0; i < str.length(); i++) {
+                if (!isStrippedChar(str.charAt(i), null)) {
+                    return false;
+                }
+            }
+        }
+        return true;
     }