RFR: 8301403: Eliminate memory allocations during signal handling

[David Holmes]

On Tue, 16 Jul 2024 20:44:00 GMT, Gerard Ziemski <gziemski at openjdk.org> wrote:

> Allocating memory while handling an error should be avoided, however `JVMFlag::printFlags()` is used by crash log and it allocates memory to print flags sorted (using qsort).
> 
> We avoid memory allocation by using a simple in place algorithm that uses JVMFlag 1 bit of unused data from its private `Flags` enum data member. It is O(n^2) algorithm, compared to O(n*log(n)) for qsort, however, it's called while handling an error log, so the speed here is not paramount. Also, I measured the real impact on a simple test case and I actually observed performance improvement of about x2.5 faster (2,885,973ns in place ordered printing vs 7,389,456ns for qsort). This is because we ended up having to qsort all flags when only a fraction of them actually end up being printed.
> 
> Running MACH5 tests...

Nice idea in general, as long as the performance aspect is okay. A few minor issues.

Also the comment block at the start of the method needs updating.

Thanks

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?

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"

src/hotspot/share/runtime/flags/jvmFlag.cpp line 717:

> 715:         if (max == nullptr) {
> 716:           max = &flagTable[i];
> 717:         }

Should be a `continue` here.

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`.

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

Changes requested by dholmes (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/20202#pullrequestreview-2181814751
PR Review Comment: https://git.openjdk.org/jdk/pull/20202#discussion_r1680367525
PR Review Comment: https://git.openjdk.org/jdk/pull/20202#discussion_r1680367622
PR Review Comment: https://git.openjdk.org/jdk/pull/20202#discussion_r1680368082
PR Review Comment: https://git.openjdk.org/jdk/pull/20202#discussion_r1680368876