From 2f02bfd0cd2655afe55ea245812b88f705ec7977 Mon Sep 17 00:00:00 2001 From: Yanming Zhou Date: Tue, 24 Sep 2024 09:16:04 +0800 Subject: [PATCH] Use volatile field instead of AtomicReference it's not necessary to use `AtomicReference` here since the result of atomic method `updateAndGet()` is unused, and not necessary to use `AtomicReferenceFieldUpdater` here since only `set()` is using, volatile field access is more appropriate. --- .../java/com/zaxxer/hikari/HikariConfig.java | 16 +++++++++------- .../java/com/zaxxer/hikari/pool/HikariPool.java | 7 ++----- .../java/com/zaxxer/hikari/pool/PoolBase.java | 12 +++++------- 3 files changed, 16 insertions(+), 19 deletions(-) diff --git a/src/main/java/com/zaxxer/hikari/HikariConfig.java b/src/main/java/com/zaxxer/hikari/HikariConfig.java index f0cc081c..25d5f800 100644 --- a/src/main/java/com/zaxxer/hikari/HikariConfig.java +++ b/src/main/java/com/zaxxer/hikari/HikariConfig.java @@ -69,7 +69,7 @@ public class HikariConfig implements HikariConfigMXBean private volatile long maxLifetime; private volatile int maxPoolSize; private volatile int minIdle; - private final AtomicReference credentials = new AtomicReference<>(Credentials.of(null, null)); + private volatile Credentials credentials = Credentials.of(null, null); // Properties NOT changeable at runtime // @@ -284,7 +284,7 @@ public class HikariConfig implements HikariConfigMXBean */ public String getPassword() { - return credentials.get().getPassword(); + return this.credentials.getPassword(); } /** @@ -294,7 +294,8 @@ public class HikariConfig implements HikariConfigMXBean @Override public void setPassword(String password) { - credentials.updateAndGet(current -> Credentials.of(current.getUsername(), password)); + Credentials current = this.credentials; + this.credentials = Credentials.of(current.getUsername(), password); } /** @@ -304,7 +305,7 @@ public class HikariConfig implements HikariConfigMXBean */ public String getUsername() { - return credentials.get().getUsername(); + return this.credentials.getUsername(); } /** @@ -315,7 +316,8 @@ public class HikariConfig implements HikariConfigMXBean @Override public void setUsername(String username) { - credentials.updateAndGet(current -> Credentials.of(username, current.getPassword())); + Credentials current = this.credentials; + this.credentials = Credentials.of(username, current.getPassword()); } /** @@ -326,7 +328,7 @@ public class HikariConfig implements HikariConfigMXBean @Override public void setCredentials(final Credentials credentials) { - this.credentials.set(credentials); + this.credentials = credentials; } /** @@ -336,7 +338,7 @@ public class HikariConfig implements HikariConfigMXBean */ public Credentials getCredentials() { - return credentials.get(); + return this.credentials; } /** {@inheritDoc} */ diff --git a/src/main/java/com/zaxxer/hikari/pool/HikariPool.java b/src/main/java/com/zaxxer/hikari/pool/HikariPool.java index ae97e403..ab202381 100644 --- a/src/main/java/com/zaxxer/hikari/pool/HikariPool.java +++ b/src/main/java/com/zaxxer/hikari/pool/HikariPool.java @@ -37,7 +37,6 @@ import java.sql.SQLException; import java.sql.SQLTransientConnectionException; import java.util.Optional; import java.util.concurrent.*; -import java.util.concurrent.atomic.AtomicReferenceFieldUpdater; import static com.zaxxer.hikari.util.ClockSource.*; import static com.zaxxer.hikari.util.ConcurrentBag.IConcurrentBagEntry.STATE_IN_USE; @@ -499,7 +498,7 @@ public final class HikariPool extends PoolBase implements HikariPoolMXBean, IBag catch (ConnectionSetupException e) { if (poolState == POOL_NORMAL) { // we check POOL_NORMAL to avoid a flood of messages if shutdown() is running concurrently logger.error("{} - Error thrown while acquiring connection from data source", poolName, e.getCause()); - lastConnectionFailure.set(e); + lastConnectionFailure = e; } } catch (Exception e) { @@ -785,8 +784,6 @@ public final class HikariPool extends PoolBase implements HikariPoolMXBean, IBag private final class HouseKeeper implements Runnable { private volatile long previous = plusMillis(currentTime(), -housekeepingPeriodMs); - @SuppressWarnings("AtomicFieldUpdaterNotStaticFinal") - private final AtomicReferenceFieldUpdater catalogUpdater = AtomicReferenceFieldUpdater.newUpdater(PoolBase.class, String.class, "catalog"); @Override public void run() @@ -798,7 +795,7 @@ public final class HikariPool extends PoolBase implements HikariPoolMXBean, IBag leakTaskFactory.updateLeakDetectionThreshold(config.getLeakDetectionThreshold()); if (config.getCatalog() != null && !config.getCatalog().equals(catalog)) { - catalogUpdater.set(HikariPool.this, config.getCatalog()); + HikariPool.this.catalog = config.getCatalog(); } final var idleTimeout = config.getIdleTimeout(); diff --git a/src/main/java/com/zaxxer/hikari/pool/PoolBase.java b/src/main/java/com/zaxxer/hikari/pool/PoolBase.java index 6ce3b616..3be4dcbe 100644 --- a/src/main/java/com/zaxxer/hikari/pool/PoolBase.java +++ b/src/main/java/com/zaxxer/hikari/pool/PoolBase.java @@ -41,7 +41,6 @@ import java.util.concurrent.Executor; import java.util.concurrent.Executors; import java.util.concurrent.ThreadFactory; import java.util.concurrent.ThreadPoolExecutor; -import java.util.concurrent.atomic.AtomicReference; import static com.zaxxer.hikari.pool.ProxyConnection.*; import static com.zaxxer.hikari.util.ClockSource.*; @@ -59,7 +58,7 @@ abstract class PoolBase protected final String poolName; volatile String catalog; - final AtomicReference lastConnectionFailure; + volatile Exception lastConnectionFailure; long connectionTimeout; long validationTimeout; @@ -109,7 +108,6 @@ abstract class PoolBase this.poolName = config.getPoolName(); this.connectionTimeout = config.getConnectionTimeout(); this.validationTimeout = config.getValidationTimeout(); - this.lastConnectionFailure = new AtomicReference<>(); initializeDataSource(); } @@ -177,7 +175,7 @@ abstract class PoolBase return false; } catch (Exception e) { - lastConnectionFailure.set(e); + lastConnectionFailure = e; logger.warn("{} - Failed to validate connection {} ({}). Possibly consider using a shorter maxLifetime value.", poolName, connection, e.getMessage()); return true; @@ -186,7 +184,7 @@ abstract class PoolBase Exception getLastConnectionFailure() { - return lastConnectionFailure.get(); + return lastConnectionFailure; } public DataSource getUnwrappedDataSource() @@ -365,7 +363,7 @@ abstract class PoolBase } setupConnection(connection); - lastConnectionFailure.set(null); + lastConnectionFailure = null; return connection; } catch (Exception e) { @@ -376,7 +374,7 @@ abstract class PoolBase logger.debug("{} - Failed to create/setup connection: {}", poolName, e.getMessage()); } - lastConnectionFailure.set(e); + lastConnectionFailure = e; throw e; } finally {