On Wed, 30 Apr 2025 17:45:04 GMT, Ioi Lam <ik...@openjdk.org> wrote:
>> This PR contains 2 parts
>>
>> - Upstream of Soft/Weak Reference support authored by @macarte from [the
>> Leyden
>> repo](https://github.com/openjdk/leyden/commit/4ca75d156519596e23abc8a312496b7c2f0e0ca5)
>> - New C++ class `AOTReferenceObjSupport` and new Java method
>> `ReferencedKeyMap::prepareForAOTCache()` developed by @iklam on the advice
>> of @fisk from the GC team. These control the lifecycles of reference objects
>> during the assembly phase to simplify the implementation.
>>
>> One problem we faced in this PR is the handling of Reference objects that
>> are waiting for clean up. Currently, the only cached Reference objects that
>> require clean up are the `WeakReferenceKey`s used by `ReferencedKeyMap`
>> (which is used by `MethodType::internTable`):
>>
>> - When the referent of a `WeakReferenceKey` K has been collected, the key
>> will be placed on `Universe::reference_pending_list()`. It's linked to other
>> pending references with the `Reference::discovered` field. At this point, K
>> is still stored in the `ReferencedKeyMap`.
>> - When heapShared.cpp discovered the `ReferencedKeyMap`, it will discover K,
>> and it may also discover other pending references that are not intended for
>> the AOT cache. As a result, we end up caching unnecessary objects.
>>
>> `ReferencedKeyMap::prepareForAOTCache()` avoids the above problem. It goes
>> over all entries in the table:
>>
>> - If an entry has not yet been collected, we make sure it will never be
>> collected.
>> - If an entry has been collected, we remove it from the table
>>
>> Therefore, by the time heapShared.cpp starts scanning the
>> `ReferencedKeyMap`, it will never see any keys that are on the pending list,
>> so we will not see unintended objects.
>>
>> This implementation is the very first step of Reference support in the AOT
>> cache, so we chose a simplified approach that makes no assumptions on when
>> the pending reference list is processed. This is sufficient for the current
>> set of references objects in the AOT cache.
>>
>> In the future, we may relax the implementation to allow for other use cases.
>
> Ioi Lam has updated the pull request incrementally with two additional
> commits since the last revision:
>
> - @AlanBateman comments
> - @xmas92 comments
Changes requested by exe-b...@github.com (no known OpenJDK username).
src/java.base/share/classes/java/lang/ref/Reference.java line 314:
> 312: }
> 313:
> 314: private static void runtimeSetup() {
The comment was added in the wrong spot:
Suggestion:
static {
runtimeSetup();
}
// Called from JVM when loading an AOT cache
private static void runtimeSetup() {
-------------
PR Review: https://git.openjdk.org/jdk/pull/24757#pullrequestreview-2809022545
PR Review Comment: https://git.openjdk.org/jdk/pull/24757#discussion_r2069766184
