On Thu, 17 Aug 2023 15:13:16 GMT, Chris Plummer <cjplum...@openjdk.org> wrote:
>> Current test expects `G1HeapRegionSize` flag to be printed out from jmap.
>> But that one is not printed when Shenandoah is enabled. I opted for the
>> minimally intrusive fix: move the additional Shenandoah printout to the
>> Shenandoah-specific block.
>>
>> Additional testing:
>> - [x] Affected test now passes with Shenandoah, continues to pass with G1,
>> Serial, Parallel
>> - [x] `sun/tools/jhsdb` passes with {Serial, Parallel, G1, Shenandoah}
>> - [x] `serviceability/sa` passes with {Serial, Parallel, G1, Shenandoah}
>
> src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/tools/HeapSummary.java
> line 89:
>
>> 87: printValMB("CompressedClassSpaceSize = ",
>> getFlagValue("CompressedClassSpaceSize", flagMap));
>> 88: printValMB("MaxMetaspaceSize = ",
>> getFlagValue("MaxMetaspaceSize", flagMap));
>> 89: printValMB("G1HeapRegionSize = ", HeapRegion.grainBytes());
>
> This call to grainBytes() is suspect because it assumes G1 is being used even
> though it may not be. That was the case even before your changes because it
> was being called for SerialGC and ParallelGC. Perhaps the old code is better,
> but also fix it to only print G1HeapRegionSize for G1. I'd also suggest
> fixing the test to just remove the check for G1HeapRegionSize. It's not
> important.
I am happy to just drop the check to `G1HeapRegionSize` in the test.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/15328#discussion_r1297406539
