On Fri, 25 Apr 2025 19:39:01 GMT, Dan Heidinga <heidi...@openjdk.org> wrote:
>> Ioi Lam has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> @fisk comment
>
> src/hotspot/share/cds/aotReferenceObjSupport.cpp line 106:
>
>> 104: assert(CDSConfig::allow_only_single_java_thread(), "Required");
>> 105:
>> 106: TempNewSymbol method_name =
>> SymbolTable::new_symbol("prepareForAOTCache");
>
> I'm slightly uncomfortable with using the same method name
> (`prepareForAOTCache`) on MethodType and on ReferenceKeyMap & ReferenceKeySet
> as they have different expected use cases. The one on MT is the "front door"
> the VM calls to remove stale Reference objects while the RKMap/RKSet are
> internal mechanisms that the VM would never call except for MT triggering it.
>
> Does it make sense to use different names for these methods? The MT one is a
> hook that could be extended to other classes to prepare them for cache
> creation while we wouldn't treat the RKMap/RKSet ones in the same way. Maybe
> append "_internal" or "_helper" to the RFMap/Set methods to distinguish them?
In my mind, `MethodType::prepareForAOTCache()` makes sure all data used by
`MethodType` are ready to be cached. `ReferenceKeySet::prepareForAOTCache()`
does the same for this particular `ReferenceKeySet` instance.
Potentially we could have
class MethodType {
static void prepareForAOTCache() {
table1.prepareForAOTCache();
table2.prepareForAOTCache();
....
}
}
We can have many levels of `prepareForAOTCache()` calls where each level
delegates the operations to its sub-components. There are no obvious
"frontend/backend" or "interface/implementation" boundaries.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/24757#discussion_r2062910787
