[ https://issues.apache.org/jira/browse/GEODE-6889?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Darrel Schneider resolved GEODE-6889. ------------------------------------- Resolution: Fixed Fix Version/s: 1.10.0 > replyWaitMaxTime stat calculation code needs improvement > -------------------------------------------------------- > > Key: GEODE-6889 > URL: https://issues.apache.org/jira/browse/GEODE-6889 > Project: Geode > Issue Type: Improvement > Components: core > Reporter: Darrel Schneider > Assignee: Murtuza Boxwala > Priority: Major > Labels: performance > Fix For: 1.10.0 > > Time Spent: 4h 40m > Remaining Estimate: 0h > > The way the replyWaitMaxTime is calculated has some problems: > 1. the elapsed time is calculated using System.currentTimeMillis but should > instead use getStatTime (it actually uses getStatTime for the begin timestamp > but not for the end timestamp). > 2. a local atomic should be used to calculate it instead of reading the value > from the Statistics instance. Reading is expensive since it uses > synchronization. > The following is a prototype fix for #2 I used when performance testing: > {noformat} > 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 91c47e2495..d591e35570 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 > @@ -14,6 +14,8 @@ > */ > package org.apache.geode.distributed.internal; > > +import java.util.concurrent.atomic.AtomicLong; > + > import org.apache.logging.log4j.Logger; > > import org.apache.geode.StatisticDescriptor; > @@ -1602,6 +1604,8 @@ public class DistributionStats implements DMStats { > return getStatTime(); > } > > + private final AtomicLong replyWaitMaxTime = new AtomicLong(); > + > @Override > public void endReplyWait(long startNanos, long initTime) { > if (enableClockStats) { > @@ -1610,8 +1614,17 @@ public class DistributionStats implements DMStats { > } > if (initTime != 0) { > long mswait = System.currentTimeMillis() - initTime; > - if (mswait > getReplyWaitMaxTime()) { > - stats.setLong(replyWaitMaxTimeId, mswait); > + boolean done = false; > + while (!done) { > + long currentReplyWaitMaxTime = replyWaitMaxTime.get(); > + if (mswait > currentReplyWaitMaxTime) { > + done = replyWaitMaxTime.compareAndSet(currentReplyWaitMaxTime, > mswait); > + if (done) { > + stats.setLong(replyWaitMaxTimeId, mswait); > + } > + } else { > + done = true; > + } > } > } > stats.incInt(replyWaitsInProgressId, -1);{noformat} -- This message was sent by Atlassian JIRA (v7.6.3#76005)