RFR: 8324241: Always record evol_method deps to avoid excessive method flushing [v2]

[Dean Long]

On Tue, 23 Jan 2024 16:37:35 GMT, Volker Simonis <simonis at openjdk.org> wrote:

>> src/hotspot/share/runtime/init.cpp line 121:
>> 
>>> 119:   if (AlwaysRecordEvolDependencies) {
>>> 120:     JvmtiExport::set_can_hotswap_or_post_breakpoint(true);
>>> 121:     JvmtiExport::set_all_dependencies_are_recorded(true);
>> 
>> I think the check for AlwaysRecordEvolDependencies needs to be moved into set_can_hotswap_or_post_breakpoint and set_all_dependencies_are_recorded, otherwise don't we risk the value being accidentally reset to false when set_can_hotswap_or_post_breakpoint() is called again by JvmtiManageCapabilities::update()?
>
> A good question, but after deep digging (it took me quite some time to figure this out myself :) I don't think that `can_hotswap_or_post_breakpoint`/`all_dependencies_are_recorded` can ever be reset. Here's how it works:
> 
> - there's a global set of `always_capabilities` which is initialized when the first JVMTI environment is created:
> 
> // capabilities which are always potentially available
> jvmtiCapabilities JvmtiManageCapabilities::always_capabilities;
> 
> void JvmtiManageCapabilities::initialize() {
>   _capabilities_lock = new Mutex(Mutex::nosafepoint, "Capabilities_lock");
>   always_capabilities = init_always_capabilities();
> 
> 
> - as the name implies, this set of capabilities contains all generally available capabilities (except for the `onload` and `solo` capabilites, which are maintained in separate sets).
> - `JvmtiManageCapabilities::update()` only operates on this global sets of capabilites (and *not* on the concrete capabilities of a specific JVMTI environment):
> 
> void JvmtiManageCapabilities::update() {
>   jvmtiCapabilities avail;
>   // all capabilities
>   either(&always_capabilities, &always_solo_capabilities, &avail);
> ...
>   // If can_redefine_classes is enabled in the onload phase then we know that the
>   // dependency information recorded by the compiler is complete.
>   if ((avail.can_redefine_classes || avail.can_retransform_classes) &&
>       JvmtiEnv::get_phase() == JVMTI_PHASE_ONLOAD) {
>     JvmtiExport::set_all_dependencies_are_recorded(true);
>   }
> ...
>   JvmtiExport::set_can_hotswap_or_post_breakpoint(
>     avail.can_generate_breakpoint_events ||
>     avail.can_redefine_classes ||
>     avail.can_retransform_classes);
> 
> - This means that `JvmtiManageCapabilities::update()` is always conservative regarding its exports to `JvmtiExport` as it always exports *all* potentially available capabilites once a JVMTI environment is created and not only the specific capabilities requested by concrete JVMTI environment. This means that even if we create a JVMTI agent with an empty set of capabilities, `can_hotswap_or_post_breakpoint`/`all_dependencies_are_recorded` will still be set in `JvmtiExport`.
> - There's no code path (at least I couldn't find one) which takes capabilities away from the `always_capabilities` set. Only if an environment requests `on_load` capabilities, they will be transferred from the global `onload_capabilities` to the `always_capabilities` set:
> 
> jvmtiError JvmtiManageCapabilities::add_capabilities(const jvmtiCapabilities *current,
>                      ...

An assert works for me.  Thanks.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17509#discussion_r1464120039