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

[Kim Barrett]

On Wed, 7 Jun 2023 11:29:06 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.
>
> Afshin Zafari has updated the pull request incrementally with one additional commit since the last revision:
> 
>   some more intx to int conversions.

Changes requested by kbarrett (Reviewer).

src/hotspot/share/gc/g1/g1Arguments.cpp line 232:

> 230:   // By default do not let the target stack size to be more than 1/4 of the entries
> 231:   if (FLAG_IS_DEFAULT(GCDrainStackTargetSize)) {
> 232:     FLAG_SET_ERGO(GCDrainStackTargetSize, MIN2(GCDrainStackTargetSize, (uint)TASKQUEUE_SIZE / 4));

TASKQUEUE_SIZE should be a constant of type size_t rather than a macro.  GCDrainStackTargetSize should be of type size_t.  Then we don't need any casts here.  But some uses may need cleaning up, either to remove casts or to adjust surrounding types.  I suggest this be split out as a separate RFE to avoid cluttering the simplicity of this PR.

src/hotspot/share/gc/parallel/jvmFlagConstraintsParallel.cpp line 50:

> 48:       JVMFlag::printError(verbose,
> 49:                           "InitialTenuringThreshold (" UINTX_FORMAT ") must be "
> 50:                           "less than or equal to MaxTenuringThreshold (%u)\n",

(github UI won't let me attach comment to the function signature)
The type of the value parameter should be changed in conjunction with the type change for the option,
both here and in jvmFlagConstraintsParallel.hpp.  Similarly for MaxTenuringThresholdXXX below.

src/hotspot/share/gc/parallel/psScavenge.cpp line 556:

> 554:                                                            survivor_limit);
> 555: 
> 556:        log_debug(gc, age)("Desired survivor size " SIZE_FORMAT " bytes, new threshold %u (max threshold %u)",

Consider also, instead of SIZE_FORMAT we can use "%zu".

src/hotspot/share/gc/shared/ageTable.cpp line 102:

> 100: 
> 101: 
> 102:   log_debug(gc, age)("Desired survivor size " SIZE_FORMAT " bytes, new threshold " UINTX_FORMAT " (max threshold %u)",

Consider also, instead of SIZE_FORMAT we can use "%zu".

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

> 432: const uintx max_uintx = (uintx)-1;
> 433: 
> 434: 

stray blank line insertion?

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

> 439: // max_uintx            0xFFFFFFFF      0xFFFFFFFFFFFFFFFF
> 440: 
> 441: typedef unsigned int uint;   NEEDS_CLEANUP

Unrelated, but I wonder why this "NEEDS_CLEANUP"?

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

PR Review: https://git.openjdk.org/jdk/pull/14259#pullrequestreview-1467936042
PR Review Comment: https://git.openjdk.org/jdk/pull/14259#discussion_r1221761557
PR Review Comment: https://git.openjdk.org/jdk/pull/14259#discussion_r1221770271
PR Review Comment: https://git.openjdk.org/jdk/pull/14259#discussion_r1221773761
PR Review Comment: https://git.openjdk.org/jdk/pull/14259#discussion_r1221774494
PR Review Comment: https://git.openjdk.org/jdk/pull/14259#discussion_r1221778201
PR Review Comment: https://git.openjdk.org/jdk/pull/14259#discussion_r1221779001