avoid any extra memory allocation in Version.fromString()

as this is called a lot, it's (apparently, Java Mission Control on a
scale lab test shows) worth it to avoid the RegExp Matcher and x3
intermediate String object allocations, which we can safe here by
implementing this a little smarter (lower level; feels like 6502!).

JIRA: OVSDB-469
Change-Id: I642e0bfd698e594d3c263f0a335b92f80fbde8c4
Signed-off-by: Michael Vorburger <vorburger@redhat.com>

diff --git a/library/impl/src/main/java/org/opendaylight/ovsdb/lib/notation/Version.java b/library/impl/src/main/java/org/opendaylight/ovsdb/lib/notation/Version.java
index 6d6d6827a01f784775345ae85f1f6cddd8742668..edcadf21299881a8d73e5013d567c524e24712a3 100644 (file)
--- a/library/impl/src/main/java/org/opendaylight/ovsdb/lib/notation/Version.java
+++ b/library/impl/src/main/java/org/opendaylight/ovsdb/lib/notation/Version.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014, 2015 Red Hat, Inc. and others. All rights reserved.
+ * Copyright (c) 2014, 2018 Red Hat, Inc. and others. All rights reserved.
  *
  * This program and the accompanying materials are made available under the
  * terms of the Eclipse Public License v1.0 which accompanies this distribution,
@@ -8,8 +8,7 @@
 
 package org.opendaylight.ovsdb.lib.notation;
 
-import java.util.regex.Matcher;
-import java.util.regex.Pattern;
+import com.google.errorprone.annotations.Var;
 
 /**
  * This class represents a version according to RFC 7047.
@@ -18,7 +17,6 @@ import java.util.regex.Pattern;
  */
 public class Version implements Comparable<Version> {
     private static final String FORMAT = "(\\d+)\\.(\\d+)\\.(\\d+)";
-    private static final Pattern PATTERN = Pattern.compile(Version.FORMAT);
 
     private int major;
     private int minor;
@@ -34,16 +32,38 @@ public class Version implements Comparable<Version> {
     public static final String NULL_VERSION_STRING = "0.0.0";
 
     public static Version fromString(String version) {
-        final Matcher matcher = Version.PATTERN.matcher(version);
-        if (!matcher.find()) {
+        final int firstDot = version.indexOf('.');
+        final int secondDot = version.indexOf('.', firstDot + 1);
+        if (firstDot == -1 || secondDot == -1) {
             throw new IllegalArgumentException("<" + version + "> does not match format " + Version.FORMAT);
         }
-        int major = Integer.parseInt(matcher.group(1));
-        int minor = Integer.parseInt(matcher.group(2));
-        int patch = Integer.parseInt(matcher.group(3));
+        final int major = parse(version, 0, firstDot);
+        final int minor = parse(version, firstDot + 1, secondDot);
+        final int patch = parse(version, secondDot + 1, version.length());
         return new Version(major, minor, patch);
     }
 
+    /**
+     * Parses the string argument from position 'start' to 'end' as a signed decimal integer.
+     * We use a custom hand written method instead of {@link Integer#parseInt(String)}
+     * just to avoid allocating three intermediate String objects in {@link #fromString(String)},
+     * as objection allocation data in Java Mission Control from ODL running in a scale lab
+     * has identified this as the top #3 (!) memory allocator overall - 1 GB avoidable String.
+     * @author Michael Vorburger.ch
+     */
+    private static int parse(String string, int start, int end) {
+        @Var int result = 0;
+        for (int i = start; i < end && i < string.length(); i++) {
+            char character = string.charAt(i);
+            int digit = Character.digit(character, 10);
+            if (digit < 0) {
+                throw new IllegalArgumentException("Not a digit: " + character);
+            }
+            result = result * 10 + digit;
+        }
+        return result;
+    }
+
     @Override
     public String toString() {
         return "" + major + "." + minor + "." + patch;

diff --git a/library/impl/src/test/java/org/opendaylight/ovsdb/lib/notation/VersionTest.java b/library/impl/src/test/java/org/opendaylight/ovsdb/lib/notation/VersionTest.java
index bc68ab5cbee28bcfc4013b78f8b908db73318d68..36a12339059d39a659486bd8075cce72a44bc5a6 100755 (executable)
--- a/library/impl/src/test/java/org/opendaylight/ovsdb/lib/notation/VersionTest.java
+++ b/library/impl/src/test/java/org/opendaylight/ovsdb/lib/notation/VersionTest.java
@@ -21,8 +21,10 @@ public class VersionTest {
      */
     @Test
     public void testToString() throws Exception {
-        Version version123 = Version.fromString("1.2.3");
-        assertEquals("1.2.3", version123.toString());
+        assertEquals("0.0.0", Version.fromString("0.0.0").toString());
+        assertEquals("1.2.3", Version.fromString("1.2.3").toString());
+        assertEquals("1.2.3", Version.fromString("01.2.003").toString());
+        assertEquals("12.34.56", Version.fromString("12.34.56").toString());
     }
 
     /**