From 3c5f9d0bc365c009e50b740063f4e96b2b35ff50 Mon Sep 17 00:00:00 2001
From: zzz <zzz@mail.i2p>
Date: Sun, 13 Dec 2015 16:48:04 +0000
Subject: [PATCH] RouterInfo: Optimize writing to avoid extra copy; eliminate
 caching previously enabled for routers with high memory limits Log tweak on
 sig verify fail DataHelper.writeLong() to write(byte) conversion
 DatabaseEntry: Remove deprecated, unused setRoutingKey()

---
 core/java/src/net/i2p/data/DatabaseEntry.java | 10 +--
 .../src/net/i2p/data/router/RouterInfo.java   | 71 ++++++++++---------
 2 files changed, 40 insertions(+), 41 deletions(-)

diff --git a/core/java/src/net/i2p/data/DatabaseEntry.java b/core/java/src/net/i2p/data/DatabaseEntry.java
index 09fc6a50a3..32f358879f 100644
--- a/core/java/src/net/i2p/data/DatabaseEntry.java
+++ b/core/java/src/net/i2p/data/DatabaseEntry.java
@@ -98,6 +98,9 @@ public abstract class DatabaseEntry extends DataStructureImpl {
 
     /**
      * Returns the raw payload data, excluding the signature, to be signed by sign().
+     *
+     * Most callers should use writeBytes() or toByteArray() instead.
+     *
      * FIXME RouterInfo throws DFE and LeaseSet returns null
      * @return null on error ???????????????????????
      */
@@ -122,13 +125,6 @@ public abstract class DatabaseEntry extends DataStructureImpl {
         return _currentRoutingKey;
     }
 
-    /**
-     * @deprecated unused
-     */
-    public void setRoutingKey(Hash key) {
-        _currentRoutingKey = key;
-    }
-
     /**
      * @throws IllegalStateException if not in RouterContext
      */
diff --git a/router/java/src/net/i2p/data/router/RouterInfo.java b/router/java/src/net/i2p/data/router/RouterInfo.java
index ef3fed9fdf..fdfd291365 100644
--- a/router/java/src/net/i2p/data/router/RouterInfo.java
+++ b/router/java/src/net/i2p/data/router/RouterInfo.java
@@ -78,8 +78,11 @@ public class RouterInfo extends DatabaseEntry {
     private volatile boolean _hashCodeInitialized;
     /** should we cache the byte and string versions _byteified ? **/
     private boolean _shouldCache;
-    /** maybe we should check if we are floodfill? */
-    private static final boolean CACHE_ALL = SystemVersion.getMaxMemory() > 128*1024*1024l;
+    /**
+     * Maybe we should check if we are floodfill?
+     * If we do bring this back, don't do on ARM or Android
+     */
+    private static final boolean CACHE_ALL = false; // SystemVersion.getMaxMemory() > 128*1024*1024l;
 
     public static final String PROP_NETWORK_ID = "netId";
     public static final String PROP_CAPABILITIES = "caps";
@@ -308,11 +311,30 @@ public class RouterInfo extends DatabaseEntry {
      */
     protected byte[] getBytes() throws DataFormatException {
         if (_byteified != null) return _byteified;
-        if (_identity == null) throw new DataFormatException("Router identity isn't set?!");
-
-        //long before = Clock.getInstance().now();
         ByteArrayOutputStream out = new ByteArrayOutputStream(2*1024);
         try {
+            writeDataBytes(out);
+        } catch (IOException ioe) {
+            throw new DataFormatException("IO Error getting bytes", ioe);
+        }
+        byte data[] = out.toByteArray();
+        if (CACHE_ALL || _shouldCache)
+            _byteified = data;
+        return data;
+    }
+
+    /** 
+     * Write out the raw payload of the routerInfo, excluding the signature.  This
+     * caches the data in memory if possible.
+     *
+     * @throws DataFormatException if the data is somehow b0rked (missing props, etc)
+     * @throws IOException
+     * @since 0.9.24
+     */
+    private void writeDataBytes(OutputStream out) throws DataFormatException, IOException {
+        if (_identity == null) throw new DataFormatException("Missing identity");
+        if (_published < 0) throw new DataFormatException("Invalid published date: " + _published);
+
             _identity.writeBytes(out);
             // avoid thrashing objects
             //DataHelper.writeDate(out, new Date(_published));
@@ -320,9 +342,9 @@ public class RouterInfo extends DatabaseEntry {
             int sz = _addresses.size();
             if (sz <= 0 || isHidden()) {
                 // Do not send IP address to peers in hidden mode
-                DataHelper.writeLong(out, 1, 0);
+                out.write((byte) 0);
             } else {
-                DataHelper.writeLong(out, 1, sz);
+                out.write((byte) sz);
                 for (RouterAddress addr : _addresses) {
                     addr.writeBytes(out);
                 }
@@ -332,7 +354,7 @@ public class RouterInfo extends DatabaseEntry {
             //         method of trusted links, which isn't implemented in the router
             //         at the moment, and may not be later.
             int psz = _peers == null ? 0 : _peers.size();
-            DataHelper.writeLong(out, 1, psz);
+            out.write((byte) psz);
             if (psz > 0) {
                 Collection<Hash> peers = _peers;
                 if (psz > 1)
@@ -345,17 +367,6 @@ public class RouterInfo extends DatabaseEntry {
                 }
             }
             DataHelper.writeProperties(out, _options);
-        } catch (IOException ioe) {
-            throw new DataFormatException("IO Error getting bytes", ioe);
-        }
-        byte data[] = out.toByteArray();
-        //if (_log.shouldLog(Log.DEBUG)) {
-        //    long after = Clock.getInstance().now();
-        //    _log.debug("getBytes()  took " + (after - before) + "ms");
-        //}
-        if (CACHE_ALL || _shouldCache)
-            _byteified = data;
-        return data;
     }
 
     /**
@@ -487,10 +498,11 @@ public class RouterInfo extends DatabaseEntry {
 
         if (!_isValid) {
             Log log = I2PAppContext.getGlobalContext().logManager().getLog(RouterInfo.class);
-            // TODO change to warn
-            //if (log.shouldWarn()) {
-                log.error("Sig verify fail: " + toString(), new Exception("from"));
-            //}
+            if (log.shouldWarn()) {
+                log.warn("Sig verify fail: " + toString(), new Exception("from"));
+            } else {
+                log.error("RI Sig verify fail: " + _identity.getHash());
+            }
         }
     }
     
@@ -587,18 +599,9 @@ public class RouterInfo extends DatabaseEntry {
      *  This does NOT validate the signature
      */
     public void writeBytes(OutputStream out) throws DataFormatException, IOException {
-        if (_identity == null) throw new DataFormatException("Missing identity");
-        if (_published < 0) throw new DataFormatException("Invalid published date: " + _published);
         if (_signature == null) throw new DataFormatException("Signature is null");
-        //if (!isValid())
-        //    throw new DataFormatException("Data is not valid");
-        ByteArrayOutputStream baos = new ByteArrayOutputStream(2048);
-        baos.write(getBytes());
-        _signature.writeBytes(baos);
-
-        byte data[] = baos.toByteArray();
-        //_log.debug("Writing routerInfo [len=" + data.length + "]: " + toString());
-        out.write(data);
+        writeDataBytes(out);
+        _signature.writeBytes(out);
     }
     
     @Override
-- 
GitLab