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

[Andrew Dinn]

On Wed, 11 Oct 2023 13:41:20 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> This proposal adds a way to monitor C1/C2 memory usage per compilation. 
>> 
>> We are flying a bit blind there: NMT is okay for initial triaging but no help when digging deeper into compiler footprint. NMT shows how much native memory the JIT uses but lumps together footprints from concurrent compilations, making its peak values arbitrary. And we have no way to associate the NMT numbers with individual allocations.
>> 
>> The statistic added by this patch shows footprint *per compilation*. It measures memory spikes per thread for a single compilation. It then presents them with additional information, e.g., the node count (for C2). 
>> 
>> Example printout:
>> 
>> 
>> 112703:
>> Compilation memory statistics
>> 
>> Legend:
>>   total  : memory allocated via arenas while compiling
>>   NA     : ...how much in node arenas (if c2)
>>   RA     : ...how much in resource areas
>>   #nodes : ...how many nodes (if c2)
>>   time   : time of last compilation (sec)
>>   type   : compiler type
>>   #rc    : how often recompiled
>>   thread : compiler thread
>> 
>> total     NA        RA        #nodes  time    type  #rc thread              method
>> 30273336  6383040   18027000  18257   1,979   c2    2   0x00007f6f0c136460  java/lang/ClassLoader.loadClass((Ljava/lang/String;)Ljava/lang/Class;) 
>> 27690456  5892160   16211800  15179   3,643   c2    2   0x00007f6f0c0433e0  org/springframework/cache/interceptor/CacheOperationSourcePointcut.matches((Ljava/lang/reflect/Method;Ljava/lang/Class;)Z) 
>> ...
>> ...
>> 
>> 
>> [full example printout here](https://github.com/openjdk/jdk/files/12853477/compiler-statistic-petclinic.txt)
>> 
>> #### Usage
>> 
>> The statistic needs to be enabled with a new compile command (`-XX:CompileCommand="CollectMemStat,*.*` or `-XX:CompileCommand="PrintMemStat,*.*"`). 
>> 
>> It then can be printed with a new jcmd: `jcmd xxx Compiler.memory`.
>> 
>> `PrintMemStat` also prints the usage out after compilation and again, as sorted table, at VM shutdown.
>> 
>> #### Technical details
>> 
>> The patch exploits the fact that compilers use arenas almost exclusively. It hooks into `Arena::grow()` to track allocations. That makes memory tracking very cheap at an acceptable loss in fidelity (we only track arena chunk allocations, not individual Arena allocations).
>> 
>> Peak values are tracked per compiler thread. That works since all arenas used during compilation are associated with a compiler thread and never change that association during the compilation.
>> 
>> We measure total footprint...
>
> Thomas Stuefe has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - remove stray newlines
>  - MemStat->CollectMemStat

Ok, modulo the typos.

src/hotspot/share/compiler/compilationMemoryStatistic.cpp line 434:

> 432:       }
> 433:     } else {
> 434:       st->print_cr("No initialized.");

No initialized --> Not initialized ?

src/hotspot/share/compiler/compilationMemoryStatistic.hpp line 44:

> 42:   // bytes when compilation started
> 43:   size_t _start;
> 44:   // bytes at least peak, total

least --> last ?

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.

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

Marked as reviewed by adinn (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/16076#pullrequestreview-1673639984
PR Review Comment: https://git.openjdk.org/jdk/pull/16076#discussion_r1356501654
PR Review Comment: https://git.openjdk.org/jdk/pull/16076#discussion_r1356500684
PR Review Comment: https://git.openjdk.org/jdk/pull/16076#discussion_r1356545332