On Wed, 20 Dec 2023 02:40:40 GMT, Andrei Pangin <apan...@openjdk.org> wrote:
>> 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 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()` Yes, changed to use `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. Thanks, that's indeed simpler. > 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. Changed to `clear_directive_flags`. > 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. Good point, updated the comment. This btw means that in another places this flag can be reused. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/14111#discussion_r1434883459 PR Review Comment: https://git.openjdk.org/jdk/pull/14111#discussion_r1434884163 PR Review Comment: https://git.openjdk.org/jdk/pull/14111#discussion_r1434884612 PR Review Comment: https://git.openjdk.org/jdk/pull/14111#discussion_r1434885291
