RFR: 8348349: Refactor CDSConfig::is_dumping_heap() [v2]
Ioi Lam
iklam at openjdk.org
Mon Jan 27 06:02:54 UTC 2025
On Thu, 23 Jan 2025 22:14:06 GMT, Matias Saavedra Silva <matsaave at openjdk.org> wrote:
>> Ioi Lam has updated the pull request incrementally with one additional commit since the last revision:
>>
>> @matias9927 comment
>
> 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
Fixed.
> 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.
`_disable_heap_dumping` is for unconditionally disabling heap dumping. The opposite does not mean we can dump the heap -- heap dumping can also be disabled if ZGC is used. So `can_dump_heap` will be misleading.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/23249#discussion_r1930017837
PR Review Comment: https://git.openjdk.org/jdk/pull/23249#discussion_r1930017826
More information about the hotspot-dev
mailing list