On Sat, 22 Jun 2024 09:30:25 GMT, Evgeny Astigeevich <[email protected]>
wrote:
>> A Java method can become non-compilable if there are issues with its
>> compilation or if its compiled version causes problems. Additionally, a
>> method can be marked as non-compilable using a compile command or a compiler
>> directive. Since compiler directives can be updated, a directive that
>> disables a method's compilation can be changed or removed.
>>
>> Currently, when a Java method is marked as non-compilable, the reason for
>> this status is unknown. If a change in a compiler directive makes the method
>> compilable again, we cannot clear the non-compilable status because we don't
>> know if the directive initially caused the method to become non-compilable.
>>
>> To resolve the issue two method flags are introduced: `is_c1_exclude` and
>> `is_c2_excluded`. They mean a Java method is excluded from compilation by a
>> directive. With these flags we can find out a Java method has been excluded
>> with a directive. If the directive changes and allows compilation of the
>> method we can detect this and clear the non-compilable status.
>>
>> As accesses to flags must be race free we have to remove getting a directive
>> from `CompileBroker::compile_method`. We combine two
>> `CompileBroker::compile_method` into one. We also move calculation whether
>> compilation is blocking into `CompileBroker::compile_method_base`. The
>> directive used for that calculation is passed to a compile task, so a
>> compile task does not need to get it again.
>>
>> A regression test is added.
>>
>> Tested fastdebug build on Linux AArch64, Linux x86_64 and Windows Server
>> 2019 x86_64:
>> - Tier1/2/3 passed.
>
> Evgeny Astigeevich has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Fix data race
Changes requested by dfenacci (Committer).
src/hotspot/share/compiler/compileBroker.cpp line 1336:
> 1334: }
> 1335:
> 1336: AbstractCompiler *comp = CompileBroker::compiler(comp_level);
Do we need to change this?
src/hotspot/share/compiler/compileBroker.cpp line 1563:
> 1561: // See if this compilation is not allowed.
> 1562: bool CompileBroker::compilation_is_prohibited(const methodHandle&
> method, int osr_bci, int comp_level) {
> 1563: assert(compiler(comp_level) != nullptr, "Ensure we have a compiler");
Are you sure `compiler(comp_level)` can never be `nullptr` (not even
potentially)? (it seems so to me too but just to make sure)
-------------
PR Review: https://git.openjdk.org/jdk/pull/19637#pullrequestreview-2148291624
PR Review Comment: https://git.openjdk.org/jdk/pull/19637#discussion_r1658904857
PR Review Comment: https://git.openjdk.org/jdk/pull/19637#discussion_r1660722626
