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

[Dmitry Chuyko]

On Wed, 20 Dec 2023 02:40:40 GMT, Andrei Pangin <apangin at 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