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

[Richard Reingruber]

On Tue, 21 Mar 2023 17:04:56 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 incrementally with one additional commit since the last revision:
> 
>   Feedback Coleene

The tests failed because `java_lang_ClassLoader::isAncestor()` must not be called for the boot loader. Protection against this got lost with the refactoring of the last change (https://github.com/openjdk/jdk/pull/12802/commits/2c2e49b5a01cd58d6f922ce105964c083ee18bfb)

So while the boot loader never needs an explicit dependency to the holder of an ordinary class (afaik) because it never delegates loading, it does have explicit dependencies to classes with class mirror holder (e.g. java.lang.invoke.LambdaForm$MH/0x0000000800082800). Therefore checking CLD dependencies cannot be skipped in `check_path_to_callee()` if `from_cld` should be the boot loader cld. Instead the `isAncestor()` call is only done if `from_cld` isn't the boot loader. `isAncestor()` could also be changed to return false if the first arg is the boot loader by simply converting the do-while-loop into a while-loop but then it wouldn't be a verbatim copy from ClassLoader.java anymore.

One test failed in GHA. This is hardly related to these changes:


TEST: jdk/internal/misc/ThreadFlock/ThreadFlockTest.java#platform
[...]
org.opentest4j.AssertionFailedError: Duration 5556ms, expected <= 4000ms ==> expected: <true> but was: <false>
	at org.junit.jupiter.api.AssertionUtils.fail(AssertionUtils.java:55)
	at org.junit.jupiter.api.AssertTrue.assertTrue(AssertTrue.java:40)
	at org.junit.jupiter.api.Assertions.assertTrue(Assertions.java:210)
	at ThreadFlockTest.checkDuration(ThreadFlockTest.java:1090)
	at ThreadFlockTest.testAwaitAllWithTimeout1(ThreadFlockTest.java:431)
[...]

Still in draft mode until local CI tests have finished.
(That'll take 24h. Last night the patch wasn't tested because of a conflict after NULL->nullptr conversion)

No issues in test.

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

PR Comment: https://git.openjdk.org/jdk/pull/12802#issuecomment-1479716413
PR Comment: https://git.openjdk.org/jdk/pull/12802#issuecomment-1480790983
PR Comment: https://git.openjdk.org/jdk/pull/12802#issuecomment-1480883996
PR Comment: https://git.openjdk.org/jdk/pull/12802#issuecomment-1483252788