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

Coleen Phillimore coleenp at openjdk.org
Fri Jun 9 14:48:46 UTC 2023


On Thu, 8 Jun 2023 09:55:44 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:
> 
>   review comments applied

I have a few comments.  Narrowing the types of these arguments was to prevent Wconversion warnings where they're used and then later narrowed to 32 bit ints.  From my sampling of some of the arguments changed, there were no casts, which was the problem.  Some of these (sample GCTimeLimit) are promoted back up to uintx but having the field be the narrowest representation allows the code to use it without an implicit conversion to a narrower int type.  So I think this is a good step for cleaning up the int types.

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

> 393:   product(uint, PausePadding, 1,                                            \
> 394:           "How much buffer to keep for pause time")                         \
> 395:           range(0, UINT_MAX)                                                \

Why did you change from max_juint -> UINT_MAX ?  max_juint should have compile without integer size mismatch.

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

PR Review: https://git.openjdk.org/jdk/pull/14259#pullrequestreview-1472203649
PR Review Comment: https://git.openjdk.org/jdk/pull/14259#discussion_r1224361701


More information about the hotspot-runtime-dev mailing list