What kind of the Map interface I should use?

Question

I need to make the following class thread-safe:

//Shared among all threads
public class SharedCache {

    private Map<Object, Future<Collection<Integer>>> chachedFutures;
    {
        chachedFutures = new ConcurrentHashMap<>(); //not sure about that
    }

    public Future<Collection<Integer>> ensureFuture(Object value,
                                                     FutureFactory<Collection<Integer>> ff){
        if(chachedFutures.containsKey(value))
            return chachedFutures.get(value);
        Future<Collection<Integer>> ftr = ff.create();
        chachedFutures.put(value, ftr);
        return ftr;
    }

    public Future<Collection<Integer>> remove(Object value){
        return chachedFutures.remove(value);
    }

}

After reading the article about the ConcurrentHashMap class it's still difficult for me to make a right decision.

Firstly, I tended to make the methods ensureFuture and remove just synchronized. And it would work, but from the performance standpoint it was not very good because of mutually-exclusing.

I don't know the exact (even approximately) amount of threads having access to the Cache simultaneously and the size of the Cache. Taking into account that

resizing this or any other kind of hash table is a relatively slow operation

I didn't specify the initial size of the map. Also the concurrencyLevel parameter. Is it justified to use ConcurrentHashMap here or synchronized methods would be enough?

It may also depend on whether you want your data to be consistent. The `ConcurrentHashMap` is thread safe per se, but will not guarantee the consistency of your data. The right idiom for both consistency and thread-safety (at the cost of performance) would be synchronizing on a `Collections.synchronizedMap`. — Mena, Sep 30 '15 at 09:21
@Mena but it's just a decorator, therefore it garuntees that the only one thread can have acces to the map at a time. — St.Antario, Sep 30 '15 at 09:23

score 2 · Accepted Answer · edited May 23 '17 at 10:29

2

You have following methods:

     public Future<Collection<Integer>> ensureFuture(Object value,
                                                     FutureFactory<Collection<Integer>> ff){
        if(chachedFutures.containsKey(value))
            return chachedFutures.get(value);
        Future<Collection<Integer>> ftr = ff.create();
        chachedFutures.put(value, ftr);
        return ftr;
    }

    public Future<Collection<Integer>> remove(Object value){
        return chachedFutures.remove(value);
    }

There are some points to be noticed:

Suppose method ensureFuture is not synchronized in that case it is possible that one thread invokes containsKey which returns true but before next line is executed another thread may remove the entry respective to that key. This can lead to race condition as it is check-then-act scenario. Check this as well.
Also you are using chachedFutures.put(value, ftr) but IMO you should use chachedFutures.putIfAbsent(value, ftr) . For this method if the specified key is not already associated with a value (or is mapped to null) associates it with the given value and returns null, else returns the current value. Using this you can also avoid contains check.

Is it justified to use ConcurrentHashMap here or synchronized methods would be enough?

It depends as CHM needs more memory compared to HashMap due to lot of bookkeeping activities etc. Another alternative is to use Collections.synchronizedMap which will provide synchronization on a regular HashMap.

edited May 23 '17 at 10:29

Community

1
1

answered Sep 30 '15 at 09:26

akhil_mittal

23,309
7
96
95

Good advice to use `putIfAbsence`, indeed. So, using the method will make the class thread-safe, right? Also it will have a better performance than the synchronized version has. – St.Antario Sep 30 '15 at 09:29
If you use method `putIfAbsence` it will save you from the `check-then-act` problem. – akhil_mittal Sep 30 '15 at 09:32
Got it. Let me ask you one more question. I'm not sure about safety if one thread's trying to remove a value and another one ensuring the same value. – St.Antario Sep 30 '15 at 09:36
In that case (when other thread ensures the same value) it may not be a problem but prevention is better than cure :). Moreover race condition should always be cured. – akhil_mittal Sep 30 '15 at 09:46
You mean, to use locking preventing it explicitly? – St.Antario Sep 30 '15 at 09:47
Nope. I mean avoid race condition as that is a problem. Locking is one way of solving this problem. We have other ways too. Also optimistic locking can also provide better results. – akhil_mittal Sep 30 '15 at 09:49

What kind of the Map interface I should use?

1 Answers1