RFR: JDK-8310913 Move ReferencedKeyMap to jdk.internal so it may be shared [v10]

[Jim Laskey]

On Mon, 5 Feb 2024 16:38:59 GMT, Roger Riggs <rriggs at openjdk.org> wrote:

>> Good catch. Your solution might be correct but I think `!contains(e)` is redundant since that is how intern starts out.
>> 
>> 
>>    static <T> T intern(ReferencedKeyMap<T, ReferenceKey<T>> setMap, T key, UnaryOperator<T> interner) {
>>         T value = existingKey(setMap, key);
>>         if (value != null) {
>>             return value;
>>         }
>>         key = interner.apply(key);
>>         return internKey(setMap, key);
>>     }
>> 
>> 
>> Agree? So changing to `return intern(e) == e;` should be sufficient.
>> 
>> The other aspect of this is that `internKey` uses `putIfAbsent` which should prevent the race (assuming `ConcurrentHashMap`).
>> 
>> 
>>     /**
>>      * Attempt to add key to map.
>>      *
>>      * @param setMap    {@link ReferencedKeyMap} where interning takes place
>>      * @param key       key to add
>>      *
>>      * @param <T> type of key
>>      *
>>      * @return the old key instance if found otherwise the new key instance
>>      */
>>     private static <T> T internKey(ReferencedKeyMap<T, ReferenceKey<T>> setMap, T key) {
>>         ReferenceKey<T> entryKey = setMap.entryKey(key);
>>         T interned;
>>         do {
>>             setMap.removeStaleReferences();
>>             ReferenceKey<T> existing = setMap.map.putIfAbsent(entryKey, entryKey);
>>             if (existing == null) {
>>                 return key;
>>             } else {
>>                 // If {@code putIfAbsent} returns non-null then was actually a
>>                 // {@code replace} and older key was used. In that case the new
>>                 // key was not used and the reference marked stale.
>>                 interned = existing.get();
>>                 if (interned != null) {
>>                     entryKey.unused();
>>                 }
>>             }
>>         } while (interned == null);
>>         return interned;
>>     }
>> 
>> 
>> Agree? Anyone else want to pipe in?
>
> ok, `intern(e) == e` is sufficient.

Created https://bugs.openjdk.org/browse/JDK-8325255

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/14684#discussion_r1478583967