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