RFR: 8340141: C1: rework ciMethod::equals following 8338471 [v3]

[Dean Long]

On Fri, 11 Oct 2024 20:28:31 GMT, Vladimir Ivanov <vlivanov at openjdk.org> wrote:

>> C1 does call 
>> 
>> 2159            dependency_recorder()->assert_evol_method(inline_target);
>> 
>> which in the CHA case would be `cha_monomorphic_target`, not `target`, so it looks like we may not detect if `target` is stale as long as `cha_monomorphic_target` is not.  It seems like a minor loophole, but I'm not sure what kind of problems it could cause, especially if the bytecodes of `target` are not used.
>
> The scenario which concerns me is performance-related. If CHA conservatively disables inlining when concurrent class redefinition takes place during parsing, then there is no mechanism in place to recuperate possible loss of performance. Normally, if inlined method is redefined later during compilation, nmethod installation fails during dependency validation. But here no inlining happens (missed optimization opportunity) and call sites in generated code are resolved based on symbolic information (except rare cases when resolved method is attached to the call site, see `SharedRuntime::find_callee_info_helper()` for details), so there are no guarantees the stale `Method*` is recorded.
> 
> I agree that the window for such sequence of events is narrow, but it may be a source of surprising performance anomalies in rare cases.

OK, it sounds like we have two choices.  Either record an evol dependency every time we short-circuit an optimization based on is_old, or bail out.  I vote for bailing out.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21148#discussion_r1797425086