From d601eb4da60e03ba0b7531dd43c0f4af903486ab Mon Sep 17 00:00:00 2001 From: Thomas Schatzl Date: Mon, 7 Jul 2025 10:06:00 +0200 Subject: [PATCH 1/3] 8361349 Hi all, please review this refactoring to improve the visibility of `CollectedHeap::stop()` and `CH::print_tracing_info()`. The change moves these methods to `protected` visibility. Testing: gha Thanks, Thomas --- src/hotspot/share/gc/epsilon/epsilonHeap.hpp | 7 ++++--- src/hotspot/share/gc/g1/g1CollectedHeap.hpp | 9 +++++---- src/hotspot/share/gc/parallel/parallelScavengeHeap.hpp | 7 ++++--- src/hotspot/share/gc/serial/serialHeap.hpp | 7 ++++--- src/hotspot/share/gc/shared/collectedHeap.hpp | 8 ++++---- .../share/gc/shenandoah/shenandoahGenerationalHeap.hpp | 7 ++++--- src/hotspot/share/gc/shenandoah/shenandoahHeap.hpp | 7 ++++--- src/hotspot/share/gc/z/zCollectedHeap.hpp | 6 ++++-- 8 files changed, 33 insertions(+), 25 deletions(-) diff --git a/src/hotspot/share/gc/epsilon/epsilonHeap.hpp b/src/hotspot/share/gc/epsilon/epsilonHeap.hpp index 03f9b6cd19786..e7b1fefe5b9f5 100644 --- a/src/hotspot/share/gc/epsilon/epsilonHeap.hpp +++ b/src/hotspot/share/gc/epsilon/epsilonHeap.hpp @@ -49,6 +49,10 @@ class EpsilonHeap : public CollectedHeap { volatile size_t _last_counter_update; volatile size_t _last_heap_print; +protected: + void print_tracing_info() const override; + void stop() override {}; + public: static EpsilonHeap* heap(); @@ -67,8 +71,6 @@ class EpsilonHeap : public CollectedHeap { jint initialize() override; void initialize_serviceability() override; - void stop() override {}; - GrowableArray memory_managers() override; GrowableArray memory_pools() override; @@ -130,7 +132,6 @@ class EpsilonHeap : public CollectedHeap { void print_heap_on(outputStream* st) const override; void print_gc_on(outputStream* st) const override {} - void print_tracing_info() const override; bool print_location(outputStream* st, void* addr) const override; private: diff --git a/src/hotspot/share/gc/g1/g1CollectedHeap.hpp b/src/hotspot/share/gc/g1/g1CollectedHeap.hpp index 5305ef475b6e5..64fa0c086dbba 100644 --- a/src/hotspot/share/gc/g1/g1CollectedHeap.hpp +++ b/src/hotspot/share/gc/g1/g1CollectedHeap.hpp @@ -882,6 +882,11 @@ class G1CollectedHeap : public CollectedHeap { private: jint initialize_concurrent_refinement(); jint initialize_service_thread(); + +protected: + void print_tracing_info() const override; + void stop() override; + public: // Initialize the G1CollectedHeap to have the initial and // maximum sizes and remembered and barrier sets @@ -891,7 +896,6 @@ class G1CollectedHeap : public CollectedHeap { // Returns whether concurrent mark threads (and the VM) are about to terminate. bool concurrent_mark_is_terminating() const; - void stop() override; void safepoint_synchronize_begin() override; void safepoint_synchronize_end() override; @@ -1309,9 +1313,6 @@ class G1CollectedHeap : public CollectedHeap { void gc_threads_do(ThreadClosure* tc) const override; - // Override - void print_tracing_info() const override; - // Used to print information about locations in the hs_err file. bool print_location(outputStream* st, void* addr) const override; }; diff --git a/src/hotspot/share/gc/parallel/parallelScavengeHeap.hpp b/src/hotspot/share/gc/parallel/parallelScavengeHeap.hpp index daa72ffc2ea3d..a5eb008875697 100644 --- a/src/hotspot/share/gc/parallel/parallelScavengeHeap.hpp +++ b/src/hotspot/share/gc/parallel/parallelScavengeHeap.hpp @@ -111,6 +111,10 @@ class ParallelScavengeHeap : public CollectedHeap { void do_full_collection(bool clear_all_soft_refs) override; +protected: + void print_tracing_info() const override; + void stop() override {}; + public: ParallelScavengeHeap() : CollectedHeap(), @@ -155,8 +159,6 @@ class ParallelScavengeHeap : public CollectedHeap { void post_initialize() override; void update_counters(); - void stop() override {}; - size_t capacity() const override; size_t used() const override; @@ -214,7 +216,6 @@ class ParallelScavengeHeap : public CollectedHeap { void print_heap_on(outputStream* st) const override; void print_gc_on(outputStream* st) const override; void gc_threads_do(ThreadClosure* tc) const override; - void print_tracing_info() const override; WorkerThreads* safepoint_workers() override { return &_workers; } diff --git a/src/hotspot/share/gc/serial/serialHeap.hpp b/src/hotspot/share/gc/serial/serialHeap.hpp index 3cc4bb7abf559..c0f3206618297 100644 --- a/src/hotspot/share/gc/serial/serialHeap.hpp +++ b/src/hotspot/share/gc/serial/serialHeap.hpp @@ -118,6 +118,10 @@ class SerialHeap : public CollectedHeap { void gc_prologue(); void gc_epilogue(bool full); +protected: + void print_tracing_info() const override; + void stop() override {}; + public: // Returns JNI_OK on success jint initialize() override; @@ -125,8 +129,6 @@ class SerialHeap : public CollectedHeap { // Does operations required after initialization has been done. void post_initialize() override; - void stop() override {}; - bool is_in_reserved(const void* addr) const { return _reserved.contains(addr); } // Performance Counter support @@ -211,7 +213,6 @@ class SerialHeap : public CollectedHeap { void print_heap_on(outputStream* st) const override; void print_gc_on(outputStream* st) const override; void gc_threads_do(ThreadClosure* tc) const override; - void print_tracing_info() const override; // Used to print information about locations in the hs_err file. bool print_location(outputStream* st, void* addr) const override; diff --git a/src/hotspot/share/gc/shared/collectedHeap.hpp b/src/hotspot/share/gc/shared/collectedHeap.hpp index 3e0431bf92561..621f3a0bbc728 100644 --- a/src/hotspot/share/gc/shared/collectedHeap.hpp +++ b/src/hotspot/share/gc/shared/collectedHeap.hpp @@ -208,6 +208,10 @@ class CollectedHeap : public CHeapObj { return static_cast(heap); } + // Print any relevant tracing info that flags imply. + // Default implementation does nothing. + virtual void print_tracing_info() const = 0; + // Stop any onging concurrent work and prepare for exit. virtual void stop() = 0; @@ -459,10 +463,6 @@ class CollectedHeap : public CHeapObj { // Iterator for all GC threads (other than VM thread) virtual void gc_threads_do(ThreadClosure* tc) const = 0; - // Print any relevant tracing info that flags imply. - // Default implementation does nothing. - virtual void print_tracing_info() const = 0; - double elapsed_gc_cpu_time() const; void print_before_gc() const; diff --git a/src/hotspot/share/gc/shenandoah/shenandoahGenerationalHeap.hpp b/src/hotspot/share/gc/shenandoah/shenandoahGenerationalHeap.hpp index ed5a6f3d9a550..4ce6933eec6c9 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahGenerationalHeap.hpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahGenerationalHeap.hpp @@ -36,6 +36,10 @@ class ShenandoahGenerationalControlThread; class ShenandoahAgeCensus; class ShenandoahGenerationalHeap : public ShenandoahHeap { +protected: + void print_tracing_info() const override; + void stop() override; + public: explicit ShenandoahGenerationalHeap(ShenandoahCollectorPolicy* policy); void post_initialize() override; @@ -53,7 +57,6 @@ class ShenandoahGenerationalHeap : public ShenandoahHeap { } void print_init_logger() const override; - void print_tracing_info() const override; size_t unsafe_max_tlab_alloc(Thread *thread) const override; @@ -126,8 +129,6 @@ class ShenandoahGenerationalHeap : public ShenandoahHeap { void gc_threads_do(ThreadClosure* tcl) const override; - void stop() override; - bool requires_barriers(stackChunkOop obj) const override; // Used for logging the result of a region transfer outside the heap lock diff --git a/src/hotspot/share/gc/shenandoah/shenandoahHeap.hpp b/src/hotspot/share/gc/shenandoah/shenandoahHeap.hpp index 4124bf8be7f5a..31046dc01cfb7 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahHeap.hpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahHeap.hpp @@ -159,6 +159,10 @@ class ShenandoahHeap : public CollectedHeap { // updated at each STW pause associated with a ShenandoahVMOp. ShenandoahGeneration* _active_generation; +protected: + void print_tracing_info() const override; + void stop() override; + public: ShenandoahHeapLock* lock() { return &_lock; @@ -204,11 +208,8 @@ class ShenandoahHeap : public CollectedHeap { void print_heap_on(outputStream* st) const override; void print_gc_on(outputStream *st) const override; - void print_tracing_info() const override; void print_heap_regions_on(outputStream* st) const; - void stop() override; - void prepare_for_verify() override; void verify(VerifyOption vo) override; diff --git a/src/hotspot/share/gc/z/zCollectedHeap.hpp b/src/hotspot/share/gc/z/zCollectedHeap.hpp index 871ff81bfdf5d..dffd081bc71b0 100644 --- a/src/hotspot/share/gc/z/zCollectedHeap.hpp +++ b/src/hotspot/share/gc/z/zCollectedHeap.hpp @@ -55,6 +55,10 @@ class ZCollectedHeap : public CollectedHeap { size_t requested_size, size_t* actual_size) override; +protected: + void print_tracing_info() const override; + void stop() override; + public: static ZCollectedHeap* heap(); @@ -63,7 +67,6 @@ class ZCollectedHeap : public CollectedHeap { const char* name() const override; jint initialize() override; void initialize_serviceability() override; - void stop() override; size_t max_capacity() const override; size_t capacity() const override; @@ -116,7 +119,6 @@ class ZCollectedHeap : public CollectedHeap { void print_heap_on(outputStream* st) const override; void print_gc_on(outputStream* st) const override; - void print_tracing_info() const override; bool print_location(outputStream* st, void* addr) const override; void prepare_for_verify() override; From 5fd5d6f7e9959a9ed811eb5bc094f42e22c0f874 Mon Sep 17 00:00:00 2001 From: Thomas Schatzl Date: Mon, 7 Jul 2025 18:16:03 +0200 Subject: [PATCH 2/3] * iwalulya suggested to make leaf classes use private; although probably one wants to use final, but Hotspot does not seem to use that --- src/hotspot/share/gc/epsilon/epsilonHeap.hpp | 2 +- src/hotspot/share/gc/g1/g1CollectedHeap.hpp | 1 - src/hotspot/share/gc/parallel/parallelScavengeHeap.hpp | 1 - src/hotspot/share/gc/serial/serialHeap.hpp | 1 - src/hotspot/share/gc/shenandoah/shenandoahGenerationalHeap.hpp | 1 - src/hotspot/share/gc/z/zCollectedHeap.hpp | 1 - 6 files changed, 1 insertion(+), 6 deletions(-) diff --git a/src/hotspot/share/gc/epsilon/epsilonHeap.hpp b/src/hotspot/share/gc/epsilon/epsilonHeap.hpp index e7b1fefe5b9f5..2bfffad8a5892 100644 --- a/src/hotspot/share/gc/epsilon/epsilonHeap.hpp +++ b/src/hotspot/share/gc/epsilon/epsilonHeap.hpp @@ -49,7 +49,7 @@ class EpsilonHeap : public CollectedHeap { volatile size_t _last_counter_update; volatile size_t _last_heap_print; -protected: +private: void print_tracing_info() const override; void stop() override {}; diff --git a/src/hotspot/share/gc/g1/g1CollectedHeap.hpp b/src/hotspot/share/gc/g1/g1CollectedHeap.hpp index 64fa0c086dbba..050f7b6e5673b 100644 --- a/src/hotspot/share/gc/g1/g1CollectedHeap.hpp +++ b/src/hotspot/share/gc/g1/g1CollectedHeap.hpp @@ -883,7 +883,6 @@ class G1CollectedHeap : public CollectedHeap { jint initialize_concurrent_refinement(); jint initialize_service_thread(); -protected: void print_tracing_info() const override; void stop() override; diff --git a/src/hotspot/share/gc/parallel/parallelScavengeHeap.hpp b/src/hotspot/share/gc/parallel/parallelScavengeHeap.hpp index a5eb008875697..68900c8dbf2dc 100644 --- a/src/hotspot/share/gc/parallel/parallelScavengeHeap.hpp +++ b/src/hotspot/share/gc/parallel/parallelScavengeHeap.hpp @@ -111,7 +111,6 @@ class ParallelScavengeHeap : public CollectedHeap { void do_full_collection(bool clear_all_soft_refs) override; -protected: void print_tracing_info() const override; void stop() override {}; diff --git a/src/hotspot/share/gc/serial/serialHeap.hpp b/src/hotspot/share/gc/serial/serialHeap.hpp index c0f3206618297..d93e895d20983 100644 --- a/src/hotspot/share/gc/serial/serialHeap.hpp +++ b/src/hotspot/share/gc/serial/serialHeap.hpp @@ -118,7 +118,6 @@ class SerialHeap : public CollectedHeap { void gc_prologue(); void gc_epilogue(bool full); -protected: void print_tracing_info() const override; void stop() override {}; diff --git a/src/hotspot/share/gc/shenandoah/shenandoahGenerationalHeap.hpp b/src/hotspot/share/gc/shenandoah/shenandoahGenerationalHeap.hpp index 4ce6933eec6c9..930c8ef7105c7 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahGenerationalHeap.hpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahGenerationalHeap.hpp @@ -36,7 +36,6 @@ class ShenandoahGenerationalControlThread; class ShenandoahAgeCensus; class ShenandoahGenerationalHeap : public ShenandoahHeap { -protected: void print_tracing_info() const override; void stop() override; diff --git a/src/hotspot/share/gc/z/zCollectedHeap.hpp b/src/hotspot/share/gc/z/zCollectedHeap.hpp index dffd081bc71b0..3d466564b5436 100644 --- a/src/hotspot/share/gc/z/zCollectedHeap.hpp +++ b/src/hotspot/share/gc/z/zCollectedHeap.hpp @@ -55,7 +55,6 @@ class ZCollectedHeap : public CollectedHeap { size_t requested_size, size_t* actual_size) override; -protected: void print_tracing_info() const override; void stop() override; From 087d72b771c48aaf24ce8f803e243879e6751d56 Mon Sep 17 00:00:00 2001 From: Thomas Schatzl Date: Tue, 8 Jul 2025 10:22:54 +0200 Subject: [PATCH 3/3] * remove one more unnecessary `private` --- src/hotspot/share/gc/epsilon/epsilonHeap.hpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/hotspot/share/gc/epsilon/epsilonHeap.hpp b/src/hotspot/share/gc/epsilon/epsilonHeap.hpp index 2bfffad8a5892..24b43fe354140 100644 --- a/src/hotspot/share/gc/epsilon/epsilonHeap.hpp +++ b/src/hotspot/share/gc/epsilon/epsilonHeap.hpp @@ -49,7 +49,6 @@ class EpsilonHeap : public CollectedHeap { volatile size_t _last_counter_update; volatile size_t _last_heap_print; -private: void print_tracing_info() const override; void stop() override {};