RFR: 8282024: add EscapeAnalysis statistics under PrintOptoStatistics [v8]

[Vladimir Kozlov]

On Wed, 4 May 2022 01:36:13 GMT, aamarsh <duke at openjdk.java.net> wrote:

>> Escape Analysis and Scalar Replacement statistics were added when the -XX:+PrintOptoStatistics flag is set. All code is placed in `#ifndef Product` block, so this code is only run when creating a debug build. Using renaissance benchmark I ran a few tests to confirm that numbers were printing correctly. Below is an example run:
>> 
>> 
>> No escape = 372, Arg escape = 74, Global escape = 1855 (EA executed in   10.49 seconds)
>> Objects scalar replaced = 240, Monitor objects removed = 44, GC barriers removed = 37, Memory barriers removed = 284
>
> aamarsh has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains one new commit since the last revision:
> 
>   adding escape analysis and scalar replacement statistics

I have few comments.

src/hotspot/share/opto/compile.cpp line 2217:

> 2215:     Atomic::add(&ConnectionGraph::_arg_escape_counter, _local_arg_escape_ctr);
> 2216:     Atomic::add(&ConnectionGraph::_global_escape_counter, _local_global_escape_ctr);
> 2217: #endif

These should be done inside  `ConnectionGraph` - don't expose EA counters to an other class. You can use a static method in `ConnectionGraph` to do that.

`_no_escape_counter, _local_no_escape_ctr + total_scalar_replaced` is wrong. You are doubling number because `total_scalar_replaced` is part of `_local_no_escape_ctr`. Keep these numbers separate. Also `mexp._local_scalar_replaced` could be update later during `PhaseMacroExpand::expand_macro_nodes()` call after loop optimizations.

And such collection is not accurate (over-counted) due to EA iterations - each iteration may add the same numbers. Which could be fine if you say that in comments so people know.

src/hotspot/share/opto/escape.cpp line 128:

> 126:   // casting jlong to long since Atomic needs Integral type
> 127:   Atomic::add(&ConnectionGraph::_time_elapsed, (long)et.milliseconds());
> 128: #endif

You should check `PrintOptoStatistic` flag.

src/hotspot/share/opto/escape.cpp line 248:

> 246: #ifndef PRODUCT
> 247:     escape_state_statistics(java_objects_worklist);
> 248: #endif

You can use `NOT_PRODUCT()` macro for one line which you have a lot in these changes.

src/hotspot/share/opto/escape.cpp line 3781:

> 3779: }
> 3780: 
> 3781: void ConnectionGraph::escape_state_statistics(GrowableArray<JavaObjectNode*>& java_objects_worklist) {

You should check `PrintOptoStatistic` flag to avoid useless work.

src/hotspot/share/opto/escape.cpp line 3784:

> 3782:   _compile->_local_no_escape_ctr = 0;
> 3783:   _compile->_local_arg_escape_ctr = 0;
> 3784:   _compile->_local_global_escape_ctr = 0;

I don't think you need these `Compile` counters - make them local to update corresponding `ConnectionGraph` counters after loop. As I said these counters are not accurate anyway. Unless you want to track for which allocation which counter was recorded. Which I think will be over-kill because the most EA done with one iteration.

src/hotspot/share/opto/macro.hpp line 216:

> 214:     static int _GC_barriers_removed_counter;
> 215:     static int _memory_barriers_removed_counter;
> 216:     int _local_scalar_replaced;

You don't need `_local_scalar_replaced` value as I said in an other comment.

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

Changes requested by kvn (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/8019