On Thu, 14 Dec 2023 15:29:06 GMT, Dmitry Chuyko <dchu...@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