Permalink
Browse files

refactor: do not synchronize on ConcurrentHashMap

PGPoolingDataSource synchronizes on ConcurrentHashMap. This is
generally discouraged because ConcurrentHashMap has its own
synchronization mechanisms that allow for higher parallelism.

Replace synchronization in PGPoolingDataSource with atomic operations
of ConcurrentHashMap.

Fixes #462, #468
  • Loading branch information...
marschall authored and vlsi committed Dec 26, 2015
1 parent 00c7eb3 commit ea4caddf0cea82e59d70e7eb938dd08db55667bb
Showing with 12 additions and 18 deletions.
  1. +12 −18 pgjdbc/src/main/java/org/postgresql/ds/PGPoolingDataSource.java
@@ -15,9 +15,9 @@
import java.sql.Connection;
import java.sql.SQLException;
import java.sql.SQLFeatureNotSupportedException;
import java.util.Map;
import java.util.Stack;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
import javax.naming.NamingException;
import javax.naming.Reference;
import javax.naming.StringRefAddr;
@@ -50,15 +50,15 @@
* implementation may provide advanced features, such as using a single pool
* across all VMs in a cluster.</p>
*
* <p>This implementation supports JDK 1.3 and higher.</p>
* <p>This implementation supports JDK 1.5 and higher.</p>
*
* @author Aaron Mulder (ammulder@chariotsolutions.com)
*/
public class PGPoolingDataSource
extends BaseDataSource
implements DataSource
{
protected static Map<String, PGPoolingDataSource> dataSources = new ConcurrentHashMap<String, PGPoolingDataSource>();
protected static ConcurrentMap<String, PGPoolingDataSource> dataSources = new ConcurrentHashMap<String, PGPoolingDataSource>();
public static PGPoolingDataSource getDataSource(String name)
{
@@ -270,19 +270,16 @@ public void setDataSourceName(String dataSourceName)
{
return ;
}
synchronized (dataSources)
PGPoolingDataSource previous = dataSources.putIfAbsent(dataSourceName, this);
if (previous != null)
{
if (getDataSource(dataSourceName) != null)
{
throw new IllegalArgumentException("DataSource with name '" + dataSourceName + "' already exists!");
}
if (this.dataSourceName != null)
{
dataSources.remove(this.dataSourceName);
}
this.dataSourceName = dataSourceName;
addDataSource(dataSourceName);
throw new IllegalArgumentException("DataSource with name '" + dataSourceName + "' already exists!");
}
if (this.dataSourceName != null)
{
dataSources.remove(this.dataSourceName);
}
this.dataSourceName = dataSourceName;
}
/**
@@ -408,10 +405,7 @@ public void close()
}
protected void removeStoredDataSource() {
synchronized (dataSources)
{
dataSources.remove(dataSourceName);
}
dataSources.remove(dataSourceName);
}
protected void addDataSource(String dataSourceName)

0 comments on commit ea4cadd

Please sign in to comment.