Skip to content

Commit 6ec2642

Browse files
committed
Ensure consistent value count in ConcurrentReferenceHashMap#Segment
Prior to this commit, the `ConcurrentReferenceHashMap#Segment` would recalculate its element count during the restructure phase by subtracting the number of polled elements from the queue to the current count. Later, the newly restructured `references` array would only contain references that 1) are not in the set of elements to purge and 2) that hold a non-null value. The issues with this approach are multiple. The newly calculated count is only an estimate, as some situations can make it invalid, even temporarily. * since we initially collected all elements to be purged from the queue, the GC might have collected new values. This means that we might filter out more references that we initially intended to * because the restructure operation re-creates new references for all elements in the original array, we might later get references from the queue that are not in the array anymore. This could lead to "duplicate" removals for the same value Because several methods in the Segment class have special no-op behavior when `count == 0`, an invalid count can lead to keys appearing missing when they are actually still present. In some scenarios, this can decrease the performance of the cache since values need to be recalculated. This commit fixes this inconsistency count issue by first using an estimate in order to decide whether the array needs a resize and then by counting the actual numbers of elements inserted in the restructured array as the new count. Fixes gh-31373
1 parent 187b4e5 commit 6ec2642

File tree

1 file changed

+14
-9
lines changed

1 file changed

+14
-9
lines changed

spring-core/src/main/java/org/springframework/util/ConcurrentReferenceHashMap.java

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@
5656
*
5757
* @author Phillip Webb
5858
* @author Juergen Hoeller
59+
* @author Brian Clozel
5960
* @since 3.2
6061
* @param <K> the key type
6162
* @param <V> the value type
@@ -568,7 +569,7 @@ public void clear() {
568569
* references that have been garbage collected.
569570
* @param allowResize if resizing is permitted
570571
*/
571-
protected final void restructureIfNecessary(boolean allowResize) {
572+
void restructureIfNecessary(boolean allowResize) {
572573
int currCount = this.count.get();
573574
boolean needsResize = allowResize && (currCount > 0 && currCount >= this.resizeThreshold);
574575
Reference<K, V> ref = this.referenceManager.pollForPurge();
@@ -581,7 +582,7 @@ private void restructure(boolean allowResize, @Nullable Reference<K, V> ref) {
581582
boolean needsResize;
582583
lock();
583584
try {
584-
int countAfterRestructure = this.count.get();
585+
int expectedCount = this.count.get();
585586
Set<Reference<K, V>> toPurge = Collections.emptySet();
586587
if (ref != null) {
587588
toPurge = new HashSet<>();
@@ -590,11 +591,11 @@ private void restructure(boolean allowResize, @Nullable Reference<K, V> ref) {
590591
ref = this.referenceManager.pollForPurge();
591592
}
592593
}
593-
countAfterRestructure -= toPurge.size();
594+
expectedCount -= toPurge.size();
594595

595-
// Recalculate taking into account count inside lock and items that
596-
// will be purged
597-
needsResize = (countAfterRestructure > 0 && countAfterRestructure >= this.resizeThreshold);
596+
// Estimate new count, taking into account count inside lock and items that
597+
// will be purged.
598+
needsResize = (expectedCount > 0 && expectedCount >= this.resizeThreshold);
598599
boolean resizing = false;
599600
int restructureSize = this.references.length;
600601
if (allowResize && needsResize && restructureSize < MAXIMUM_SEGMENT_SIZE) {
@@ -607,6 +608,7 @@ private void restructure(boolean allowResize, @Nullable Reference<K, V> ref) {
607608
(resizing ? createReferenceArray(restructureSize) : this.references);
608609

609610
// Restructure
611+
int newCount = 0;
610612
for (int i = 0; i < this.references.length; i++) {
611613
ref = this.references[i];
612614
if (!resizing) {
@@ -615,10 +617,13 @@ private void restructure(boolean allowResize, @Nullable Reference<K, V> ref) {
615617
while (ref != null) {
616618
if (!toPurge.contains(ref)) {
617619
Entry<K, V> entry = ref.get();
620+
// Also filter out null references that are now null
621+
// they should be polled the queue in a later restructure call.
618622
if (entry != null) {
619623
int index = getIndex(ref.getHash(), restructured);
620624
restructured[index] = this.referenceManager.createReference(
621625
entry, ref.getHash(), restructured[index]);
626+
newCount++;
622627
}
623628
}
624629
ref = ref.getNext();
@@ -630,7 +635,7 @@ private void restructure(boolean allowResize, @Nullable Reference<K, V> ref) {
630635
this.references = restructured;
631636
this.resizeThreshold = (int) (this.references.length * getLoadFactor());
632637
}
633-
this.count.set(Math.max(countAfterRestructure, 0));
638+
this.count.set(Math.max(newCount, 0));
634639
}
635640
finally {
636641
unlock();
@@ -667,14 +672,14 @@ private int getIndex(int hash, Reference<K, V>[] references) {
667672
/**
668673
* Return the size of the current references array.
669674
*/
670-
public final int getSize() {
675+
public int getSize() {
671676
return this.references.length;
672677
}
673678

674679
/**
675680
* Return the total number of references in this segment.
676681
*/
677-
public final int getCount() {
682+
public int getCount() {
678683
return this.count.get();
679684
}
680685
}

0 commit comments

Comments
 (0)