Use AtomicInteger instead of unsafe increment on volatile fields

This commit is contained in:
stsypanov
2019-01-18 14:18:41 +02:00
committed by Juergen Hoeller
parent e402a93e41
commit 248d3f8e8b
3 changed files with 30 additions and 26 deletions

View File

@@ -18,6 +18,7 @@ package org.springframework.aop.support;
import java.io.Serializable; import java.io.Serializable;
import java.lang.reflect.Method; import java.lang.reflect.Method;
import java.util.concurrent.atomic.AtomicInteger;
import org.springframework.aop.ClassFilter; import org.springframework.aop.ClassFilter;
import org.springframework.aop.MethodMatcher; import org.springframework.aop.MethodMatcher;
@@ -43,7 +44,7 @@ public class ControlFlowPointcut implements Pointcut, ClassFilter, MethodMatcher
@Nullable @Nullable
private String methodName; private String methodName;
private volatile int evaluations; private final AtomicInteger evaluations = new AtomicInteger(0);
/** /**
@@ -92,7 +93,7 @@ public class ControlFlowPointcut implements Pointcut, ClassFilter, MethodMatcher
@Override @Override
public boolean matches(Method method, Class<?> targetClass, Object... args) { public boolean matches(Method method, Class<?> targetClass, Object... args) {
this.evaluations++; this.evaluations.incrementAndGet();
for (StackTraceElement element : new Throwable().getStackTrace()) { for (StackTraceElement element : new Throwable().getStackTrace()) {
if (element.getClassName().equals(this.clazz.getName()) && if (element.getClassName().equals(this.clazz.getName()) &&
@@ -107,7 +108,7 @@ public class ControlFlowPointcut implements Pointcut, ClassFilter, MethodMatcher
* It's useful to know how many times we've fired, for optimization. * It's useful to know how many times we've fired, for optimization.
*/ */
public int getEvaluations() { public int getEvaluations() {
return this.evaluations; return this.evaluations.get();
} }

View File

@@ -31,6 +31,7 @@ import java.util.NoSuchElementException;
import java.util.Set; import java.util.Set;
import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap; import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.locks.ReentrantLock; import java.util.concurrent.locks.ReentrantLock;
import org.springframework.lang.Nullable; import org.springframework.lang.Nullable;
@@ -476,7 +477,7 @@ public class ConcurrentReferenceHashMap<K, V> extends AbstractMap<K, V> implemen
* The total number of references contained in this segment. This includes chained * The total number of references contained in this segment. This includes chained
* references and references that have been garbage collected but not purged. * references and references that have been garbage collected but not purged.
*/ */
private volatile int count = 0; private final AtomicInteger count = new AtomicInteger(0);
/** /**
* The threshold when resizing of the references should occur. When {@code count} * The threshold when resizing of the references should occur. When {@code count}
@@ -496,7 +497,7 @@ public class ConcurrentReferenceHashMap<K, V> extends AbstractMap<K, V> implemen
if (restructure == Restructure.WHEN_NECESSARY) { if (restructure == Restructure.WHEN_NECESSARY) {
restructureIfNecessary(false); restructureIfNecessary(false);
} }
if (this.count == 0) { if (this.count.get() == 0) {
return null; return null;
} }
// Use a local copy to protect against other threads writing // Use a local copy to protect against other threads writing
@@ -520,7 +521,7 @@ public class ConcurrentReferenceHashMap<K, V> extends AbstractMap<K, V> implemen
if (task.hasOption(TaskOption.RESTRUCTURE_BEFORE)) { if (task.hasOption(TaskOption.RESTRUCTURE_BEFORE)) {
restructureIfNecessary(resize); restructureIfNecessary(resize);
} }
if (task.hasOption(TaskOption.SKIP_IF_EMPTY) && this.count == 0) { if (task.hasOption(TaskOption.SKIP_IF_EMPTY) && this.count.get() == 0) {
return task.execute(null, null, null); return task.execute(null, null, null);
} }
lock(); lock();
@@ -536,7 +537,7 @@ public class ConcurrentReferenceHashMap<K, V> extends AbstractMap<K, V> implemen
Entry<K, V> newEntry = new Entry<>((K) key, value); Entry<K, V> newEntry = new Entry<>((K) key, value);
Reference<K, V> newReference = Segment.this.referenceManager.createReference(newEntry, hash, head); Reference<K, V> newReference = Segment.this.referenceManager.createReference(newEntry, hash, head);
Segment.this.references[index] = newReference; Segment.this.references[index] = newReference;
Segment.this.count++; Segment.this.count.incrementAndGet();
} }
}; };
return task.execute(ref, entry, entries); return task.execute(ref, entry, entries);
@@ -553,14 +554,14 @@ public class ConcurrentReferenceHashMap<K, V> extends AbstractMap<K, V> implemen
* Clear all items from this segment. * Clear all items from this segment.
*/ */
public void clear() { public void clear() {
if (this.count == 0) { if (this.count.get() == 0) {
return; return;
} }
lock(); lock();
try { try {
this.references = createReferenceArray(this.initialSize); this.references = createReferenceArray(this.initialSize);
this.resizeThreshold = (int) (this.references.length * getLoadFactor()); this.resizeThreshold = (int) (this.references.length * getLoadFactor());
this.count = 0; this.count.set(0);
} }
finally { finally {
unlock(); unlock();
@@ -574,12 +575,12 @@ public class ConcurrentReferenceHashMap<K, V> extends AbstractMap<K, V> implemen
* @param allowResize if resizing is permitted * @param allowResize if resizing is permitted
*/ */
protected final void restructureIfNecessary(boolean allowResize) { protected final void restructureIfNecessary(boolean allowResize) {
boolean needsResize = (this.count > 0 && this.count >= this.resizeThreshold); boolean needsResize = (this.count.get() > 0 && this.count.get() >= this.resizeThreshold);
Reference<K, V> ref = this.referenceManager.pollForPurge(); Reference<K, V> ref = this.referenceManager.pollForPurge();
if (ref != null || (needsResize && allowResize)) { if (ref != null || (needsResize && allowResize)) {
lock(); lock();
try { try {
int countAfterRestructure = this.count; int countAfterRestructure = this.count.get();
Set<Reference<K, V>> toPurge = Collections.emptySet(); Set<Reference<K, V>> toPurge = Collections.emptySet();
if (ref != null) { if (ref != null) {
toPurge = new HashSet<>(); toPurge = new HashSet<>();
@@ -628,7 +629,7 @@ public class ConcurrentReferenceHashMap<K, V> extends AbstractMap<K, V> implemen
this.references = restructured; this.references = restructured;
this.resizeThreshold = (int) (this.references.length * getLoadFactor()); this.resizeThreshold = (int) (this.references.length * getLoadFactor());
} }
this.count = Math.max(countAfterRestructure, 0); this.count.set(Math.max(countAfterRestructure, 0));
} }
finally { finally {
unlock(); unlock();
@@ -674,7 +675,7 @@ public class ConcurrentReferenceHashMap<K, V> extends AbstractMap<K, V> implemen
* Return the total number of references in this segment. * Return the total number of references in this segment.
*/ */
public final int getCount() { public final int getCount() {
return this.count; return this.count.get();
} }
} }

View File

@@ -34,6 +34,8 @@ import org.springframework.expression.spel.support.StandardEvaluationContext;
import org.springframework.lang.Nullable; import org.springframework.lang.Nullable;
import org.springframework.util.Assert; import org.springframework.util.Assert;
import java.util.concurrent.atomic.AtomicInteger;
/** /**
* A {@code SpelExpression} represents a parsed (valid) expression that is ready to be * A {@code SpelExpression} represents a parsed (valid) expression that is ready to be
* evaluated in a specified context. An expression can be evaluated standalone or in a * evaluated in a specified context. An expression can be evaluated standalone or in a
@@ -69,7 +71,7 @@ public class SpelExpression implements Expression {
// Count of many times as the expression been interpreted - can trigger compilation // Count of many times as the expression been interpreted - can trigger compilation
// when certain limit reached // when certain limit reached
private volatile int interpretedCount = 0; private final AtomicInteger interpretedCount = new AtomicInteger(0);
// The number of times compilation was attempted and failed - enables us to eventually // The number of times compilation was attempted and failed - enables us to eventually
// give up trying to compile it when it just doesn't seem to be possible. // give up trying to compile it when it just doesn't seem to be possible.
@@ -124,7 +126,7 @@ public class SpelExpression implements Expression {
catch (Throwable ex) { catch (Throwable ex) {
// If running in mixed mode, revert to interpreted // If running in mixed mode, revert to interpreted
if (this.configuration.getCompilerMode() == SpelCompilerMode.MIXED) { if (this.configuration.getCompilerMode() == SpelCompilerMode.MIXED) {
this.interpretedCount = 0; this.interpretedCount.set(0);
this.compiledAst = null; this.compiledAst = null;
} }
else { else {
@@ -159,7 +161,7 @@ public class SpelExpression implements Expression {
catch (Throwable ex) { catch (Throwable ex) {
// If running in mixed mode, revert to interpreted // If running in mixed mode, revert to interpreted
if (this.configuration.getCompilerMode() == SpelCompilerMode.MIXED) { if (this.configuration.getCompilerMode() == SpelCompilerMode.MIXED) {
this.interpretedCount = 0; this.interpretedCount.set(0);
this.compiledAst = null; this.compiledAst = null;
} }
else { else {
@@ -186,7 +188,7 @@ public class SpelExpression implements Expression {
catch (Throwable ex) { catch (Throwable ex) {
// If running in mixed mode, revert to interpreted // If running in mixed mode, revert to interpreted
if (this.configuration.getCompilerMode() == SpelCompilerMode.MIXED) { if (this.configuration.getCompilerMode() == SpelCompilerMode.MIXED) {
this.interpretedCount = 0; this.interpretedCount.set(0);
this.compiledAst = null; this.compiledAst = null;
} }
else { else {
@@ -221,7 +223,7 @@ public class SpelExpression implements Expression {
catch (Throwable ex) { catch (Throwable ex) {
// If running in mixed mode, revert to interpreted // If running in mixed mode, revert to interpreted
if (this.configuration.getCompilerMode() == SpelCompilerMode.MIXED) { if (this.configuration.getCompilerMode() == SpelCompilerMode.MIXED) {
this.interpretedCount = 0; this.interpretedCount.set(0);
this.compiledAst = null; this.compiledAst = null;
} }
else { else {
@@ -251,7 +253,7 @@ public class SpelExpression implements Expression {
catch (Throwable ex) { catch (Throwable ex) {
// If running in mixed mode, revert to interpreted // If running in mixed mode, revert to interpreted
if (this.configuration.getCompilerMode() == SpelCompilerMode.MIXED) { if (this.configuration.getCompilerMode() == SpelCompilerMode.MIXED) {
this.interpretedCount = 0; this.interpretedCount.set(0);
this.compiledAst = null; this.compiledAst = null;
} }
else { else {
@@ -286,7 +288,7 @@ public class SpelExpression implements Expression {
catch (Throwable ex) { catch (Throwable ex) {
// If running in mixed mode, revert to interpreted // If running in mixed mode, revert to interpreted
if (this.configuration.getCompilerMode() == SpelCompilerMode.MIXED) { if (this.configuration.getCompilerMode() == SpelCompilerMode.MIXED) {
this.interpretedCount = 0; this.interpretedCount.set(0);
this.compiledAst = null; this.compiledAst = null;
} }
else { else {
@@ -314,7 +316,7 @@ public class SpelExpression implements Expression {
catch (Throwable ex) { catch (Throwable ex) {
// If running in mixed mode, revert to interpreted // If running in mixed mode, revert to interpreted
if (this.configuration.getCompilerMode() == SpelCompilerMode.MIXED) { if (this.configuration.getCompilerMode() == SpelCompilerMode.MIXED) {
this.interpretedCount = 0; this.interpretedCount.set(0);
this.compiledAst = null; this.compiledAst = null;
} }
else { else {
@@ -351,7 +353,7 @@ public class SpelExpression implements Expression {
catch (Throwable ex) { catch (Throwable ex) {
// If running in mixed mode, revert to interpreted // If running in mixed mode, revert to interpreted
if (this.configuration.getCompilerMode() == SpelCompilerMode.MIXED) { if (this.configuration.getCompilerMode() == SpelCompilerMode.MIXED) {
this.interpretedCount = 0; this.interpretedCount.set(0);
this.compiledAst = null; this.compiledAst = null;
} }
else { else {
@@ -473,17 +475,17 @@ public class SpelExpression implements Expression {
* @param expressionState the expression state used to determine compilation mode * @param expressionState the expression state used to determine compilation mode
*/ */
private void checkCompile(ExpressionState expressionState) { private void checkCompile(ExpressionState expressionState) {
this.interpretedCount++; this.interpretedCount.incrementAndGet();
SpelCompilerMode compilerMode = expressionState.getConfiguration().getCompilerMode(); SpelCompilerMode compilerMode = expressionState.getConfiguration().getCompilerMode();
if (compilerMode != SpelCompilerMode.OFF) { if (compilerMode != SpelCompilerMode.OFF) {
if (compilerMode == SpelCompilerMode.IMMEDIATE) { if (compilerMode == SpelCompilerMode.IMMEDIATE) {
if (this.interpretedCount > 1) { if (this.interpretedCount.get() > 1) {
compileExpression(); compileExpression();
} }
} }
else { else {
// compilerMode = SpelCompilerMode.MIXED // compilerMode = SpelCompilerMode.MIXED
if (this.interpretedCount > INTERPRETED_COUNT_THRESHOLD) { if (this.interpretedCount.get() > INTERPRETED_COUNT_THRESHOLD) {
compileExpression(); compileExpression();
} }
} }
@@ -524,7 +526,7 @@ public class SpelExpression implements Expression {
*/ */
public void revertToInterpreted() { public void revertToInterpreted() {
this.compiledAst = null; this.compiledAst = null;
this.interpretedCount = 0; this.interpretedCount.set(0);
this.failedAttempts = 0; this.failedAttempts = 0;
} }