On Thu, 17 Aug 2023 10:11:58 GMT, Aleksey Shipilev <sh...@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}
Changes requested by cjplummer (Reviewer).
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.
-------------
PR Review: https://git.openjdk.org/jdk/pull/15328#pullrequestreview-1582859258
PR Review Comment: https://git.openjdk.org/jdk/pull/15328#discussion_r1297370064
