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

[Igor Veresov]

On Fri, 16 Aug 2024 22:45:58 GMT, Dean Long <dlong 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.
>>> 
>> 
>> 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?

Yes, the problem is that the tests still expect you to inline things even with small or 0 thresholds. Hence the `1/min_invocations()` floor.

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

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