[GitHub] [geode] albertogpz commented on a diff in pull request #7715: GEODE-10323: Remove schedule threads in MemoryAllocatorImpl constructor

[GitBox <gi...@apache.org>]

albertogpz commented on code in PR #7715:
URL: https://github.com/apache/geode/pull/7715#discussion_r904097361


##########
geode-core/src/main/java/org/apache/geode/internal/offheap/MemoryAllocatorImpl.java:
##########
@@ -98,20 +95,17 @@ public static MemoryAllocatorImpl getAllocator() {
 
   public static MemoryAllocator create(OutOfOffHeapMemoryListener ooohml, OffHeapMemoryStats stats,
       int slabCount, long offHeapMemorySize, long maxSlabSize,
-      int updateOffHeapStatsFrequencyMs) {
+      Supplier<Integer> updateOffHeapStatsFrequencyMsSupplier,
+      Supplier<NonRealTimeStatsUpdater> nonRealTimeStatsUpdaterSupplier) {
     return create(ooohml, stats, slabCount, offHeapMemorySize, maxSlabSize, null,
-        SlabImpl::new, updateOffHeapStatsFrequencyMs);
+        SlabImpl::new, updateOffHeapStatsFrequencyMsSupplier, nonRealTimeStatsUpdaterSupplier);
   }
 
-  public static MemoryAllocator create(OutOfOffHeapMemoryListener ooohml, OffHeapMemoryStats stats,
-      int slabCount, long offHeapMemorySize, long maxSlabSize) {
-    return create(ooohml, stats, slabCount, offHeapMemorySize, maxSlabSize, null,
-        SlabImpl::new, UPDATE_OFF_HEAP_STATS_FREQUENCY_MS);
-  }
-
-  private static MemoryAllocatorImpl create(OutOfOffHeapMemoryListener ooohml,
+  static MemoryAllocatorImpl create(OutOfOffHeapMemoryListener ooohml,
       OffHeapMemoryStats stats, int slabCount, long offHeapMemorySize, long maxSlabSize,
-      Slab[] slabs, SlabFactory slabFactory, int updateOffHeapStatsFrequencyMs) {
+      Slab[] slabs, SlabFactory slabFactory,
+      Supplier<Integer> updateOffHeapStatsFrequencyMsSupplier,

Review Comment:
   It's a good idea but there is one test case in which you don't pass a Supplier so that the default `NonRealTimeStatsUpdater` is constructed but you need to specify a smaller frequency so that the test does not need to wait for the default frequency value:
   See `testUpdateNonRealTimeOffHeapStorageStats()` from class `OffHeapStorageNonRuntimeStatsJUnitTest`.
   For this test case I would need to specify the frequency but not the supplier.



-- 
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-unsubscribe@geode.apache.org

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