RFR: 8337031: Improvements to CompilationMemoryStatistic

[Ashutosh Mehra]

On Fri, 26 Jul 2024 06:08:03 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> Some minor improvements to CompilationMemoryStatistic. More details are in [JDK-8337031](https://bugs.openjdk.org/browse/JDK-8337031)
>> 
>> Testing:
>>   test/hotspot/jtreg/compiler/print/CompileCommandPrintMemStat.java
>>   test/hotspot/jtreg/serviceability/dcmd/compiler/CompilerMemoryStatisticTest.java
>
> src/hotspot/share/compiler/compilationMemoryStatistic.cpp line 118:
> 
>> 116:   if (total != _current) {
>> 117:     log_info(compilation, alloc)("WARNING!!! Total does not match current");
>> 118:   }
> 
> Why do we calculate total? Just for this test? I would then put this into an ASSERT section, and make the info log an assert. 
> 
> However, I wonder if this is really needed. The logic updating both _current and _tags_size is pretty straightforward, I don't see how there could be a mismatch.

This code should not have been there. I forgot to remove it. There is no use of `total` here.

> src/hotspot/share/compiler/compilationMemoryStatistic.cpp line 204:
> 
>> 202:   size_t _total;
>> 203:   // usage per arena tag when total peaked
>> 204:   size_t _tags_size_at_peak[Arena::tag_count()];
> 
> Can you please make sure Arena::tag_count() evaluates to constexpr? When in doubt, just use the enum value instead.

Arena::tag_count() is declared as a constexpr. I wanted to avoid writing `static_cast<int>(Arena::Tag::tag_count)` every time I need tag_count, so I wrapped it in Arena::tag_count() and declared it with constexpr. Is that not sufficient to make it a constexpr?

> src/hotspot/share/compiler/compilationMemoryStatistic.cpp line 242:
> 
>> 240:     for (int tag = 0; tag < Arena::tag_count(); tag++) {
>> 241:       st->print_cr("  " LEGEND_KEY_FMT ": %s", Arena::tag_name[tag], Arena::tag_desc[tag]);
>> 242:     }
> 
> use x macro?

What do you mean by x macro? Do you have an example that shows the use of x macro?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20304#discussion_r1693443814
PR Review Comment: https://git.openjdk.org/jdk/pull/20304#discussion_r1693443227
PR Review Comment: https://git.openjdk.org/jdk/pull/20304#discussion_r1693445269