On Tue, 29 Apr 2025 19:06:08 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 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 14 additional commits since the > last revision: > > - Merge branch 'master' into 8354897-soft-weak-references-in-aot-cache > - @fisk offline comments -- tighten up and simplify eligibility check; > @DanHeidinga comment -- renamed to MethodType::assemblySetup() > - @DanHeidinga comments > - @fisk comment > - Fixed windows path issues > - Refactored code > - Fixed error when running "make test JTREG=AOT_JDK=true > TEST=open/test/hotspot/jtreg/runtime/invokedynamic" > - Work around hotspot/jtreg/sources/TestNoNULL.java > - Merge branch 'master' into 8354897-soft-weak-references-in-aot-cache > - cleaned up test cases > - ... and 4 more: https://git.openjdk.org/jdk/compare/09137395...70216566
Other than reading the referent with the wrong accessor in the assert, I think this looks very good. Thanks for sorting this out in a clean and robust way. ------------- Marked as reviewed by eosterlund (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/24757#pullrequestreview-2804941687
