RFR: JDK-8317683: Add JIT memory statistics [v3]

[Thomas Stuefe]

On Thu, 12 Oct 2023 09:24:12 GMT, Andrew Dinn <adinn at openjdk.org> wrote:

>> Thomas Stuefe has updated the pull request incrementally with two additional commits since the last revision:
>> 
>>  - remove stray newlines
>>  - MemStat->CollectMemStat
>
> src/hotspot/share/compiler/compilerOracle.hpp line 65:
> 
>> 63:   option(PrintInlining, "PrintInlining", Bool) \
>> 64:   option(PrintIntrinsics, "PrintIntrinsics", Bool) \
>> 65:   option(PrintMemStat, "PrintMemStat", Bool)   \
> 
> Ok, I still think it would be more coherent for users to have a single 3-way option
> 
>   MemStat=Off/Collect/Print  # or None instead of Off or whatever
> 
> I looked further into how the option validation works and it seems it would be possible to provide a single configuration option MemStat of type Ccstr to implement this. In order for it to work you would need to employ a validator in method scan_value (file compilerOracle.cpp:635) for the case when a MemStat option turns up during Ccstr option processing -- very like what happens for Ccstrlist options such as ControlIntrinsic. If the string text was not one of the 3 legitimate options it would be rejected.
> Your methods which test whether to collect or print would need to be tweaked accordingly to check the string value (or probably just its first element).
> I'll leave it up to you whether or not you want to make this change.

I would like to leave it as it is:

I managed to do what you advised: use cstr for an enum-like option "memstat" with a value parser that accepts (and canonicalizes) "Print" and "Collect". 

I also managed to give it a default *value*, since that did not work before - so, omitting the value would equal to empty string which would equal some reasonable value default for the option.

One remaining problem was that I needed a quick way to find out if, for any method, the print option had been set. I need this at VM shutdown - if any directive exists that caused printing, we want to print the final report. Here I either would have to scan through all directives, or remember the state in a global flag like `CompilerOracle::_quiet`. It is all possible and solvable, but at this point, I would like to give up and wrap up this patch. 

We can re-visit this topic at a later date maybe by adding support for enums. There are other option combinations that would benefit being turned into an enum too, e.g. `NoRTMLockEliding` and `UseRTMLockEliding`.

One minor aesthetic point is that "PrintMemStat" fits nicely with the other "PrintXXX" options, and feels like something every compiler dev knows.

Thanks again for the review.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16076#discussion_r1357037542