On Mon, 13 May 2024 13:03:26 GMT, Evgeny Astigeevich <eastigeev...@openjdk.org> wrote:
> Backout of [JDK-8309271](https://bugs.openjdk.org/browse/JDK-8309271) which > has known bugs, possible bugs and performance issues. REDO work is tracked by > [JDK-8331749](https://bugs.openjdk.org/browse/JDK-8331749). > > Found bugs: > - When refreshing `CompilerDirectivesAddDCmd::execute` will call > `DirectivesStack::hasMatchingDirectives(mh, true)` which only considers the > compiler directive which is on the top of the directives stack. As more than > one directive can be added, `CompilerDirectivesAddDCmd::execute` will not > behave as expected. > - A Java method with old directives might be in the compilation queue. A > request to recompile it with new directives will be ignored. > > There are other concerns: bugs and performance issues. > > Possible bugs: > - `has_matching_directives` might not be cleared. A nmethod might get into > the unloading state before `CodeCache::recompile_marked_directives_matches`. > If the nmethod has been used to mark a Java method and it is the only > nmethod, there will be no nmethod in CodeCache to reach the Java method to > clear the mark. > - A Java method might have been compiled with new directives before > `CodeCache::recompile_marked_directives_matches`. > `CodeCache::recompile_marked_directives_matches` will recompile it again. > - JIT compiler might be compiling a Java method with old directives. A > request to recompile it with new directives will be ignored. > > Performance issues: > - Usually directives are updated for a small number of Java methods. If > CodeCache has thousands of nmethods, > `CodeCache::recompile_marked_directives_matches` will be traversing nmethods > most of which don't need recompilation. > > The backout is not clean because of removal of `CompiledMethod`. > > Tested with release and fastdebug builds: tier1 and tier2 passed. I agree with this backout. Thank you @eastig for explaining your point. We have about 3 weeks before RDP1 and it is better we have less issues before that. Let redo implementation in next release taking into account the issues you found and have more time for testing. ------------- Marked as reviewed by kvn (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19215#pullrequestreview-2053940066
