RFR: 8281455: Change JVM options with small ranges from 64 to 32 bits, for gc_globals.hpp [v2]
Afshin Zafari
azafari at openjdk.org
Thu Jun 8 09:44:54 UTC 2023
On Wed, 7 Jun 2023 15:06:54 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:
>> Afshin Zafari has updated the pull request incrementally with one additional commit since the last revision:
>>
>> some more intx to int conversions.
>
> 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.
The change is applied and some cleanup made. Maybe no need to a new RFE, if you agree.
> 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.
Fixed.
> 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".
`%zu` is used instead.
> 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".
`%zu` is used.
> src/hotspot/share/utilities/globalDefinitions.hpp line 434:
>
>> 432: const uintx max_uintx = (uintx)-1;
>> 433:
>> 434:
>
> stray blank line insertion?
Fixed.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/14259#discussion_r1222726600
PR Review Comment: https://git.openjdk.org/jdk/pull/14259#discussion_r1222726943
PR Review Comment: https://git.openjdk.org/jdk/pull/14259#discussion_r1222727513
PR Review Comment: https://git.openjdk.org/jdk/pull/14259#discussion_r1222727820
PR Review Comment: https://git.openjdk.org/jdk/pull/14259#discussion_r1222728790
More information about the hotspot-gc-dev
mailing list