Question about synchronized collections
Hi, I designed a class with a static Map as a cache for expensive objects. I declared the Map as final, and created it in a static initializer through the Collections.synchronizedMap(Map<K, V>) method. The class which uses this Map is a singleton and I synchronized on the map object for every read/write operation, so I'm sure that my current implementation is thread-safe. However, I wanted to ask what's the difference between having a normal static Map on which I synchronized every time and having a synchronized Map returned by the Collections utility class? Could it be that I could use the synchronized Map without synchronizing access to it?
Thanks for any answers,
Marco
[707 byte] By [
mtedonea] at [2007-11-27 4:42:29]
# 1
Collections.synchronizedMap wraps the Map interface and handles all the synchronization to make it thread-safe. This means you can have multiple threads calling add at the same time without any problems. Although you can still have to synchronize some operations by yourself such as a Get, Compare and Remove.
# 2
> problems. Although you can still have to synchronize> some operations by yourself such as a Get, Compare> and Remove.The only thing you have to explicitly sync on is around iterations over the entrySet(), ketSet(), or values().
# 3
> > problems. Although you can still have to
> synchronize
> > some operations by yourself such as a Get, Compare
> > and Remove.
>
> The only thing you have to explicitly sync on is
> around iterations over the entrySet(), ketSet(), or
> values().
What about the put, get, containsKey? I've got a method which returns an InitialContext (which I'm caching in the map). The method is something like that:
public InitialContext getJbossInitialContext(String providerUrl) {
synchronized (icCache) {
if (icCache.containsKey(providerUrl)) {
return icCache.get(providerUrl);
} else {
//Create expensive object, and cache it
}
}
}
Is this too much synchronization to synchronized as above on a synchronized Map? Would it be ok for me just to have a simple Map and synchronized as above? Or could I take the above synchronization off?
Thanks
# 4
does we pass the data from one thread to another is yes then how?/
# 5
> does we pass the data from one thread to another is> yes then how?/The maps are not passed to different threads. Maps are 'touched' only by one thread, but the methods which could change the map's state can be invoked by multiple threads concurrently
# 6
So, just to confirm. From what it has been said in this forum I assume the following should be thread-safe:
/*
* MyCache.java
*
* Created on 19 May 2007, 07:06
*
* To change this template, choose Tools | Template Manager
* and open the template in the editor.
*/
package experiments;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import java.util.Properties;
import javax.naming.Context;
import javax.naming.InitialContext;
import javax.naming.NamingException;
/**
*
* @author mtedone
*/
public class MyCache {
/** The Map containing my cache */
private static final Map<String, InitialContext> myCache;
/** An instance of this class */
private static MyCache singleton;
static {
myCache = Collections.synchronizedMap(new HashMap<String, InitialContext>(20));
}
/** Creates a new instance of MyCache */
private MyCache() {
}
/**
* Implements a simplified version of the singleton pattern
*/
public static MyCache getInstance() {
if (null == singleton) {
singleton = new MyCache();
}
return singleton;
}
/**
* Returns an (possibly cached) InitialContext for the given provider url
*/
public InitialContext getInitialContext(String providerUrl) {
if (myCache.containsKey(providerUrl)) {
return myCache.get(providerUrl);
} else {
try {
Properties props = new Properties();
props.put(Context.PROVIDER_URL, providerUrl);
InitialContext ctx = new InitialContext(props);
myCache.put(providerUrl, ctx);
return ctx;
} catch (NamingException ex) {
ex.printStackTrace();
}
}
return null;
}
/**
* Clears the cache
*/
public void clearMyCache() {
myCache.clear();
}
/*
* Removes an element from the cache
*/
public void removeInitialContext(String providerUrl) {
if (myCache.containsKey(providerUrl)) {
myCache.remove(providerUrl);
}
}
}
# 7
Use java.util.concurrent.ConcurrentMap.That is the best Map implementation to be used as a cache,and support concurrent access.
# 8
> Use java.util.concurrent.ConcurrentMap.
> That is the best Map implementation to be used as a
> cache,
> and support concurrent access.
Ok, but looking at the Javadocs for ConcurrentHashMap I can see:
[...]
However, even though all operations are thread-safe, retrieval operations do not entail locking[...] Retrieval operations (including get) generally do not block, so may overlap with update operations (including put and remove), etc
Now my question is: what happens if there is a race condition between two threads accessing getInitialContext? Is there the risk that the InitialContext will be created twice, even if with the putIfAbsent method we'll only put it in the table once?
# 9
Hi Marco,
I have not tested this code yet, especially for concurrency and contention issues. Perhaps you would like to write a test for it?
Anyway, what might work is to put a placeholder into the concurrent map using the PutIfAbsent method, together with a virtual constructor that then shows it how to construct it, if it is actually put into the map and then subsequently used. I hope I'm making sense :-)
Let's start with a generic class called ConcurrentProxyCache:
package eu.javaspecialists.tjsn.concurrent;
import java.util.concurrent.*;
public class ConcurrentProxyCache<K,V> {
private final ConcurrentMap<K, V> map =
new ConcurrentHashMap<K, V>();
private final Class<V> valueInterface;
public ConcurrentProxyCache(Class<V> valueInterface) {
if (!valueInterface.isInterface()) {
throw new IllegalArgumentException(
"valueInterface is a class");
}
this.valueInterface = valueInterface;
}
public V getValue(K key, VirtualConstructor<V> constructor) {
V result = map.get(key);
if (result == null) {
map.putIfAbsent(key,
ProxyFactory.virtualProxy(valueInterface, constructor));
result = map.get(key);
}
return result;
}
public void clear() {
map.clear();
}
public V remove(K key) {
return map.remove(key);
}
}
We then define the virtual constructor, a la Factory Method:
package eu.javaspecialists.tjsn.concurrent;
public interface VirtualConstructor<T> {
public T make() throws Throwable;
}
And the ProxyFactory:
package eu.javaspecialists.tjsn.concurrent;
import java.lang.reflect.*;
public class ProxyFactory {
public static <T> T virtualProxy(
Class<T> target,
final VirtualConstructor<T> virtualConstructor) {
return target.cast(
Proxy.newProxyInstance(
Thread.currentThread().getContextClassLoader(),
new Class<?>[]{target},
new InvocationHandler() {
private final Object creationLock = new Object();
private volatile T realObject;
public Object invoke(
Object proxy,
Method method,
Object[] args) throws Throwable {
if (realObject == null) {
synchronized (creationLock) {
if (realObject == null) {
realObject = virtualConstructor.make();
}
}
}
return method.invoke(realObject, args);
}
}
));
}
}
Your code would then simply use that cache inside the MyCache class. I removed the singleton code, since that is not really the point of the discussion here:
import eu.javaspecialists.tjsn.concurrent.*;
import javax.naming.*;
import java.util.Properties;
/**
* @author mtedone and heinz kabutz
*/
public class MyCache {
private final ConcurrentProxyCache<String, Context> cache =
new ConcurrentProxyCache<String, Context>(Context.class);
/**
* Returns a (possibly cached) Context for the given provider url
*/
public Context getContext(final String providerUrl) {
return cache.getValue(providerUrl,
new VirtualConstructor<Context>() {
public Context make() throws NamingException {
Properties props = new Properties();
props.put(Context.PROVIDER_URL, providerUrl);
return new InitialContext(props);
}
});
}
/**
* Clears the cache
*/
public void clearMyCache() {
cache.clear();
}
/*
* Removes an element from the cache
*/
public void removeContext(String providerUrl) {
cache.remove(providerUrl);
}
}
WARNING: I have not run this code, and I do not know whether it will work or not. I think it will, but I'm not a JVM :-)
Heinz Kabutz
The Java Specialists' Newsletter
http://www.javaspecialists.eu
# 10
If you need something simpler, you can do the following:
package experiments;
import java.util.*;
import javax.naming.*;
public class MyCache {
private final Map<String, Context> myCache =
new HashMap<String, Context>();
private final static MyCache singleton = new MyCache();
private MyCache() { }
public static MyCache getInstance() {
return singleton;
}
/**
* Returns an (possibly cached) InitialContext for the given provider url
*/
public Context getInitialContext(String providerUrl) throws NamingException {
synchronized(myCache) {
Context ctx = myCache.get(providerUrl);
if (ctx == null) {
Properties props = new Properties();
props.put(Context.PROVIDER_URL, providerUrl);
ctx = new InitialContext(props);
myCache.put(providerUrl, ctx);
}
return ctx;
}
}
public void clearMyCache() {
synchronized(myCache) {
myCache.clear();
}
}
public void removeInitialContext(String providerUrl) {
synchronized(myCache) {
myCache.remove(providerUrl);
}
}
}
Heinz
# 11
Yeah, I'd normally have gone for this solution. The only reason why I chose a ConcurrentMap is that the iterator is not fail-fast, that means it doesn't throw a ConcurrentModificationException if the underlying collection changes. If I use a map that also iterations through the iterator would need to be synchronized, even if I returned a read-only collection through Collections.unmodifiableMap().
