dschneider-pivotal commented on code in PR #7715:
URL: https://github.com/apache/geode/pull/7715#discussion_r904053762
##########
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 seems like the update frequency "int" should be part of the
NonRealTimeStatsUpdater and then you wouldn't need two suppliers.
##########
geode-core/src/main/java/org/apache/geode/internal/offheap/MemoryAllocatorImpl.java:
##########
@@ -239,11 +230,19 @@ private MemoryAllocatorImpl(final
OutOfOffHeapMemoryListener oooml,
this.stats.incMaxMemory(freeList.getTotalMemory());
this.stats.incFreeMemory(freeList.getTotalMemory());
- updateNonRealTimeStatsExecutor =
- LoggingExecutors.newSingleThreadScheduledExecutor("Update Freelist
Stats thread");
- updateNonRealTimeStatsFuture =
-
updateNonRealTimeStatsExecutor.scheduleAtFixedRate(freeList::updateNonRealTimeStats,
0,
- updateOffHeapStatsFrequencyMs, TimeUnit.MILLISECONDS);
+ this.updateOffHeapStatsFrequencyMs = updateOffHeapStatsFrequencyMs;
+
+ if (nonRealTimeStatsUpdaterSupplier == null) {
+ nonRealTimeStatsUpdater = new
NonRealTimeStatsUpdater(freeList::updateNonRealTimeStats);
+ } else {
+ nonRealTimeStatsUpdater = nonRealTimeStatsUpdaterSupplier.get();
+ }
+ }
+
+ void start() {
+ if (nonRealTimeStatsUpdater != null) {
Review Comment:
In the constructor above it looks like you always initialize
nonRealTimeStatsUpdaterSupplier to non-null. So what good are these null
checks? Oh I see. Your supplier returns null. That works but I think it would
be better to have the supplier returns a dummy NonRealTimeStatsUpdater that
does nothing.
--
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
