RFR: 8309271: A way to align already compiled methods with compiler directives [v15]

[Andrei Pangin]

On Thu, 14 Dec 2023 15:29:06 GMT, Dmitry Chuyko <dchuyko at openjdk.org> wrote:

>> Compiler Control (https://openjdk.org/jeps/165) provides method-context dependent control of the JVM compilers (C1 and C2). The active directive stack is built from the directive files passed with the `-XX:CompilerDirectivesFile` diagnostic command-line option and the Compiler.add_directives diagnostic command. It is also possible to clear all directives or remove the top from the stack.
>> 
>> A matching directive will be applied at method compilation time when such compilation is started. If directives are added or changed, but compilation does not start, then the state of compiled methods doesn't correspond to the rules. This is not an error, and it happens in long running applications when directives are added or removed after compilation of methods that could be matched. For example, the user decides that C2 compilation needs to be disabled for some method due to a compiler bug, issues such a directive but this does not affect the application behavior. In such case, the target application needs to be restarted, and such an operation can have high costs and risks. Another goal is testing/debugging compilers.
>> 
>> It would be convenient to optionally reconcile at least existing matching nmethods to the current stack of compiler directives (so bypass inlined methods).
>> 
>> Natural way to eliminate the discrepancy between the result of compilation and the broken rule is to discard the compilation result, i.e. deoptimization. Prior to that we can try to re-compile the method letting compile broker to perform it taking new directives stack into account. Re-compilation helps to prevent hot methods from execution in the interpreter.
>> 
>> A new flag `-r` has beed introduced for some directives related to compile commands: `Compiler.add_directives`, `Compiler.remove_directives`, `Compiler.clear_directives`. The default behavior has not changed (no flag). If the new flag is present, the command scans already compiled methods and puts methods that have any active non-default matching compiler directives to re-compilation if possible, otherwise marks them for deoptimization. There is currently no distinction which directives are found. In particular, this means that if there are rules for inlining into some method, it will be refreshed. On the other hand, if there are rules for a method and it was inlined, top-level methods won't be refreshed, but this can be achieved by having rules for them.
>> 
>> In addition, a new diagnostic command `Compiler.replace_directives...
>
> Dmitry Chuyko has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 33 commits:
> 
>  - Merge branch 'openjdk:master' into compiler-directives-force-update
>  - Merge branch 'openjdk:master' into compiler-directives-force-update
>  - Merge branch 'openjdk:master' into compiler-directives-force-update
>  - Merge branch 'openjdk:master' into compiler-directives-force-update
>  - Merge branch 'openjdk:master' into compiler-directives-force-update
>  - Merge branch 'openjdk:master' into compiler-directives-force-update
>  - Merge branch 'openjdk:master' into compiler-directives-force-update
>  - Merge branch 'openjdk:master' into compiler-directives-force-update
>  - Merge branch 'openjdk:master' into compiler-directives-force-update
>  - Merge branch 'openjdk:master' into compiler-directives-force-update
>  - ... and 23 more: https://git.openjdk.org/jdk/compare/fde5b168...44d680cd

src/hotspot/share/code/codeCache.cpp line 1409:

> 1407:   while(iter.next()) {
> 1408:     CompiledMethod* nm = iter.method();
> 1409:     methodHandle mh(thread, nm->method());

If there are two CompiledMethods for the same Java method, will it be scheduled for recompilation twice? Related question: if `nm` is an OSR method, does it make sense to go directly for deoptimization rather than compiling a non-OSR version?

src/hotspot/share/code/codeCache.cpp line 1413:

> 1411:       ResourceMark rm;
> 1412:       // Try the max level and let the directives be applied during the compilation.
> 1413:       int complevel = CompLevel::CompLevel_full_optimization;

Should the highest level depend on the configuration instead of the hard-coded constant? Perhaps, needs to be `highest_compile_level()`

src/hotspot/share/compiler/compilerDirectives.cpp line 750:

> 748:       if (!dir->is_default_directive() && dir->match(method)) {
> 749:         match_found = true;
> 750:         break;

`match_found` is redundant: for better readability, you may just return true. Curly braces around MutexLocker won't be needed either.

src/hotspot/share/oops/method.hpp line 820:

> 818:   // Clear the flags related to compiler directives that were set by the compilerBroker,
> 819:   // because the directives can be updated.
> 820:   void clear_method_flags() {

The function name is a bit misleading - it clears only flags related to directives.

src/hotspot/share/oops/methodFlags.hpp line 61:

> 59:    status(has_loops_flag_init         , 1 << 14) /* The loop flag has been initialized */ \
> 60:    status(on_stack_flag               , 1 << 15) /* RedefineClasses support to keep Metadata from being cleaned */ \
> 61:    status(has_matching_directives     , 1 << 16) /* The method has matching directives */ \

It's worth noting that the flag is temporary and is valid only during DCmd execution.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/14111#discussion_r1432195677
PR Review Comment: https://git.openjdk.org/jdk/pull/14111#discussion_r1432187571
PR Review Comment: https://git.openjdk.org/jdk/pull/14111#discussion_r1432200716
PR Review Comment: https://git.openjdk.org/jdk/pull/14111#discussion_r1432210229
PR Review Comment: https://git.openjdk.org/jdk/pull/14111#discussion_r1432212171