[GEODE-6890] sentMessagesMaxTime stat calculation needs improvement

• Type: Improvement
• Status: Closed
• Priority: Major
• Resolution: Fixed
• Fix Version/s: 1.10.0
• Component/s: statistics
• Assignee: Murtuza Boxwala
• Reporter: Darrel Schneider
• Created: 18/Jun/19 21:35
• Updated: 26/Sep/19 18:03
• Resolved: 21/Jun/19 16:40

Description

The sentMessagesMaxTime stat is calculated by reading the old value. This read is an expensive synchronized operation.

The following is a prototype fix that uses a local atomic:

diff --git a/geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionStats.java b/geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionStats.java
index d591e35570..0b5017b857 100644
--- a/geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionStats.java
+++ b/geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionStats.java
@@ -1035,6 +1035,8 @@ public class DistributionStats implements DMStats {
     return this.stats.getLong(sentMessagesTimeId);
   }
 
+  private final AtomicLong sentMessagesMaxTime = new AtomicLong();
+
   /**
    * Increments the total number of nanoseconds spend sending messages.
    * <p>
@@ -1045,8 +1047,17 @@ public class DistributionStats implements DMStats {
     if (enableClockStats) {
       this.stats.incLong(sentMessagesTimeId, nanos);
       long millis = nanos / 1000000;
-      if (getSentMessagesMaxTime() < millis) {
-        this.stats.setLong(sentMessagesMaxTimeId, millis);
+      boolean done = false;
+      while (!done) {
+        long currentSentMessagesMaxTime = sentMessagesMaxTime.get();
+        if (millis > currentSentMessagesMaxTime) {
+          done = sentMessagesMaxTime.compareAndSet(currentSentMessagesMaxTime, millis);
+          if (done) {
+            stats.setLong(sentMessagesMaxTimeId, millis);
+          }
+        } else {
+          done = true;
+        }
       }
     }
   }