From 707bfbbf8b80ee2ef1d8d5234bb5bc608dfa7181 Mon Sep 17 00:00:00 2001 From: zzz Date: Sun, 3 May 2015 17:00:22 +0000 Subject: [PATCH] Router: Put the GarlicMessageParser in the RouterContext to reduce object churn, we only need one Add DeliveryInstructions.create() to return immmutable local instructions, to reduce object churn Minor cleanups --- .../i2p/data/i2np/DeliveryInstructions.java | 13 ++++++++++++ .../src/net/i2p/data/i2np/GarlicClove.java | 4 ++-- .../src/net/i2p/router/RouterContext.java | 13 ++++++++++++ .../router/message/GarlicMessageParser.java | 21 ++++++++++++------- .../router/message/GarlicMessageReceiver.java | 13 ++++++------ 5 files changed, 47 insertions(+), 17 deletions(-) diff --git a/router/java/src/net/i2p/data/i2np/DeliveryInstructions.java b/router/java/src/net/i2p/data/i2np/DeliveryInstructions.java index 2126fd397..e4e32377c 100644 --- a/router/java/src/net/i2p/data/i2np/DeliveryInstructions.java +++ b/router/java/src/net/i2p/data/i2np/DeliveryInstructions.java @@ -63,6 +63,19 @@ public class DeliveryInstructions extends DataStructureImpl { */ public static final DeliveryInstructions LOCAL = new LocalInstructions(); + /** + * Returns immutable local instructions, or new + * + * @since 0.9.20 + */ + public static DeliveryInstructions create(byte[] data, int offset) throws DataFormatException { + if (data[offset] == 0) + return LOCAL; + DeliveryInstructions rv = new DeliveryInstructions(); + rv.readBytes(data, offset); + return rv; + } + public DeliveryInstructions() { _deliveryMode = -1; } diff --git a/router/java/src/net/i2p/data/i2np/GarlicClove.java b/router/java/src/net/i2p/data/i2np/GarlicClove.java index 378b9041a..628874d81 100644 --- a/router/java/src/net/i2p/data/i2np/GarlicClove.java +++ b/router/java/src/net/i2p/data/i2np/GarlicClove.java @@ -89,8 +89,8 @@ public class GarlicClove extends DataStructureImpl { */ public int readBytes(byte source[], int offset) throws DataFormatException { int cur = offset; - _instructions = new DeliveryInstructions(); - cur += _instructions.readBytes(source, cur); + _instructions = DeliveryInstructions.create(source, offset); + cur += _instructions.getSize(); //if (_log.shouldLog(Log.DEBUG)) // _log.debug("Read instructions: " + _instructions); try { diff --git a/router/java/src/net/i2p/router/RouterContext.java b/router/java/src/net/i2p/router/RouterContext.java index dbccc14bb..31dbb49b3 100644 --- a/router/java/src/net/i2p/router/RouterContext.java +++ b/router/java/src/net/i2p/router/RouterContext.java @@ -17,6 +17,7 @@ import net.i2p.internal.InternalClientManager; import net.i2p.router.client.ClientManagerFacadeImpl; import net.i2p.router.crypto.TransientSessionKeyManager; import net.i2p.router.dummy.*; +import net.i2p.router.message.GarlicMessageParser; import net.i2p.router.networkdb.kademlia.FloodfillNetworkDatabaseFacade; import net.i2p.router.peermanager.PeerManagerFacadeImpl; import net.i2p.router.peermanager.ProfileManagerImpl; @@ -68,6 +69,7 @@ public class RouterContext extends I2PAppContext { private RouterThrottle _throttle; private RouterAppManager _appManager; private RouterKeyGenerator _routingKeyGenerator; + private GarlicMessageParser _garlicMessageParser; private final Set _finalShutdownTasks; // split up big lock on this to avoid deadlocks private volatile boolean _initialized; @@ -179,6 +181,7 @@ public class RouterContext extends I2PAppContext { _clientManagerFacade = new DummyClientManagerFacade(this); // internal client manager is null } + _garlicMessageParser = new GarlicMessageParser(this); _clientMessagePool = new ClientMessagePool(this); _jobQueue = new JobQueue(this); _jobQueue.startup(); @@ -621,4 +624,14 @@ public class RouterContext extends I2PAppContext { public RouterKeyGenerator routerKeyGenerator() { return _routingKeyGenerator; } + + /** + * Since we only need one. + * + * @return non-null after initAll() + * @since 0.9.20 + */ + public GarlicMessageParser garlicMessageParser() { + return _garlicMessageParser; + } } diff --git a/router/java/src/net/i2p/router/message/GarlicMessageParser.java b/router/java/src/net/i2p/router/message/GarlicMessageParser.java index 11b0d40c9..df83702a2 100644 --- a/router/java/src/net/i2p/router/message/GarlicMessageParser.java +++ b/router/java/src/net/i2p/router/message/GarlicMessageParser.java @@ -10,6 +10,7 @@ package net.i2p.router.message; import java.util.Date; +import net.i2p.I2PAppContext; import net.i2p.crypto.SessionKeyManager; import net.i2p.data.Certificate; import net.i2p.data.DataFormatException; @@ -17,16 +18,16 @@ import net.i2p.data.DataHelper; import net.i2p.data.PrivateKey; import net.i2p.data.i2np.GarlicClove; import net.i2p.data.i2np.GarlicMessage; -import net.i2p.router.RouterContext; import net.i2p.util.Log; /** - * Read a GarlicMessage, decrypt it, and return the resulting CloveSet - * + * Read a GarlicMessage, decrypt it, and return the resulting CloveSet. + * Thread-safe, does not contain any state. + * Public as it's now in the RouterContext. */ -class GarlicMessageParser { +public class GarlicMessageParser { private final Log _log; - private final RouterContext _context; + private final I2PAppContext _context; /** * Huge limit just to reduce chance of trouble. Typ. usage is 3. @@ -34,15 +35,18 @@ class GarlicMessageParser { */ private static final int MAX_CLOVES = 32; - public GarlicMessageParser(RouterContext context) { + public GarlicMessageParser(I2PAppContext context) { _context = context; _log = _context.logManager().getLog(GarlicMessageParser.class); } - /** @param skm use tags from this session key manager */ + /** + * @param skm use tags from this session key manager + * @return null on error + */ public CloveSet getGarlicCloves(GarlicMessage message, PrivateKey encryptionKey, SessionKeyManager skm) { byte encData[] = message.getData(); - byte decrData[] = null; + byte decrData[]; try { if (_log.shouldLog(Log.DEBUG)) _log.debug("Decrypting with private key " + encryptionKey); @@ -50,6 +54,7 @@ class GarlicMessageParser { } catch (DataFormatException dfe) { if (_log.shouldLog(Log.WARN)) _log.warn("Error decrypting", dfe); + return null; } if (decrData == null) { // This is the usual error path and it's logged at WARN level in GarlicMessageReceiver diff --git a/router/java/src/net/i2p/router/message/GarlicMessageReceiver.java b/router/java/src/net/i2p/router/message/GarlicMessageReceiver.java index de573717d..c040a9e41 100644 --- a/router/java/src/net/i2p/router/message/GarlicMessageReceiver.java +++ b/router/java/src/net/i2p/router/message/GarlicMessageReceiver.java @@ -30,7 +30,6 @@ public class GarlicMessageReceiver { private final Log _log; private final CloveReceiver _receiver; private final Hash _clientDestination; - private final GarlicMessageParser _parser; public interface CloveReceiver { public void handleClove(DeliveryInstructions instructions, I2NPMessage data); @@ -50,15 +49,14 @@ public class GarlicMessageReceiver { _context = context; _log = context.logManager().getLog(GarlicMessageReceiver.class); _clientDestination = clientDestination; - _parser = new GarlicMessageParser(context); _receiver = receiver; //_log.error("New GMR dest = " + clientDestination); // all createRateStat in OCMOSJ.init() } public void receive(GarlicMessage message) { - PrivateKey decryptionKey = null; - SessionKeyManager skm = null; + PrivateKey decryptionKey; + SessionKeyManager skm; if (_clientDestination != null) { LeaseSetKeys keys = _context.keyManager().getKeys(_clientDestination); skm = _context.clientManager().getClientSessionKeyManager(_clientDestination); @@ -74,7 +72,7 @@ public class GarlicMessageReceiver { skm = _context.sessionKeyManager(); } - CloveSet set = _parser.getGarlicCloves(message, decryptionKey, skm); + CloveSet set = _context.garlicMessageParser().getGarlicCloves(message, decryptionKey, skm); if (set != null) { for (int i = 0; i < set.getCloveCount(); i++) { GarlicClove clove = set.getClove(i); @@ -109,7 +107,8 @@ public class GarlicMessageReceiver { private boolean isValid(GarlicClove clove) { String invalidReason = _context.messageValidator().validateMessage(clove.getCloveId(), clove.getExpiration().getTime()); - if (invalidReason != null) { + boolean rv = invalidReason == null; + if (!rv) { String howLongAgo = DataHelper.formatDuration(_context.clock().now()-clove.getExpiration().getTime()); if (_log.shouldLog(Log.DEBUG)) _log.debug("Clove is NOT valid: id=" + clove.getCloveId() @@ -121,6 +120,6 @@ public class GarlicMessageReceiver { clove.getData().getClass().getSimpleName(), "Clove is not valid (expiration " + howLongAgo + " ago)"); } - return (invalidReason == null); + return rv; } }