remove NTCP bottlenecks (some)

[jogger]

While profiling NTCP I found some areas eating lots of CPU. Following are easy to fix:

• Syncing on a method that retrieves just one field does nothing. Many occurrences in the transport, NTCPConnection.isEstablished() eats most.
• _conlock also blocks a lot. Suggest using the double check idiom in NTCPTransport.outboundMessageReady()

            con = _conByIdent.get(ih);
            if (con == null) {
                synchronized (_conLock) {
                    con = _conByIdent.get(ih);
                    if (con == null) {

[zzz]

I'll look at 2nd bullet in OP. CC'ing zab for advice in first bullet

[Zlatin Balevsky]

NTCPConnection.isEstablished() eats most.

as far as I can see that delegates to either OutboundNTCP2State.isComplete or EstablishBase.isComplete()

The one for outbound does not have any logic that requires a full monitor, so it can be replaced with volatile easily.

The one in EstablishBase does have some logic which is easier to do with a monitor, but with a little creativity could be replaced with an AtomicReference which would be much cheaper to read.

[Zlatin Balevsky]

Replace _stateLock with an AtomicReference?

[zzz]

followup and an alternative proposal at ​http://zzz.i2p/topics/2860 , posted there during an extended downtime for trac

[zzz]

re: 2nd bullet, I've been testing it for a month, no issues. As recommended by OP, it double-checks that the con is NOT in the map, so there won't be dups. There's no lock that the con IS in the map, so in rare cases we could end up transmitting on a con that is closed and about to be removed. But that can happen anyway.. see locking in removeCon().

So here's the diff I've been testing for a month. Looking for comments on it, and also on the diff linked in the above comment for the 1st bullet from OP.

#
# old_revision [39905af4247c2d16678a63ca36025c13a1cd3237]
#
# patch "router/java/src/net/i2p/router/transport/ntcp/NTCPTransport.java"
#  from [a6961bb8f3db3fac9a3fb38a627ab039d494fc8c]
#    to [ce5e8f9b2ade2214aa9d069f5eab39a1b7e922fb]
#
============================================================
--- router/java/src/net/i2p/router/transport/ntcp/NTCPTransport.java	a6961bb8f3db3fac9a3fb38a627ab039d494fc8c
+++ router/java/src/net/i2p/router/transport/ntcp/NTCPTransport.java	ce5e8f9b2ade2214aa9d069f5eab39a1b7e922fb
@@ -359,35 +359,38 @@ public class NTCPTransport extends Trans
             RouterInfo target = msg.getTarget();
             RouterIdentity ident = target.getIdentity();
             Hash ih = ident.calculateHash();
-            NTCPConnection con = null;
             int newVersion = 0;
             boolean fail = false;
-            synchronized (_conLock) {
-                con = _conByIdent.get(ih);
-                if (con == null) {
-                    RouterAddress addr = getTargetAddress(target);
-                    if (addr != null) {
-                        newVersion = getNTCPVersion(addr);
-                        if (newVersion != 0) {
-                            try {
-                                con = new NTCPConnection(_context, this, ident, addr, newVersion);
-                                establishing(con);
-                                //if (_log.shouldLog(Log.DEBUG))
-                                //    _log.debug("Send on a new con: " + con + " at " + addr + " for " + ih);
-                                // Note that outbound conns go in the map BEFORE establishment
-                                _conByIdent.put(ih, con);
-                            } catch (DataFormatException dfe) {
-                                if (_log.shouldWarn())
-                                    _log.warn("bad address? " + target, dfe);
+            // _conLock contention, check outside lock first
+            NTCPConnection con = _conByIdent.get(ih);
+            if (con == null) {
+                synchronized (_conLock) {
+                    con = _conByIdent.get(ih);
+                    if (con == null) {
+                        RouterAddress addr = getTargetAddress(target);
+                        if (addr != null) {
+                            newVersion = getNTCPVersion(addr);
+                            if (newVersion != 0) {
+                                try {
+                                    con = new NTCPConnection(_context, this, ident, addr, newVersion);
+                                    establishing(con);
+                                    //if (_log.shouldLog(Log.DEBUG))
+                                    //    _log.debug("Send on a new con: " + con + " at " + addr + " for " + ih);
+                                    // Note that outbound conns go in the map BEFORE establishment
+                                    _conByIdent.put(ih, con);
+                                } catch (DataFormatException dfe) {
+                                    if (_log.shouldWarn())
+                                        _log.warn("bad address? " + target, dfe);
+                                    fail = true;
+                                }
+                            } else {
                                 fail = true;
                             }
                         } else {
+                            // race, RI changed out from under us
+                            // call afterSend below outside of conLock
                             fail = true;
                         }
-                    } else {
-                        // race, RI changed out from under us
-                        // call afterSend below outside of conLock
-                        fail = true;
                     }
                 }
             }