RFR: 8301403: Eliminate memory allocations in JVMFlag::printFlags during signal handling [v2]

Gerard Ziemski gziemski at openjdk.org
Wed Jul 17 19:46:16 UTC 2024


On Wed, 17 Jul 2024 03:49:21 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Gerard Ziemski has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   change name from max to best
>
> src/hotspot/share/runtime/flags/jvmFlag.cpp line 710:
> 
>> 708:     }
>> 709:   }
>> 710:   // Print the flag with highest sort value, then mark it
> 
> Surely it is the lowest sort value if printing in alphabetical order?

I was thinking the "highest" in the sense that it will make it appear 1st, but to avoid any misunderstandings I will use "best" instead.

> src/hotspot/share/runtime/flags/jvmFlag.cpp line 712:
> 
>> 710:   // Print the flag with highest sort value, then mark it
>> 711:   for (size_t j = 0; j < length; j++) {
>> 712:     JVMFlag* max = nullptr;
> 
> I would consider this to be the "min" not "max"

I will use "best" instead.

> src/hotspot/share/runtime/flags/jvmFlag.cpp line 717:
> 
>> 715:         if (max == nullptr) {
>> 716:           max = &flagTable[i];
>> 717:         }
> 
> Should be a `continue` here.

Fixed.

> src/hotspot/share/runtime/flags/jvmFlag.cpp line 719:
> 
>> 717:         }
>> 718:         if (strcmp(max->name(), flagTable[i].name()) > 0) {
>> 719:           max = &flagTable[i];
> 
> Just to clarify the terminology this is saying
> 
> if (max > flag[i])
>   max = flag[i]
> 
> so `max` is getting smaller and so is in fact the `min`.

I will use "best" instead.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20202#discussion_r1681628123
PR Review Comment: https://git.openjdk.org/jdk/pull/20202#discussion_r1681628337
PR Review Comment: https://git.openjdk.org/jdk/pull/20202#discussion_r1681629181
PR Review Comment: https://git.openjdk.org/jdk/pull/20202#discussion_r1681628701


More information about the hotspot-runtime-dev mailing list