RFR: 8296440: Remove Method* handling from cleanup_inline_caches_impl [v5]

[Richard Reingruber]

On Wed, 29 Mar 2023 19:54:50 GMT, Richard Reingruber <rrich at openjdk.org> wrote:

>> This PR replaces cleaning of static stubs in CompiledMethod::cleanup_inline_caches_impl() with a guarantee that it is actually not needed because the holder of the embedded target Method* is alive if the caller nmethod is not unloading.
>> 
>> The holder of the target Method* has to be alive because it is reachable from the caller nmethod's oop pool. This is checked by `check_path_to_callee()` when a statically bound call gets resolved.
>> 
>> C2i entry barriers can be removed for the same reason.
>> 
>> Testing:
>> 
>> Many rounds in our CI testing which includes most JCK and JTREG tests, Renaissance benchmark and SAP specific tests with fastdebug and release builds on the standard platforms plus PPC64.
>> 
>> I've also done tier1 and tier2 tests with -XX:-Inline and tier1 tests with ZGC.
>> 
>> I've started hotspot and jdk tier1 tests with -Xcomp. They were not finished when I stopped them after 24h.
>
> Richard Reingruber has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 14 commits:
> 
>  - Check only in debug vm
>  - Merge branch 'master'
>  - Merge branch 'master'
>  - Fix
>  - Feedback Coleene
>  - Adding TestStaticallyBoundTargetIsReachable.java
>  - Path to target exists also if the receiver is a constant of the caller
>  - Use nmethod::oops_do() to search for to_holder in from_nm
>  - Merge branch 'master'
>  - Remove MacroAssembler::resolve_weak_handle()
>  - ... and 4 more: https://git.openjdk.org/jdk/compare/ff368d50...7dd06446

So from my point of view this is ready for integration. Thanks for the help, feeback, and reviews!

Before I'd like to share some final thoughts. So the search in `check_path_to_callee()` is shallow. It might yield false positives. I hope for support by compiler folks adding dependencies at compile time, even if only needed to avoid false positives, and even more so, if no path exists.

To make this really robust, new dependencies could be added to the caller nmethod if no path to the holder of the call target is found. This is not possible atm but maybe the nmethod's oop pool could be changed to be a growable kind of (weak) OopStorage outside the code cache if needed.

An adhoc fix in the case of missing(?) path could be not to resolve the call if the callee should be interpreted. Instead the callee could be compiled in the background. Execution would continue without resolving the call.

@fisk may I ask if you are also ok with the general approach of this pr?

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

PR Comment: https://git.openjdk.org/jdk/pull/12802#issuecomment-1490429890