RFR(s): 8153771: Introduce MinPLABSize option

Thomas Schatzl thomas.schatzl at oracle.com
Mon May 8 15:45:13 UTC 2017


Hi Sangheon,

On Wed, 2017-05-03 at 16:53 -0700, sangheon wrote:
> Hi,
> 
> Could I get some reviews for this change of introducing MinPLABSize
> option?
> 
> In some cases, PLAB and TLAB requirements would be different. But 
> currently PLAB::min_size() is calculated from MinTLABSize option.
> This proposal introduces MinPLABSize option and replaces MinTLABSize
> with MinPLABSize at PLAB::min_size().
> 
> CR: https://bugs.openjdk.java.net/browse/JDK-8153771
> Webrev: http://cr.openjdk.java.net/~sangheki/8153771/webrev.0
> Testing: JPRT, TestOptionsWithRanges.java for all platforms

- globals.hpp: why is the upper bound for MinPLABSize using SIZE_MAX
and not max_uintx like MinTLABSize? (Note that this does not mean this
is wrong).

I also think the upper bound of SIZE_MAX/2 may result in overflows when
calculating using it as MinPLABSize is in HeapWords and MinTLABSize in
bytes.

- commandLineFlagConstraintsGC.cpp:154: I think the min_size calculated
here should not take AlignmentReserve into account as the calculation
in PLAB::min_size() adds another AlignmentReserve unconditionally.

Otherwise, maybe min_size should be
align_object_size(oopDesc::header_size()) + AlignmentReserve here? (And
similar changes done for MinTLABSize).

The code then also needs to check that the value given by the user is a
multiple of object_size.

That is maybe another issue though :)

- only curious: I see that using MinTLABSize as MinPLABSize is
annoying, but did some particular problem show up recently?

- adding a new product flag requires a CCC/CSR request [1]

Thanks,
  Thomas

[1] https://wiki.openjdk.java.net/display/csr/Main




More information about the hotspot-gc-dev mailing list