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

[Vladimir Ivanov]

On Thu, 5 Sep 2024 03:23:16 GMT, Dean Long <dlong at openjdk.org> wrote:

>> 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.

Thanks for additional details, Dean. Thinking more about it, it looks like the worst case scenario possible is that there'll be more data gathered than needed, but there'll be always a slot reserved for the data.
IMO current fix seems like a good compromise.

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

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