RFR: 8339299: C1 will miss type profile when inline final method [v3]

[Dean Long]

On Wed, 4 Sep 2024 16:59:50 GMT, Vladimir Ivanov <vlivanov at openjdk.org> wrote:

>> I was just trying to think how to preserve the original intend of this code, which seemed to be "skip profiling if the profiling info is not needed", but getting it right seems complicated, so I'm OK with always doing it.
>> 
>>> C1 should not optimize call sites in Level 3 compilation.
>> 
>> You mean don't use can_be_statically_bound() and other checks to devirtualize virtual calls?  I believe C1 does that at all compilation levels.
>
> It does look attractive to align the logic with C2 usage of type profiles (avoid profiling when C2 doesn't consume the data). But I feel more comfortable unifying different modes of profiling at the expense of some micro-optimization opportunities. In other words, if interpreter collects some bit of data, I'd prefer to see C1 doing the same (and vice versa). 
> 
> I took a look at interpreter code (in `TemplateTable::invokevirtual_helper()`) and it makes the decision at runtime based on `is_vfinal` flag on `ResolvedMethodEntry`. The flag is set in `ConstantPoolCache::set_direct_or_vtable_call()` and covers both private and final methods. Moreover, receiver profiling is not performed on `invokeinterface` of private methods which is not taken into account by `should_profile_receiver_type()` now.
> 
> It looks tempting to replicated what interpreter does (inspect `vfinal` flag on resolved method), but C1 has to gracefully work with not-yet-resolved call sites. So, either a recompilation or a runtime check is needed to align the behavior with interpreter.
> 
> I haven't looked into the details, but performing profiling in C1 when the rest of the JVM doesn't expect that makes me a bit nervous. Smells like a possible source of profile data corruption.

What profiling can be done seems to be decided by MethodData::compute_data_size()/MethodData::initialize_data(), which uses profile_arguments_for_invoke() and profile_return_for_invoke().  At runtime, I believe profiling is restricted by what it finds in the MethodData.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20786#discussion_r1744750197