On Tue, 18 Jul 2023 14:56:53 GMT, Chen Liang <li...@openjdk.org> wrote:
>> 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) Changing. > 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. Changing. > 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`. This was a bit tricky. Lambdas can't be used in phase1 so messed when MethodType kicked in. So I created two paths, one for normal and one for the UnaryOperator. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/14684#discussion_r1268104420 PR Review Comment: https://git.openjdk.org/jdk/pull/14684#discussion_r1268104006 PR Review Comment: https://git.openjdk.org/jdk/pull/14684#discussion_r1268108150