Improve time tracking overheads a bit

We are performing unneeded computation when we are rescheduling
the keepalive timer:
- we always convert the keepAlive timer from seconds to nanos
  when checking expiry
- we are forcing netty to translate seconds to nanos again when
  we are re-scheduling the timer
- we end up doing a superfluous addition, which then undone when
  we are computing the next duration

This normalizes all the timer delays to nanoseconds, so we
eliminate these inefficiencies.

JIRA: BGPCEP-872
Change-Id: Ic5912020a1cfa0df4fc6d4cf401d846cd9140ee4
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
(cherry picked from commit 7fbf2d3b2ef7b03fab9208e03a27e053acd415e2)

diff --git a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/BGPSessionImpl.java b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/BGPSessionImpl.java
index d44eafdedb04bd6ef52256684ab383685b13166c..a2f11fbb3ebff5af5b92e45ca54ca2bba31eab7a 100644 (file)
--- a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/BGPSessionImpl.java
+++ b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/BGPSessionImpl.java
@@ -117,8 +117,8 @@ public class BGPSessionImpl extends SimpleChannelInboundHandler<Notification> im
 
     private final Set<BgpTableType> tableTypes;
     private final List<AddressFamilies> addPathTypes;
-    private final int holdTimerValue;
-    private final int keepAlive;
+    private final long holdTimerNanos;
+    private final long keepAliveNanos;
     private final AsNumber asNumber;
     private final Ipv4Address bgpId;
     private final BGPPeerRegistry peerRegistry;
@@ -139,9 +139,13 @@ public class BGPSessionImpl extends SimpleChannelInboundHandler<Notification> im
         this.channel = requireNonNull(channel);
         this.limiter = new ChannelOutputLimiter(this);
         this.channel.pipeline().addLast(this.limiter);
-        this.holdTimerValue = remoteOpen.getHoldTimer() < localHoldTimer ? remoteOpen.getHoldTimer() : localHoldTimer;
-        LOG.info("BGP HoldTimer new value: {}", this.holdTimerValue);
-        this.keepAlive = this.holdTimerValue / KA_TO_DEADTIMER_RATIO;
+
+        final int holdTimerValue = remoteOpen.getHoldTimer() < localHoldTimer ? remoteOpen.getHoldTimer()
+                : localHoldTimer;
+        LOG.info("BGP HoldTimer new value: {}", holdTimerValue);
+        this.holdTimerNanos = TimeUnit.SECONDS.toNanos(holdTimerValue);
+        this.keepAliveNanos = TimeUnit.SECONDS.toNanos(holdTimerValue / KA_TO_DEADTIMER_RATIO);
+
         this.asNumber = AsNumberUtil.advertizedAsNumber(remoteOpen);
         this.peerRegistry = peerRegistry;
         this.sessionState = new BGPSessionStateImpl();
@@ -189,12 +193,12 @@ public class BGPSessionImpl extends SimpleChannelInboundHandler<Notification> im
                 MultiPathSupportImpl.createParserMultiPathSupport(this.addPathTypes));
         }
 
-        if (this.holdTimerValue != 0) {
-            channel.eventLoop().schedule(this::handleHoldTimer, this.holdTimerValue, TimeUnit.SECONDS);
-            channel.eventLoop().schedule(this::handleKeepaliveTimer, this.keepAlive, TimeUnit.SECONDS);
+        if (holdTimerValue != 0) {
+            channel.eventLoop().schedule(this::handleHoldTimer, this.holdTimerNanos, TimeUnit.NANOSECONDS);
+            channel.eventLoop().schedule(this::handleKeepaliveTimer, this.keepAliveNanos, TimeUnit.NANOSECONDS);
         }
         this.bgpId = remoteOpen.getBgpIdentifier();
-        this.sessionState.advertizeCapabilities(this.holdTimerValue, channel.remoteAddress(), channel.localAddress(),
+        this.sessionState.advertizeCapabilities(holdTimerValue, channel.remoteAddress(), channel.localAddress(),
                 this.tableTypes, bgpParameters);
     }
 
@@ -402,7 +406,7 @@ public class BGPSessionImpl extends SimpleChannelInboundHandler<Notification> im
         }
 
         final long ct = System.nanoTime();
-        final long nextHold = this.lastMessageReceivedAt + TimeUnit.SECONDS.toNanos(this.holdTimerValue);
+        final long nextHold = this.lastMessageReceivedAt + holdTimerNanos;
 
         if (ct >= nextHold) {
             LOG.debug("HoldTimer expired. {}", new Date());
@@ -420,17 +424,18 @@ public class BGPSessionImpl extends SimpleChannelInboundHandler<Notification> im
      */
     private synchronized void handleKeepaliveTimer() {
         if (this.state == State.IDLE) {
+            LOG.debug("Skipping keepalive on session idle {}", this);
             return;
         }
 
         final long ct = System.nanoTime();
-        final long keepNanos = TimeUnit.SECONDS.toNanos(this.keepAlive);
-        long nextKeepalive = this.lastMessageSentAt + keepNanos;
+        final long nextKeepalive = this.lastMessageSentAt + keepAliveNanos;
+        long nextNanos = nextKeepalive - ct;
 
-        if (ct >= nextKeepalive) {
+        if (nextNanos <= 0) {
             final ChannelFuture future = this.writeAndFlush(KEEP_ALIVE);
             LOG.debug("Enqueued session {} keepalive as {}", this, future);
-            nextKeepalive = ct + keepNanos;
+            nextNanos = keepAliveNanos;
             if (LOG.isDebugEnabled()) {
                 future.addListener(compl -> LOG.debug("Session {} keepalive completed as {}", this, compl));
             }
@@ -438,7 +443,6 @@ public class BGPSessionImpl extends SimpleChannelInboundHandler<Notification> im
             LOG.debug("Skipping keepalive on session {}", this);
         }
 
-        final long nextNanos = nextKeepalive - ct;
         LOG.debug("Scheduling next keepalive on {} in {} nanos", this, nextNanos);
         this.channel.eventLoop().schedule(this::handleKeepaliveTimer, nextNanos, TimeUnit.NANOSECONDS);
     }