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