kohlmu-pivotal commented on a change in pull request #7456:
URL: https://github.com/apache/geode/pull/7456#discussion_r832774433



##########
File path: 
geode-core/src/main/java/org/apache/geode/internal/cache/control/HeapMemoryMonitor.java
##########
@@ -144,82 +84,34 @@
   private final ResourceAdvisor resourceAdvisor;
   private final InternalCache cache;
   private final ResourceManagerStats stats;
-  private final TenuredHeapConsumptionMonitor tenuredHeapConsumptionMonitor;
 
-  @MutableForTesting
-  private static boolean testDisableMemoryUpdates = false;
   @MutableForTesting
   private static long testBytesUsedForThresholdSet = -1;
 
-  /**
-   * Determines if the name of the memory pool MXBean provided matches a list 
of known tenured pool
-   * names.
-   *
-   * Package private for testing.
-   * checkTenuredHeapConsumption
-   *
-   * @param memoryPoolMXBean The memory pool MXBean to check.
-   * @return True if the pool name matches a known tenured pool name, false 
otherwise.
-   */
-  static boolean isTenured(MemoryPoolMXBean memoryPoolMXBean) {
-    if (memoryPoolMXBean.getType() != MemoryType.HEAP) {
-      return false;
-    }
-
-    String name = memoryPoolMXBean.getName();
-
-    return name.equals("CMS Old Gen") // Sun Concurrent Mark Sweep GC
-        || name.equals("PS Old Gen") // Sun Parallel GC
-        || name.equals("G1 Old Gen") // Sun G1 GC
-        || name.equals("Old Space") // BEA JRockit 1.5, 1.6 GC
-        || name.equals("Tenured Gen") // Hitachi 1.5 GC
-        || name.equals("Java heap") // IBM 1.5, 1.6 GC
-        || name.equals("GenPauseless Old Gen") // azul C4/GPGC collector
-
-        // Allow an unknown pool name to monitor
-        || (HEAP_POOL != null && name.equals(HEAP_POOL));
-  }
+  @Immutable
+  private static final ServiceLoader<HeapUsageProvider> heapUsageMonitorLoader 
=
+      ServiceLoader.load(HeapUsageProvider.class);

Review comment:
       This is a novel approach to service loading. Why would we have the 
ServiceLoading on static initialization of the class but the looking up of the 
implementation only at construction time of the HeapMemoryMonitor? Given that 
you limit it to the first implementation found AND on construction time of the 
HeapMemoryMonitor, I believe all this code could be done in the same location. 
i.e in the findHeapUsageMonitor method 




-- 
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.

To unsubscribe, e-mail: notifications-unsubscr...@geode.apache.org

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


Reply via email to