RFR: 8255414: Ensure OS functions always report to NMT
Johan Sjölen
jsjolen at openjdk.org
Mon Mar 6 13:01:07 UTC 2023
On Thu, 2 Mar 2023 14:05:14 GMT, Johan Sjölen <jsjolen at openjdk.org> wrote:
> Hi,
>
> This PR attempts to enforce the convention that:
>
> 1. Public `os` functions report to NMT
> 2. `pd_` prefixed functions do not report to NMT
> 3. Public `os` functions call into `pd_`-prefixed functions to do the actual work.
>
> This is a convention that has been only partially enforced, leading to some difficulties. For example, it's easy for double-accounting to NMT can occur if it's not clear what the `pd_`-prefixed functions do and do not do.
Hi David, thanks for the input! I'm not sure which approach is the correct one, so getting this PR out for review was important as a discussion starter.
> Sorry but this is wrong IMO. A pd_ function should in general still call the os:: version of other functions because it doesn't know what that might do in addition to eventually calling the pd_ variant.
I see your point, but right now the only difference is NMT code. Previously it was just inconsistent.
> I think the functions that do the actual work should report to NMT. That way if you have a call chain:
>
> ```
> os::foo() -> os::pd_foo() -> os::bar() -> os::pd_bar();
> ```
>
> then the reporting only happens in `os::pd_bar()`.
What I dislike about this is that a reader will always go through `os::foo()` to know whether it reports to NMT or not. With this solution, the reader needs to go through the entire call chain. It seems to me like the situation might become more complex when a new codepath is added that depends on two `pd_` calls in sequence (might lead to incorrect accounting). Being 'shallow' with NMT makes sense in this context.
-------------
PR: https://git.openjdk.org/jdk/pull/12832
More information about the hotspot-runtime-dev
mailing list