On Thu, 13 Jul 2023 15:07:35 GMT, Jim Laskey <jlas...@openjdk.org> wrote:
>> java.lang.runtime.ReferencedKeyMap was introduced to provide a concurrent
>> caching scheme for Carrier objects. The technique used is generally useful
>> for a variety of caching schemes and is being moved to be shared in other
>> parts of the jdk. The MethodType interning case is one example.
>
> Jim Laskey has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Update test to check for gc.
src/java.base/share/classes/jdk/internal/util/ReferencedKeyMap.java line 354:
> 352: */
> 353: @SuppressWarnings("unchecked")
> 354: K intern(K key) {
Should this be made static for type safety?
public static <E> E intern(ReferencedKeyMap<E, ReferenceKey<E>> m, E key)
which will save the 2 unchecked casts at `get` and `putIfAbsent`.
In addition, it would be nice if we can compute a more proper key for interning
than the query key, via a binary operator; for example, we want a `String` to
go through `String::intern`, or a `BaseLocale` to format its language, script,
region, and variant and intern them (see
https://github.com/openjdk/jdk/pull/14404/files#diff-c0e20847ffb6e33bcf62ff52b1b5b4c8a85520ca101a6f54128d97289cd8c2f9R169-R172)
src/java.base/share/classes/jdk/internal/util/ReferencedKeySet.java line 71:
> 69: * @param <T> the type of elements maintained by this set
> 70: */
> 71: public class ReferencedKeySet<T> extends AbstractSet<T> {
Suggestion:
public final class ReferencedKeySet<T> extends AbstractSet<T> {
Make this class final.
src/java.base/share/classes/jdk/internal/util/ReferencedKeySet.java line 97:
> 95: * @param <E> the type of elements maintained by this set
> 96: */
> 97: public static <E> jdk.internal.util.ReferencedKeySet<E>
Suggestion:
public static <E> ReferencedKeySet<E>
src/java.base/share/classes/jdk/internal/util/ReferencedKeySet.java line 185:
> 183: * @return the old element if present, otherwise, the new element
> 184: */
> 185: public T intern(T e) {
I would like to see an additional `T intern(T e, UnaryOperator<T>
internFunction)`, where the `internFunction` behaves like
`LocalObjectCache::normalizeKey`.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/14684#discussion_r1266902706
PR Review Comment: https://git.openjdk.org/jdk/pull/14684#discussion_r1266911305
PR Review Comment: https://git.openjdk.org/jdk/pull/14684#discussion_r1266897522
PR Review Comment: https://git.openjdk.org/jdk/pull/14684#discussion_r1266914091
