RFR: 8301997: Move method resolution information out of the cpCache [v2]

[Andrew Dinn]

On Tue, 17 Oct 2023 17:15:56 GMT, Matias Saavedra Silva <matsaave at openjdk.org> wrote:

>> src/hotspot/share/ci/ciReplay.cpp line 436:
>> 
>>> 434: #endif
>>> 435:         ResolvedMethodEntry* method_entry = cp->cache()->resolved_method_entry_at(index);
>>> 436:         cp->cache()->set_method_handle(index, callInfo);
>> 
>> It looks a bit odd that you obtain a pointer to the `ResolvedMethodEntry` at `index` by calling `resolved_method_entry_at` and then pass `index` back into `set_method_handle` in order to update it with the call info. Obviously this is done so that the set routine can handle data races. Would it not be better to modify `set_method_handle` so that it handled the race and also returned the `ResolvedMethodEntry` at `index`.
>> 
>> Likewise you pass `index` back in the call to `appendix_if_resolved` below. Would it not be better to have this method accept a `ResolvedMethodEntry` pointer?
>
> Right, currently these calls seem redundant since it reads the resolved method entry twice. If I understand correctly, you are suggesting that the methods look something like this?
> `ResolvedMethodEntry* set_method_handle(int index, const CallInfo &call_info)`
> `oop appendix_if_resolved(ResolvedMethodEntry* method_entry)`

Yes, removing the redundant lookup is the correct rationale. However, I suggested a better solution below in a comment attached to method `ConstantPoolCache::appendix_if_resolved(int index)`.

Keep the current method of `ConstantPoolCache` that takes an index but modify it to delegate the appendix check to a new method `ResolvedMethodEntry::appendix_if_resolved()`. You can make a direct call to the latter method when you have already looked up the `ResolvedMethodEntry`.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/15455#discussion_r1363642133