RFR: 8366118: DontCompileHugeMethods is not respected with -XX:-TieredCompilation [v3]

Jiangli Zhou jiangli at openjdk.org
Fri Aug 29 01:04:55 UTC 2025


On Wed, 27 Aug 2025 19:03:59 GMT, Man Cao <manc at openjdk.org> wrote:

>>> AFAICT, the block of code here is intended for handling the case when intermediate is not disabled. Your change subtly alters that.
>>> When TieredCompilation is disabled, the large method compilation is done via CompileBroker::compile_method if !CompileBroker::compilation_is_in_queue(mh) is true. I confirmed that in lldb, see below. Is there any reason to not do can_be_compiled check when calling CompileBroker::compile_method?
>> 
>> Trying to compile the large method under `-XX:-TieredCompilation` is the bug. The large method should not be compiled under `-XX:+DontCompileHugeMethods`.
>> 
>> The bug is caused by erroneously guarding the `!can_be_compiled()` and `!can_be_osr_compiled()` checks behind `!CompilationModeFlag::disable_intermediate()`. The correct behavior is to do the following checks and returns regardless of `TieredCompilation`:
>> 
>> if ((bci == InvocationEntryBci && !can_be_compiled(mh, level))) {
>>   return;
>> }
>> if ((bci != InvocationEntryBci && !can_be_osr_compiled(mh, level))) {
>>   return;
>> }
>> 
>> Only the recursive call to `compile(mh, bci, CompLevel_simple, THREAD)` and `osr_nm->make_not_entrant()` need to be guarded under `!disable_intermediate()`.
>> 
>> It is possible to add the above two checks for `bci`, `can_be_compiled()` and `!can_be_osr_compiled()` to inside `CompileBroker::compile_method()`, specifically inside `CompileBroker::compilation_is_prohibited()`. If compiler-dev team prefers this way, we could move them.
>
> To answer more directly:
>> Additionally, should can_be_compiled check only be done for c2 compilation or if it should also be applied to c1 compilation?
> 
> `can_be_compiled()` and `can_be_osr_compiled()` should be applied to both C1 and C2 compilation.
> 
>> Is there any reason to not do `can_be_compiled` check when calling `CompileBroker::compile_method`?
> 
> My rationale is to keep the code similar prior to [JDK-8251462](https://bugs.openjdk.org/browse/JDK-8251462). As mentioned above, it is possible to add those checks to `CompileBroker::compile_method()` or `CompileBroker::compilation_is_prohibited()`. I could do that if there's a strong preference. A potential issue with that approach is that the code here in `CompilationPolicy::compile()` is still confusing: why do those two `if (...) { return;}` checks only apply to `!CompilationModeFlag::disable_intermediate()`.

Thanks for addressing my questions. The change seems fine.

One of my other comment/question that I raised in internal code review was history context on why the size limit was set. I could not find historical info on that by searching the bugs in https://bugs.openjdk.org. That's however indirectly related to your fix.

The jtreg test looks good. Thanks for adding.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/26932#discussion_r2308876145


More information about the hotspot-compiler-dev mailing list