On Thu, 23 May 2024 20:50:47 GMT, Matias Saavedra Silva <matsa...@openjdk.org>
wrote:
>> Ioi Lam has updated the pull request with a new target base due to a merge
>> or a rebase. The incremental webrev excludes the unrelated changes brought
>> in by the merge/rebase. The pull request contains two additional commits
>> since the last revision:
>>
>> - Merge branch 'master' into 8293980-resolve-fields-at-dumptime
>> - 8293980: Resolve CONSTANT_FieldRef at CDS dump time
>
> src/hotspot/share/oops/constantPool.cpp line 464:
>
>> 462: if (cache() != nullptr) {
>> 463: // cache() is null if this class is not yet linked.
>> 464: remove_resolved_field_entries_if_non_deterministic();
>
> These methods look like they can belong to the constant pool cache instead.
> Can cpCache call the ClassLinker code instead so this can be part of
> `cache()->remove_unshareable_info()`?
I moved remove_resolved_field_entries_if_non_deterministic() to cpCache as you
suggested. I removed the functions for indy and method, as dumptime resolution
for those types of entries is not yet implemented.
> src/hotspot/share/oops/constantPool.cpp line 520:
>
>> 518: int cp_index = rfi->constant_pool_index();
>> 519: bool archived = false;
>> 520: bool resolved = rfi->is_resolved(Bytecodes::_putfield) ||
>
> Is one of these meant to be `is_resolved(Bytecodes::get_field)` ?
Fixed.
> src/hotspot/share/oops/resolvedFieldEntry.hpp line 65:
>
>> 63: _tos_state = other._tos_state;
>> 64: _flags = other._flags;
>> 65: _get_code = other._get_code;
>
> The fields `_get_code` and `_put_code` are normally set atomically, does this
> need to be the case when copying as well?
This is done inside while the ResolvedFieldEntries are being prepared during
class rewriting. All access is single threaded so there's no need for atomic
operations.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/19355#discussion_r1614389410
PR Review Comment: https://git.openjdk.org/jdk/pull/19355#discussion_r1614389465
PR Review Comment: https://git.openjdk.org/jdk/pull/19355#discussion_r1614390838
