RFR: 8333098: ubsan: bytecodeInfo.cpp:318:59: runtime error: division by zero

[Dean Long]

On Fri, 16 Aug 2024 22:44:44 GMT, Igor Veresov <iveresov at openjdk.org> wrote:

>>>  think the existing behavior is correct - the goal is to compute a minimum call freq that is considered for inlining. 1.0/min_invocations() means call site was reached at least once before the method was submitted for a C2 compile.
>> 
>> Do we really need to take `1.0/min_invocations()` into account?  `InlineSmallCode` should handle cases like that.
>> 
>> Also `MinInlineFrequencyRatio ` is 0.0085 which corresponds to  `Tier4MinInvocationThreshold` value equal approximately  117. The default value is 600. The only way to change it is to use `CompileThreshold` or `CompileThresholdScaling` which nobody use in production but only in testing. So in production we use `MinInlineFrequencyRatio` as `min_freq`.  Why complicate the code.
>>  
>> An other issue. This is `should_not_inline()` method. Returning `false` means **inlining** called method. I was curious why we return `false` when we run with `-Xcomp` (see check `(UseInterpreter)` at line 301) ?
>> If we return `false` when `min_invocations()` is 0 it will be similar to `-Xcomp` mode.
>> 
>> Is it intentional to inline when we don't have profiling info?
>
>> > think the existing behavior is correct - the goal is to compute a minimum call freq that is considered for inlining. 1.0/min_invocations() means call site was reached at least once before the method was submitted for a C2 compile.
>> 
>> Do we really need to take `1.0/min_invocations()` into account? `InlineSmallCode` should handle cases like that.
>> 
> 
> I'm not sure I understand the question. `InlineSmallCode` acts on already compiled methods. This logic analyzes call site frequencies.
> 
>> Also `MinInlineFrequencyRatio ` is 0.0085 which corresponds to `Tier4MinInvocationThreshold` value equal approximately 117. The default value is 600. The only way to change it is to use `CompileThreshold` or `CompileThresholdScaling` which nobody use in production but only in testing. So in production we use `MinInlineFrequencyRatio` as `min_freq`. Why complicate the code.
> 
> Can't exactly tell. But I'm sure I had a reason, probably some corner case exposed by testing. Probably something  didn't get inlined with very low thresholds.
> 
>> 
>> An other issue. This is `should_not_inline()` method. Returning `false` means **inlining** called method. I was curious why we return `false` when we run with `-Xcomp` (see check `(UseInterpreter)` at line 301) ? If we return `false` when `min_invocations()` is 0 it will be similar to `-Xcomp` mode.
>> 
>> Is it intentional to inline when we don't have profiling info?
> 
> I think some tests run with -Xcomp and expect inlining to happen.

So lowering CompileThreshold to below 117, means "compile sooner", but inline less, because should_not_inline will return true more often?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20615#discussion_r1720426670