Skip to content

cleanups to internal collection-related utils #10080

New issue

Have a question about this project? No Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “No Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? No Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,12 @@
import java.util.function.BiConsumer;
import java.util.function.Consumer;

import static java.lang.System.identityHashCode;
import static org.hibernate.internal.util.collections.CollectionHelper.setOfSize;

/**
* A {@code Map} where keys are compared by object identity,
* rather than {@code equals()}.
* A {@link Map} where keys are compared by object identity,
* rather than using {@link #equals(Object)}.
*/
public final class IdentityMap<K,V> implements Map<K,V> {

Expand Down Expand Up @@ -45,11 +48,10 @@ private IdentityMap(LinkedHashMap<IdentityKey<K>,V> underlyingMap) {

/**
* Return the map entries (as instances of {@code Map.Entry} in a collection that
* is safe from concurrent modification). ie. we may safely add new instances to
* the underlying {@code Map} during iteration of the {@code entries()}.
* is safe from concurrent modification). That is, we may safely add new instances
* to the underlying {@code Map} during iteration of the {@code entries()}.
*
* @param map The map of entries
* @return Collection
*/
public static <K,V> Entry<K,V>[] concurrentEntries(Map<K,V> map) {
return ( (IdentityMap<K,V>) map ).entryArray();
Expand All @@ -61,7 +63,7 @@ public static <K,V> void onEachKey(Map<K,V> map, Consumer<K> consumer) {
}

/**
* Override Map{@link #forEach(BiConsumer)} to provide a more efficient implementation
* Overrides {@link Map#forEach(BiConsumer)} with a more efficient implementation.
* @param action the operation to apply to each element
*/
@Override
Expand All @@ -84,9 +86,8 @@ public boolean isEmpty() {
}

@Override
@SuppressWarnings("unchecked")
public boolean containsKey(Object key) {
return map.containsKey( new IdentityKey( key ) );
return map.containsKey( new IdentityKey<>( key ) );
}

@Override
Expand All @@ -96,9 +97,8 @@ public boolean containsValue(Object val) {
}

@Override
@SuppressWarnings( {"unchecked"})
public V get(Object key) {
return map.get( new IdentityKey( key ) );
return map.get( new IdentityKey<>( key ) );
}

@Override
Expand All @@ -108,15 +108,14 @@ public V put(K key, V value) {
}

@Override
@SuppressWarnings( {"unchecked"})
public V remove(Object key) {
this.entryArray = null;
return map.remove( new IdentityKey( key ) );
return map.remove( new IdentityKey<>( key ) );
}

@Override
public void putAll(Map<? extends K, ? extends V> otherMap) {
for ( Entry<? extends K, ? extends V> entry : otherMap.entrySet() ) {
for ( var entry : otherMap.entrySet() ) {
put( entry.getKey(), entry.getValue() );
}
}
Expand All @@ -141,21 +140,22 @@ public Collection<V> values() {

@Override
public Set<Entry<K,V>> entrySet() {
Set<Entry<K,V>> set = CollectionHelper.setOfSize( map.size() );
for ( Entry<IdentityKey<K>, V> entry : map.entrySet() ) {
final Set<Entry<K,V>> set = setOfSize( map.size() );
for ( var entry : map.entrySet() ) {
set.add( new IdentityMapEntry<>( entry.getKey().key, entry.getValue() ) );
}
return set;
}

public Entry<K,V>[] entryArray() {
if ( entryArray == null ) {
//noinspection unchecked
entryArray = new Entry[ map.size() ];
final Iterator<Entry<IdentityKey<K>, V>> itr = map.entrySet().iterator();
final var iterator = map.entrySet().iterator();
int i = 0;
while ( itr.hasNext() ) {
final Entry<IdentityKey<K>, V> me = itr.next();
entryArray[i++] = new IdentityMapEntry<>( me.getKey().key, me.getValue() );
while ( iterator.hasNext() ) {
final var entry = iterator.next();
entryArray[i++] = new IdentityMapEntry<>( entry.getKey().key, entry.getValue() );
}
}
return entryArray;
Expand All @@ -166,45 +166,33 @@ public String toString() {
return map.toString();
}

private static final class KeyIterator<K> implements Iterator<K> {
private final Iterator<IdentityKey<K>> identityKeyIterator;

private KeyIterator(Iterator<IdentityKey<K>> iterator) {
identityKeyIterator = iterator;
}

private record KeyIterator<K>(Iterator<IdentityKey<K>> identityKeyIterator)
implements Iterator<K> {
@Override
public boolean hasNext() {
return identityKeyIterator.hasNext();
}

@Override
public K next() {
return identityKeyIterator.next().key;
}

@Override
public void remove() {
throw new UnsupportedOperationException();
}

}

private static final class IdentityMapEntry<K,V> implements Entry<K,V> {

private final K key;
private final V value;

IdentityMapEntry(final K key, final V value) {
this.key=key;
this.value=value;
}

private record IdentityMapEntry<K, V>(K key, V value)
implements Entry<K, V> {
@Override
public K getKey() {
return key;
}

@Override
public V getValue() {
return value;
}

@Override
public V setValue(final V value) {
throw new UnsupportedOperationException();
}
Expand All @@ -213,30 +201,21 @@ public V setValue(final V value) {
/**
* We need to base the identity on {@link System#identityHashCode(Object)}
*/
private static final class IdentityKey<K> implements Serializable {

private final K key;

IdentityKey(K key) {
this.key = key;
}

@SuppressWarnings( {"EqualsWhichDoesntCheckParameterClass"})
private record IdentityKey<K>(K key)
implements Serializable {
@Override
public boolean equals(Object other) {
return other != null && key == ( (IdentityKey) other ).key;
return other instanceof IdentityKey<?> that
&& this.key == that.key;
}

@Override
public int hashCode() {
return System.identityHashCode( key );
return identityHashCode( key );
}

@Override
public String toString() {
return key.toString();
}

}

}
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,13 @@
*/
package org.hibernate.internal.util.collections;

import java.util.Collections;
import java.util.HashSet;
import java.util.Set;
import java.util.function.Consumer;
import java.util.function.Supplier;

import static java.util.Collections.emptySet;

/**
* @author Steve Ebersole
*/
Expand Down Expand Up @@ -40,6 +41,6 @@ public void forEach(Consumer<T> action) {
}

public Set<T> getUnderlyingSet() {
return set == null ? Collections.emptySet() : set;
return set == null ? emptySet() : set;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@
* SPDX-License-Identifier: Apache-2.0
* Copyright Red Hat Inc. and Hibernate Authors
*/
package org.hibernate.internal.util.collections;
package org.hibernate.envers.internal.tools;

import java.io.IOException;
import java.io.Serial;
import java.io.Serializable;
import java.lang.ref.Reference;
import java.lang.ref.ReferenceQueue;
Expand Down Expand Up @@ -122,14 +123,15 @@
* @author Doug Lea
* @author Jason T. Greene
*/
public class ConcurrentReferenceHashMap<K, V> extends AbstractMap<K, V>
class ConcurrentReferenceHashMap<K, V> extends AbstractMap<K, V>
implements java.util.concurrent.ConcurrentMap<K, V>, Serializable {
@Serial
private static final long serialVersionUID = 7249069246763182397L;

/*
* The basic strategy is to subdivide the table among Segments,
* each of which itself is a concurrently readable hash table.
*/
* The basic strategy is to subdivide the table among Segments,
* each of which itself is a concurrently readable hash table.
*/

/**
* An option specifying which Java reference type should be used to refer
Expand Down Expand Up @@ -455,42 +457,43 @@ static <K, V> HashEntry<K, V>[] newArray(int i) {
*/
static final class Segment<K, V> extends ReentrantLock implements Serializable {
/*
* Segments maintain a table of entry lists that are ALWAYS
* kept in a consistent state, so can be read without locking.
* Next fields of nodes are immutable (final). All list
* additions are performed at the front of each bin. This
* makes it easy to check changes, and also fast to traverse.
* When nodes would otherwise be changed, new nodes are
* created to replace them. This works well for hash tables
* since the bin lists tend to be short. (The average length
* is less than two for the default load factor threshold.)
*
* Read operations can thus proceed without locking, but rely
* on selected uses of volatiles to ensure that completed
* write operations performed by other threads are
* noticed. For most purposes, the "count" field, tracking the
* number of elements, serves as that volatile variable
* ensuring visibility. This is convenient because this field
* needs to be read in many read operations anyway:
*
* - All (unsynchronized) read operations must first read the
* "count" field, and should not look at table entries if
* it is 0.
*
* - All (synchronized) write operations should write to
* the "count" field after structurally changing any bin.
* The operations must not take any action that could even
* momentarily cause a concurrent read operation to see
* inconsistent data. This is made easier by the nature of
* the read operations in Map. For example, no operation
* can reveal that the table has grown but the threshold
* has not yet been updated, so there are no atomicity
* requirements for this with respect to reads.
*
* As a guide, all critical volatile reads and writes to the
* count field are marked in code comments.
*/
* Segments maintain a table of entry lists that are ALWAYS
* kept in a consistent state, so can be read without locking.
* Next fields of nodes are immutable (final). All list
* additions are performed at the front of each bin. This
* makes it easy to check changes, and also fast to traverse.
* When nodes would otherwise be changed, new nodes are
* created to replace them. This works well for hash tables
* since the bin lists tend to be short. (The average length
* is less than two for the default load factor threshold.)
*
* Read operations can thus proceed without locking, but rely
* on selected uses of volatiles to ensure that completed
* write operations performed by other threads are
* noticed. For most purposes, the "count" field, tracking the
* number of elements, serves as that volatile variable
* ensuring visibility. This is convenient because this field
* needs to be read in many read operations anyway:
*
* - All (unsynchronized) read operations must first read the
* "count" field, and should not look at table entries if
* it is 0.
*
* - All (synchronized) write operations should write to
* the "count" field after structurally changing any bin.
* The operations must not take any action that could even
* momentarily cause a concurrent read operation to see
* inconsistent data. This is made easier by the nature of
* the read operations in Map. For example, no operation
* can reveal that the table has grown but the threshold
* has not yet been updated, so there are no atomicity
* requirements for this with respect to reads.
*
* As a guide, all critical volatile reads and writes to the
* count field are marked in code comments.
*/

@Serial
private static final long serialVersionUID = 2249069246763182397L;

/**
Expand Down Expand Up @@ -952,7 +955,7 @@ public ConcurrentReferenceHashMap(
identityComparisons = options != null && options.contains( Option.IDENTITY_COMPARISONS );

for ( int i = 0; i < this.segments.length; ++i ) {
this.segments[i] = new Segment<K, V>(
this.segments[i] = new Segment<>(
cap, loadFactor,
keyType, valueType, identityComparisons
);
Expand Down Expand Up @@ -1642,6 +1645,7 @@ public V nextElement() {
* This class is needed for JDK5 compatibility.
*/
static class SimpleEntry<K, V> implements Entry<K, V>, Serializable {
@Serial
private static final long serialVersionUID = -8499721149061103585L;

private final K key;
Expand Down Expand Up @@ -1848,6 +1852,7 @@ public void clear() {
* for each key-value mapping, followed by a null pair.
* The key-value mappings are emitted in no particular order.
*/
@Serial
private void writeObject(java.io.ObjectOutputStream s) throws IOException {
s.defaultWriteObject();

Expand Down Expand Up @@ -1883,6 +1888,7 @@ private void writeObject(java.io.ObjectOutputStream s) throws IOException {
*
* @param s the stream
*/
@Serial
@SuppressWarnings("unchecked")
private void readObject(java.io.ObjectInputStream s)
throws IOException, ClassNotFoundException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
import org.hibernate.envers.exception.AuditException;
import org.hibernate.envers.internal.entities.PropertyData;
import org.hibernate.envers.tools.Pair;
import org.hibernate.internal.util.collections.ConcurrentReferenceHashMap;
import org.hibernate.property.access.spi.Getter;
import org.hibernate.property.access.spi.PropertyAccessStrategy;
import org.hibernate.property.access.spi.PropertyAccessStrategyResolver;
Expand Down