RFR: 8348349: Refactor CDSConfig::is_dumping_heap()

[Matias Saavedra Silva]

On Thu, 23 Jan 2025 02:35:06 GMT, Ioi Lam <iklam at openjdk.org> wrote:

> Please review this small clean up:
> 
> `HeapShared::can_write()` and `CDSConfig::is_dumping_heap()` are both for deciding whether CDS should dump heap objects. I removed the former and consolidated all the logic to the latter.
> 
> I also updated the logging message in case heap objects cannot be dumped.
> 
> I also updated VMProps to clarify what `vmCDSCanWriteArchivedJavaHeap()` means.

Change looks good, thanks for the cleanup! I have some small comments that you can address if you think it's valuable:

src/hotspot/share/cds/cdsConfig.cpp line 572:

> 570: bool CDSConfig::is_dumping_heap() {
> 571:   // heap dump is not supported in dynamic dump
> 572:   if (!is_dumping_static_archive()) {

I think you can compress these three cases into one if-statement

src/hotspot/share/cds/cdsConfig.hpp line 49:

> 47: 
> 48:   static bool  _old_cds_flags_used;
> 49:   static bool  _disable_heap_dumping;

Following with the previous implementation, I'd prefer `can_dump_heap` to avoid the double negative.

-------------

Marked as reviewed by matsaave (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/23249#pullrequestreview-2571137386
PR Review Comment: https://git.openjdk.org/jdk/pull/23249#discussion_r1927751925
PR Review Comment: https://git.openjdk.org/jdk/pull/23249#discussion_r1927759369