RFR: 8301996: Move field resolution information out of the cpCache [v2]

[Matias Saavedra Silva]

On Fri, 30 Jun 2023 14:14:53 GMT, Frederic Parain <fparain at openjdk.org> wrote:

>> Matias Saavedra Silva has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Coleen and Amit comments
>
> src/hotspot/share/interpreter/bytecodeTracer.cpp line 627:
> 
>> 625:     case Bytecodes::_putfield:
>> 626:     case Bytecodes::_getfield:
>> 627:       // TODO: get_index_u2() does not work here due to using java_u2 instead of native_u2
> 
> Is the TODO comment still relevant?

This was removed after the merge

> src/hotspot/share/interpreter/interpreterRuntime.cpp line 719:
> 
>> 717:   ResolvedFieldEntry* entry = pool->resolved_field_entry_at(field_index);
>> 718:   entry->fill_in(info.field_holder(), info.offset(), (u2)info.index(), (u1)state, (u1)get_code, (u1)put_code);
>> 719:   entry->set_flags(info.access_flags().is_final(), info.access_flags().is_volatile());
> 
> I think there's an issue here: fill_in() will mark the field entry as resolved (by setting get_code or put_code), and by consequence usable by other threads *before* the flags are initialized. This means another thread could see the entry as being resolved but still get the uninitialized flags.

Great catch! Switching these calls should be sufficient, correct?

> src/hotspot/share/oops/resolvedFieldEntry.hpp line 95:
> 
>> 93:       return (put_code() == code);
>> 94:     default:
>> 95:       assert(code == Bytecodes::_nop, "Must be get, put, or nop bytecode");
> 
> In which cases the is_resolved() method is called with the _nop bytecode in argument?

After some testing, this does not seem necessary, the bytecode can only be get or put based on the callers. The default case will be replaced with a ShouldNotReachHere() instead.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/14129#discussion_r1253529383
PR Review Comment: https://git.openjdk.org/jdk/pull/14129#discussion_r1253529663
PR Review Comment: https://git.openjdk.org/jdk/pull/14129#discussion_r1253529233