8354929: ZGC: Update collection stats while holding page allocator lock

src/hotspot/share/gc/z/zGeneration.cpp

@@ -279,7 +279,6 @@ void ZGeneration::reset_statistics() {
   _freed = 0;
   _promoted = 0;
   _compacted = 0;
-  _page_allocator->reset_statistics(_id);
 }
 
 size_t ZGeneration::freed() const {
@@ -860,7 +859,7 @@ void ZGenerationYoung::mark_start() {
   _remembered.flip();
 
   // Update statistics
-  stat_heap()->at_mark_start(_page_allocator->stats(this));
+  stat_heap()->at_mark_start(_page_allocator->update_and_stats(this));
 }
 
 void ZGenerationYoung::mark_roots() {
@@ -1209,7 +1208,7 @@ void ZGenerationOld::mark_start() {
   _mark.start();
 
   // Update statistics
-  stat_heap()->at_mark_start(_page_allocator->stats(this));
+  stat_heap()->at_mark_start(_page_allocator->update_and_stats(this));
 
   // Note that we start a marking cycle.
   // Unlike other GCs, the color switch implicitly changes the nmethods

src/hotspot/share/gc/z/zPageAllocator.cpp

@@ -1373,9 +1373,25 @@ size_t ZPageAllocator::unused() const {
   return unused > 0 ? (size_t)unused : 0;
 }
 
-ZPageAllocatorStats ZPageAllocator::stats(ZGeneration* generation) const {
-  ZLocker<ZLock> locker(&_lock);
+void ZPageAllocator::update_collection_stats(ZGenerationId id) {
+  assert(SafepointSynchronize::is_at_safepoint(), "Should be at safepoint");
 
+#ifdef ASSERT
+  size_t total_used = 0;
+
+  ZPartitionIterator iter(&_partitions);
+  for (ZPartition* partition; iter.next(&partition);) {
+    total_used += partition->_used;
+  }
+
+  assert(total_used == _used, "Must be consistent %zu == %zu", total_used, _used);
+#endif
+
+  _collection_stats[(int)id]._used_high = _used;
+  _collection_stats[(int)id]._used_low = _used;
+}
+
+ZPageAllocatorStats ZPageAllocator::stats_inner(ZGeneration* generation) const {
   return ZPageAllocatorStats(_min_capacity,
                              _max_capacity,
                              soft_max_capacity(),
@@ -1390,29 +1406,16 @@ ZPageAllocatorStats ZPageAllocator::stats(ZGeneration* generation) const {
                              _stalled.size());
 }
 
-void ZPageAllocator::reset_statistics(ZGenerationId id) {
-  assert(SafepointSynchronize::is_at_safepoint(), "Should be at safepoint");
-#ifdef ASSERT
-  {
-    // We may free without safepoint synchronization, take the lock to get
-    // consistent values.
-    ZLocker<ZLock> locker(&_lock);
-    size_t total_used = 0;
-
-    ZPartitionIterator iter(&_partitions);
-    for (ZPartition* partition; iter.next(&partition);) {
-      total_used += partition->_used;
-    }
-
-    assert(total_used == _used, "Must be consistent at safepoint %zu == %zu", total_used, _used);
-  }
-#endif
+ZPageAllocatorStats ZPageAllocator::stats(ZGeneration* generation) const {
+  ZLocker<ZLock> locker(&_lock);
+  return stats_inner(generation);
+}
 
-  // Read once, we may have concurrent writers.
-  const size_t used = Atomic::load(&_used);
+ZPageAllocatorStats ZPageAllocator::update_and_stats(ZGeneration* generation) {
+  ZLocker<ZLock> locker(&_lock);
 
-  _collection_stats[(int)id]._used_high = used;
-  _collection_stats[(int)id]._used_low = used;
+  update_collection_stats(generation->id());
+  return stats_inner(generation);
 }
 
 void ZPageAllocator::increase_used_generation(ZGenerationId id, size_t size) {

src/hotspot/share/gc/z/zPageAllocator.hpp

@@ -235,6 +235,9 @@ class ZPageAllocator {
   void notify_out_of_memory();
   void restart_gc() const;
 
+  void update_collection_stats(ZGenerationId id);
+  ZPageAllocatorStats stats_inner(ZGeneration* generation) const;
+
   void print_on_inner(outputStream* st) const;
 
 public:
@@ -262,8 +265,7 @@ class ZPageAllocator {
   void promote_used(const ZPage* from, const ZPage* to);
 
   ZPageAllocatorStats stats(ZGeneration* generation) const;
-
-  void reset_statistics(ZGenerationId id);
+  ZPageAllocatorStats update_and_stats(ZGeneration* generation);
 
   ZPage* alloc_page(ZPageType type, size_t size, ZAllocationFlags flags, ZPageAge age);
   void safe_destroy_page(ZPage* page);