On Thu, 17 Aug 2023 15:51:47 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}
>
> Aleksey Shipilev has updated the pull request incrementally with one
> additional commit since the last revision:
>
> More fixes
Looks okay.
Thanks,
Serguei
src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/tools/HeapSummary.java line
90:
> 88: printValMB("MaxMetaspaceSize = ",
> getFlagValue("MaxMetaspaceSize", flagMap));
> 89: if (heap instanceof G1CollectedHeap) {
> 90: printValMB("G1HeapRegionSize = ",
> HeapRegion.grainBytes());
Nit: I'd suggest to move this line to the `G1CollectedHeap` specific block
after line 123. It would be consistent with your change for
`ShenandoahRegionSize`. Probably, the test needs to be updated to adopt to this
tweak as well.
-------------
Marked as reviewed by sspitsyn (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/15328#pullrequestreview-1583054266
PR Review Comment: https://git.openjdk.org/jdk/pull/15328#discussion_r1297495312
