RFR: 8281455: Change JVM options with small ranges from 64 to 32 bits, for gc_globals.hpp

Thomas Schatzl tschatzl at openjdk.org
Wed Jun 7 09:47:58 UTC 2023


On Thu, 1 Jun 2023 11:15:48 GMT, Afshin Zafari <azafari at openjdk.org> wrote:

> The `uintx/intx` options whose ranges are small enough are changed to `uint/int`, otherwise gcc complains 
> when `-Wconversion` is used in build.
> Their uses in printf formats are changed accordingly.
> 
> ### Tests
> Locally hotspot:tier1 tested on linux-x64
> mach5 tiers 1-4 on Linux and Windows 64bits platforms passed.

More candidates:

ParGCArrayScanChunk
MarkStackSize(Max) (these should probably be a `size_t`)
VerifyArchivedFields
ScavengeALotInterval
FullGCALotInterval
FullGCALotStart
FullGCALotDummies
VerifyGCLevel
MaxTenuringThreshold
InitialTenuringThreshold
GCDrainStackTargetSize

Some of these aren't uintx->uint conversions, so feel free to ignore.

src/hotspot/share/gc/parallel/psAdaptiveSizePolicy.cpp line 352:

> 350:           "PSAdaptiveSizePolicy::compute_eden_space_size: gc time limit"
> 351:           " gc_cost: %f "
> 352:           " GCTimeLimit: " UINT32_FORMAT,

We use `%u` as format specifiers for uint.

src/hotspot/share/gc/shared/gcOverheadChecker.cpp line 94:

> 92:   if (UseGCOverheadLimit) {
> 93:     if (gc_overhead_limit_exceeded()) {
> 94:       log_trace(gc, ergo)("GC is exceeding overhead limit of %d%%", GCTimeLimit);

Not sure why this place uses `%d` as format specifier compared to the `UINT32_FORMAT` the previous places did. Use `%u` ;)

src/hotspot/share/gc/shared/gc_globals.hpp line 169:

> 167:           "(effective only when using concurrent collectors)")              \
> 168:                                                                             \
> 169:   product(uint, GCLockerEdenExpansionPercent, 5,                           \

The `` needs to be aligned back.

src/hotspot/share/utilities/globalDefinitions.hpp line 442:

> 440: 
> 441: typedef unsigned int uint;   NEEDS_CLEANUP
> 442: const uint  max_uint  = (uint)-1;

The change should use the existing `UINT_MAX` instead of adding a new constant. Instead of a cast of `-1`, one should also use `~0` to get all-ones.

-------------

Changes requested by tschatzl (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/14259#pullrequestreview-1467140885
PR Review Comment: https://git.openjdk.org/jdk/pull/14259#discussion_r1221262042
PR Review Comment: https://git.openjdk.org/jdk/pull/14259#discussion_r1221263784
PR Review Comment: https://git.openjdk.org/jdk/pull/14259#discussion_r1221264239
PR Review Comment: https://git.openjdk.org/jdk/pull/14259#discussion_r1221267209


More information about the hotspot-gc-dev mailing list