Volatile HashMap vs ConcurrentHashMap

Question

I have a cache class which contains a volatile HashMap<T> to store cache items.

I'm curious what would be the consequences of changing volatile HashMap to ConcurrentHashMap?

Would i gain performance increase? This cache is readonly cache.

What would be the best option to use? just HashMap? Cache is being populated on a interval.

Solution 1

First, it appears you don't understand what the volatile keyword does. It makes sure that if the reference value held by the variable declared volatile changes, other threads will see it rather than having a cached copy. It has nothing to do with thread-safety in regard to accessing the HashMap

Given that, and the fact that you say the HashMap is read-only ... you certainly don't need to use anything that provides thread-safety including a ConcurrentHashMap

Edit to add: Your last edit you now say "The cache is being populated on a interval"

That's not read-only then, is it?

If you're going to have threads reading from it while you are writing (updating the existing HashMap) then you should use a ConcurrentHashMap, yes.

If you are populating an entirely new HashMap then assigning it to the existing variable, then you use volatile

Solution 2

You say the cache is read-only, but also being updated on an interval which seems contradictory.

If the whole cache gets updated on an interval, I'd keep using the volatile. The volatile will make sure that the updated map is safely published.

public final class  Cache
{
   private volatile Map<?,?> cache;

   private void mapUpdate() {
      Map<?,?> newCache = new HashMap<>();

      // populate the map

      // update the reference with an immutable collection
      cache = Collections.unmodifiableMap(newCache);
   }
}

If the interval update is modifying the same cache, then you probably want to use a ConcurrentHashMap, or copy the map, update the copy, and update the reference.

public final class  Cache
{
   private volatile Map<?,?> cache;

   private void mapUpdate() {
      Map<?,?> newCache = new HashMap<>(cache);

      // update the map

      // update the reference with an immutable collection
      cache = Collections.unmodifiableMap(newCache);
   }
}

Comments

[DarthVader]

well that was the reason why i was using volatile, there was a background thread reading from file and making a new hashmap then assigning to the cache. I m planning to change the design, that s why i asked . You answered both cases though. Thanks.

[Voo]

Well actually you can use volatile to make sure people see the most current elements (basically the same way you do to get "volatile" array elements), it's just a) bad performance, b) convoluted, c) doesn't help with the internal race conditions when adding data and d) plain stupid. But doable! ;)

[DarthVader]

@Voo what would u use? or how would u do it?

[Voo]

@Darth Note my comment was about using volatile to add entries on a hashmap and make sure they're visible - not what you want to do. b2t: I'd simply go with ConcurrentHashmap first - it was built for that purpose. If I had a high performance bottleneck in the map, I'd go for Cliff's lockfree HashMap implementation, in any case with a background thread to remove old entries from the cache. Less memory overhead, no big spikes and still quite good performance. If you really only want to create a new cache every X minutes, Michael's solution will work fine.

[toto2]

@BrianRoach I don't get your comment that a read-only map does not need to be thread-safe. Some thread has to put some data in the map at some point and if that data is added after the map has been passed around to other threads, you are in trouble.

[toto2]

I guess you mean an immutable object does not need any synchronization. You could use Map immutableMap = Collections.unmodifiableMap(someMap).

[Brian Roach]

@toto2 - Concurrency. It's perfectly fine if you were to populate a read-only map before any other thread were to access it; you do have control of that when programming. Then there is no concurrency therefore no need for synchronization.

[vlsergey]

I would consider to keep cache field non-volatile. Because of Collections.unmodifiableMap final field link to new version of map will be safely published. At some point all threads would pickup up new version of cache map. Usually (but not always!) it doesn't matter if cache will be available not immediately but in couple of milliseconds. And making this field non-volatile improve CPU cache performance a bit.

[kisna]

Why do you need volatile here if the activeRequests map reference remains same and does not change and only the values inside the map change? Assuming, this is a singleton instance shared by multiple threads. You should use a concurrentHashMap instead of a synchronized method as you don't change anything else here.