WIP: Improve IPAddress performance and reduce allocations

core/api/src/main/java/org/opennms/core/network/IPAddress.java

@@ -33,22 +33,27 @@
 public class IPAddress implements Comparable<IPAddress> {
     private static final Pattern LEADING_ZEROS = Pattern.compile("^0:[0:]+");
     protected final InetAddress m_inetAddress;
+    private final byte[] m_addressBytes;
 
     public IPAddress(final IPAddress addr) {
         m_inetAddress = addr.m_inetAddress;
+        m_addressBytes = addr.m_addressBytes;
     }
 
     public IPAddress(final String dottedNotation) {
         m_inetAddress = getInetAddress(dottedNotation);
+        m_addressBytes = extractAddressBytes(m_inetAddress);
     }
 
     public IPAddress(final InetAddress inetAddress) {
         m_inetAddress = inetAddress;
+        m_addressBytes = extractAddressBytes(m_inetAddress);
     }
 
     public IPAddress(final byte[] ipAddrOctets) {
         try {
-            m_inetAddress = InetAddress.getByAddress(ipAddrOctets);
+            m_addressBytes = Arrays.copyOf(ipAddrOctets, ipAddrOctets.length);
+            m_inetAddress = InetAddress.getByAddress(m_addressBytes);
         } catch (final UnknownHostException e) {
             throw new IllegalArgumentException("Cannot convert bytes to an InetAddress.", e);
         }
@@ -63,14 +68,14 @@ public InetAddress toInetAddress() {
     }
 
     public byte[] toOctets() {
-        return m_inetAddress.getAddress();
+        return Arrays.copyOf(m_addressBytes, m_addressBytes.length);
     }
 
     @Override
     public boolean equals(final Object obj) {
         if (obj == null) return false;
         if (obj instanceof IPAddress) {
-            return Arrays.equals(m_inetAddress.getAddress(), ((IPAddress) obj).m_inetAddress.getAddress());
+            return Arrays.equals(m_addressBytes, ((IPAddress) obj).m_addressBytes);
         }
         return false;
     }
@@ -82,17 +87,17 @@ public int hashCode() {
 
     @Override
     public int compareTo(final IPAddress o) {
-        return compare(m_inetAddress.getAddress(), o.m_inetAddress.getAddress());
+        return compare(m_addressBytes, o.m_addressBytes);
     }
 
     public String toUserString() {
         if (m_inetAddress instanceof Inet4Address) {
-            return toIpAddrString(m_inetAddress);
+            return toIpAddrString(m_addressBytes);
         } else if (m_inetAddress instanceof Inet6Address) {
             /*
              * <p>From: <a href="https://code.google.com/p/guava-libraries/source/browse/guava/src/com/google/common/primitives/Ints.java">Guava</a>.</p>
              */
-            final byte[] bytes = m_inetAddress.getAddress();
+            final byte[] bytes = m_addressBytes;
             final int[] hextets = new int[8];
             for (int i = 0; i < hextets.length; i++) {
                 hextets[i] = fromBytes(
@@ -116,12 +121,12 @@ public String toString() {
     }
 
     public String toDbString() {
-        return toIpAddrString(m_inetAddress);
+        return toIpAddrString(m_addressBytes);
     }
 
     /** {@inheritDoc} */
     public BigInteger toBigInteger() {
-        return new BigInteger(1, m_inetAddress.getAddress());
+        return new BigInteger(1, m_addressBytes);
     }
 
     /**
@@ -130,7 +135,7 @@ public BigInteger toBigInteger() {
      * @return a {@link org.opennms.core.network.IPAddress} object.
      */
     public IPAddress incr() {
-        final byte[] current = m_inetAddress.getAddress();
+        final byte[] current = m_addressBytes;
         final byte[] b = new byte[current.length];
 
         int carry = 1;
@@ -154,7 +159,7 @@ public IPAddress incr() {
      * @return a {@link org.opennms.core.network.IPAddress} object.
      */
     public IPAddress decr() {
-        final byte[] current = m_inetAddress.getAddress();
+        final byte[] current = m_addressBytes;
         final byte[] b = new byte[current.length];
 
         int borrow = 1;
@@ -180,7 +185,7 @@ public IPAddress decr() {
      * @return a boolean.
      */
     public boolean isPredecessorOf(final IPAddress other) {
-        return other.decr().equals(this);
+        return isImmediateSuccessor(other.m_addressBytes, m_addressBytes);
     }
 
     /**
@@ -190,7 +195,7 @@ public boolean isPredecessorOf(final IPAddress other) {
      * @return a boolean.
      */
     public boolean isSuccessorOf(final IPAddress other) {
-        return other.incr().equals(this);
+        return isImmediateSuccessor(m_addressBytes, other.m_addressBytes);
     }
 
     /**
@@ -272,7 +277,7 @@ protected String toIpAddrString(final InetAddress addr) {
 
     protected String toIpAddrString(final byte[] addr) {
         if (addr.length == 4) {
-            return getInetAddress(addr).getHostAddress();
+            return formatIpv4(addr);
         } else if (addr.length == 16) {
             return String.format("%02x%02x:%02x%02x:%02x%02x:%02x%02x:%02x%02x:%02x%02x:%02x%02x:%02x%02x",
                                  addr[0],
@@ -311,8 +316,15 @@ private InetAddress getInetAddress(final byte[] ipAddrOctets) {
     }
 
     private InetAddress getInetAddress(final String dottedNotation) {
+        if (dottedNotation == null) {
+            return null;
+        }
+        final byte[] ipv4Bytes = tryParseIpv4Bytes(dottedNotation);
+        if (ipv4Bytes != null) {
+            return getInetAddress(ipv4Bytes);
+        }
         try {
-            return dottedNotation == null? null : InetAddress.getByName(dottedNotation);
+            return InetAddress.getByName(dottedNotation);
         } catch (final UnknownHostException e) {
             throw new IllegalArgumentException("Invalid IPAddress " + dottedNotation);
         }
@@ -326,28 +338,87 @@ private int compare(final byte[] a, final byte[] b) {
         } else if (b == null) {
             return 1;
         } else {
-            // Make shorter byte arrays "less than" longer arrays
-            if (a.length < b.length) {
-                return -1;
-            } else if (a.length > b.length) {
-                return 1;
-            } else {
-                // Compare byte-by-byte
-                for (int i = 0; i < a.length; i++) {
-                    final int aInt = unsignedByteToInt(a[i]);
-                    final int bInt = unsignedByteToInt(b[i]);
-                    if (aInt < bInt) {
-                        return -1;
-                    } else if (aInt > bInt) {
-                        return 1;
-                    }
+            final int lengthCmp = Integer.compare(a.length, b.length);
+            if (lengthCmp != 0) {
+                return lengthCmp;
+            }
+            for (int i = 0; i < a.length; i++) {
+                final int byteCmp = Integer.compare(a[i] & 0xFF, b[i] & 0xFF);
+                if (byteCmp != 0) {
+                    return byteCmp;
                 }
-                // OK both arrays are the same length and every byte is identical so they are equal
-                return 0;
             }
+            return 0;
         }
     }
 
+    private static boolean isImmediateSuccessor(final byte[] successor, final byte[] base) {
+        if (successor.length != base.length) {
+            return false;
+        }
+        int carry = 1;
+        for (int i = base.length - 1; i >= 0; i--) {
+            final int sum = (base[i] & 0xFF) + carry;
+            if ((successor[i] & 0xFF) != (sum & 0xFF)) {
+                return false;
+            }
+            carry = sum >> 8;
+        }
+        return carry == 0;
+    }
+
+    private static String formatIpv4(final byte[] addr) {
+        return new StringBuilder(15)
+                .append(addr[0] & 0xFF).append('.')
+                .append(addr[1] & 0xFF).append('.')
+                .append(addr[2] & 0xFF).append('.')
+                .append(addr[3] & 0xFF)
+                .toString();
+    }
+
+    /**
+     * Parse a dotted-decimal IPv4 literal without invoking the JDK name service.
+     *
+     * @return four address octets, or {@code null} if the input is not a strict
+     *         decimal IPv4 literal (caller should fall back to {@link InetAddress#getByName(String)}).
+     */
+    private static byte[] tryParseIpv4Bytes(final String s) {
+        if (s == null || s.isEmpty()) {
+            return null;
+        }
+        final byte[] result = new byte[4];
+        int part = 0;
+        int value = 0;
+        boolean hasDigit = false;
+        for (int i = 0; i < s.length(); i++) {
+            final char c = s.charAt(i);
+            if (c == '.') {
+                if (!hasDigit || value > 255 || part >= 3) {
+                    return null;
+                }
+                result[part++] = (byte) value;
+                value = 0;
+                hasDigit = false;
+            } else if (c >= '0' && c <= '9') {
+                if (!hasDigit && c == '0' && i + 1 < s.length() && s.charAt(i + 1) != '.') {
+                    return null;
+                }
+                hasDigit = true;
+                value = value * 10 + (c - '0');
+                if (value > 255) {
+                    return null;
+                }
+            } else {
+                return null;
+            }
+        }
+        if (!hasDigit || value > 255 || part != 3) {
+            return null;
+        }
+        result[3] = (byte) value;
+        return result;
+    }
+
     /**
      * Returns the {@code int} value whose byte representation is the given 4
      * bytes, in big-endian order; equivalent to {@code Ints.fromByteArray(new
@@ -430,7 +501,14 @@ private static String hextetsToIPv6String(final int[] hextets) {
         return matcher.replaceAll(":");
     }
 
-    private int unsignedByteToInt(final byte b) {
-        return b < 0 ? ((int)b)+256 : ((int)b);
+    private static byte[] extractAddressBytes(final InetAddress addr) {
+        if (addr == null) {
+            throw new IllegalArgumentException("Cannot convert null InetAddress to a byte array");
+        }
+        final byte[] address = addr.getAddress();
+        if (address == null) {
+            throw new IllegalArgumentException("InetAddress instance violates contract by returning a null address from getAddress()");
+        }
+        return address;
     }
 }

core/api/src/test/java/org/opennms/core/network/IPAddressTest.java

@@ -0,0 +1,93 @@
+/*
+ * Licensed to The OpenNMS Group, Inc (TOG) under one or more
+ * contributor license agreements.  See the LICENSE.md file
+ * distributed with this work for additional information
+ * regarding copyright ownership.
+ *
+ * TOG licenses this file to You under the GNU Affero General
+ * Public License Version 3 (the "License") or (at your option)
+ * any later version.  You may not use this file except in
+ * compliance with the License.  You may obtain a copy of the
+ * License at:
+ *
+ *      https://www.gnu.org/licenses/agpl-3.0.txt
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND,
+ * either express or implied.  See the License for the specific
+ * language governing permissions and limitations under the
+ * License.
+ */
+package org.opennms.core.network;
+
+import static org.junit.Assert.assertArrayEquals;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import java.net.InetAddress;
+
+import org.junit.Test;
+
+public class IPAddressTest {
+
+    @Test
+    public void shouldReturnDefensiveCopyFromToOctets() {
+        final IPAddress ipAddress = new IPAddress("192.0.2.10");
+        final byte[] octets = ipAddress.toOctets();
+        octets[0] = 0;
+
+        // Mutating the returned array must not alter internal state.
+        assertEquals("192.0.2.10", ipAddress.toDbString());
+        assertTrue(ipAddress.equals(new IPAddress("192.0.2.10")));
+    }
+
+    @Test
+    public void shouldIncrementAndDecrementUsingCachedBytes() {
+        final IPAddress ipAddress = new IPAddress("192.0.2.10");
+
+        final IPAddress incremented = ipAddress.incr();
+        final IPAddress decremented = incremented.decr();
+
+        assertEquals(ipAddress, decremented);
+        assertArrayEquals(new byte[] {(byte) 192, 0, 2, 11}, incremented.toOctets());
+    }
+
+    @Test
+    public void shouldFormatIpv4FromBytesWithoutInetAddressRoundTrip() {
+        final IPAddress ipAddress = new IPAddress(new byte[] {(byte) 192, 0, 2, 10});
+        assertEquals("192.0.2.10", ipAddress.toDbString());
+        assertEquals("192.0.2.10", ipAddress.toUserString());
+    }
+
+    @Test
+    public void shouldParseIpv4LiteralsWithoutNameService() throws Exception {
+        final IPAddress fast = new IPAddress("10.20.30.40");
+        final IPAddress jdk = new IPAddress(InetAddress.getByName("10.20.30.40"));
+        assertEquals(jdk, fast);
+        assertEquals("10.20.30.40", fast.toDbString());
+    }
+
+    @Test
+    public void shouldFallBackToNameServiceForNonDecimalIpv4Literals() throws Exception {
+        final IPAddress ipv6 = new IPAddress("::1");
+        assertTrue(ipv6.toInetAddress() instanceof java.net.Inet6Address);
+    }
+
+    @Test
+    public void shouldDetectImmediatePredecessorAndSuccessorWithoutAllocating() {
+        final IPAddress zero = new IPAddress("0.0.0.0");
+        final IPAddress one = new IPAddress("0.0.0.1");
+        final IPAddress two = new IPAddress("0.0.0.2");
+
+        assertTrue(zero.isPredecessorOf(one));
+        assertTrue(one.isSuccessorOf(zero));
+        assertTrue(one.isPredecessorOf(two));
+        assertTrue(two.isSuccessorOf(one));
+
+        assertFalse(one.isPredecessorOf(one));
+        assertFalse(one.isSuccessorOf(one));
+        assertFalse(zero.isSuccessorOf(two));
+    }
+}

core/lib/src/main/java/org/opennms/core/utils/ByteArrayComparator.java

@@ -42,13 +42,13 @@ public int compare(byte[] a, byte[] b) {
             return 1;
         } else {
             // Make shorter byte arrays "less than" longer arrays
-            int comparison = Integer.valueOf(a.length).compareTo(Integer.valueOf(b.length));
+            int comparison = Integer.compare(a.length, b.length);
             if (comparison != 0) {
                 return comparison;
             } else {
                 // Compare byte-by-byte
                 for (int i = 0; i < a.length; i++) {
-                    int byteComparison = Integer.valueOf(unsignedByteToInt(a[i])).compareTo(Integer.valueOf(unsignedByteToInt(b[i])));
+                    int byteComparison = Integer.compare(unsignedByteToInt(a[i]), unsignedByteToInt(b[i]));
                     if (byteComparison != 0) {
                         return byteComparison;
                     }