jchen21 commented on a change in pull request #5536:
URL: https://github.com/apache/geode/pull/5536#discussion_r493856692



##########
File path: 
geode-core/src/main/java/org/apache/geode/management/internal/beans/stats/GCStatsMonitor.java
##########
@@ -39,69 +39,71 @@
  * @see org.apache.geode.management.internal.beans.stats.MBeanStatsMonitor
  */
 public class GCStatsMonitor extends MBeanStatsMonitor {
+  // this class uses these volatile variables to make sure reads are reading 
the latest values
+  // it is not using the parent's siteMap which is not volatile to keep the 
stats values.
   private volatile long collections = 0;
   private volatile long collectionTime = 0;
 
-  long getCollections() {
-    return collections;
-  }
-
-  long getCollectionTime() {
-    return collectionTime;
-  }
-
   public GCStatsMonitor(String name) {
     super(name);
   }
 
-  void decreasePrevValues(Map<String, Number> statsMap) {
-    collections -= statsMap.getOrDefault(StatsKey.VM_GC_STATS_COLLECTIONS, 
0).longValue();
-    collectionTime -= 
statsMap.getOrDefault(StatsKey.VM_GC_STATS_COLLECTION_TIME, 0).longValue();
-  }
-
-  void increaseStats(String name, Number value) {
-    if (name.equals(StatsKey.VM_GC_STATS_COLLECTIONS)) {
-      collections += value.longValue();
-      return;
-    }
-
-    if (name.equals(StatsKey.VM_GC_STATS_COLLECTION_TIME)) {
-      collectionTime += value.longValue();
-      return;
+  @Override
+  // this will be called multiple times with different collector's stats
+  public void addStatisticsToMonitor(Statistics stats) {

Review comment:
       The `addStatisticsToMonitor` method ignores the descriptors other than 
`VM_GC_STATS_COLLECTIONS` and `VM_GC_STATS_COLLECTION_TIME`. Can we guarantee 
that there is no other descriptor? 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to