RFR: 8317761: Combine two versions of print_statistics() in java.cpp

Vladimir Kozlov kvn at openjdk.org
Tue Oct 10 16:53:10 UTC 2023


On Mon, 9 Oct 2023 23:05:26 GMT, Ioi Lam <iklam at openjdk.org> wrote:

> The non-product version of `print_statistics()` can be compiled in the product build as well, so there's no need to keep two different versions.
> 
> BTW, for some reason `print_method_profiling_data()` is called unconditionally in non-product builds, but is guarded by `PrintMethodData` in product builds. I made the behavior the same as before in this PR. This can probably be cleaned up in a future PR.
> 
> I verified that:
> 
> - All functions call by the original product version are also called by the non-product version (but could be in different order).
> - For functions that are called by the non-product version but not call by the original product version: all such calls are guarded by non-product flags (e.g. `TimeOopMap`), or the function itself does nothing (e.g., declared as `PRODUCT_RETURN`)
> 
> Testing: tier1, tier2, build-tiers5

src/hotspot/share/runtime/java.cpp line 290:

> 288:     // TODO: why is this unconditional in non-product builds?
> 289:     print_method_profiling_data();
> 290:   }

I think it should be called unconditional always because we have check inside method.

I think the check in product VM is historical. Originally  `print_method_profiling_data()` was called only in NOT_PRODUCT and later [8037970](https://github.com/openjdk/jdk/commit/b21d142f016360986212a941659115c0c96a3426#diff-d9e4fa0ecdd187c3fce3dcb8b0344264134d7746225956636b237e26ce6ad369) add call to PRODUCT under `PrintMethodData` check (there was no this flag check inside method).
Then [8042727](https://github.com/openjdk/jdk/commit/ead7a2760b46226947dc07548ea0fa4897aef18d) added the flag check inside method to avoid unconditional walk class data (I assume in debug VM). This change made the check before call the method in product version redundant.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16110#discussion_r1352932770


More information about the hotspot-dev mailing list