CODEWITHSUNDEEP
javaconcurrency
membersound
membersound

Reputation: 86697

How to lock a hashmap during refresh?

I have a static HashMap that is populated on application startup, and refreshed daily.

How can I ensure that during refresh no other thread can access the map?

@ThreadSafe
public class MyService {

   private static final Map<String, Object> map = new HashMap<>();
   private MyDao dao;

   public void refresh(List<Object> objects) {
       map.clear();
       map.addAll(dao.findAll()); //maybe long running routine
   }

   public Object get(String key) {
       map.get(key); //ensure this waits during a refresh??
   }
}

Should I introduce a simple boolean lock that is set and cleared during refresh()? Or are there better choices? Or is the synchronized mechanism a way to go?

Upvotes: 2

Views: 1838

Answers (4)

Markus Kull
Markus Kull

Reputation: 1479

Please dont make the map-attribute static, all accessor-methods are non-static.

If get should wait or refresh mutates the map instead of completely exchanging it, then ReadWriteLock is the way to go. ConcurrentMap if the collection is mutated but get should not wait.

But if refresh completely replaces the map, i may suggest different non-waiting implementations:

1) do the long running operation outside the synchronized block

public void refresh() {
       Map<String, Object> objs = dao.findAll();
       synchronized(this) {
         map.clear();
         map.addAll(objs); 
       }
}

public Object get(String key) {
    synchronized(this) {
       return map.get(key); 
    }
}

The readers are not run in parallel, but else perfectly valid.

2) use a volatile non-final reference of an nonchanged collection:

// guava's ImmutableHashMap instead of Map would be even better
private volatile Map<String, Object> map = new HashMap<>();

public void refresh() {
    Map<String, Object> map = dao.findAll();
    this.map = map;
}

3) AtomicReference of nonchanged collection

Instead of a volatile reference also a AtomicReference may be used. Probably better because more explicit than the easily missed volatile.

// guava's ImmutableHashMap instead of Map would be even better
private final AtomicReference<Map<String, Object>> mapRef = 
    new AtomicReference<>(new HashMap<String, Object>());

public void refresh() {
    mapRef.set(dao.findAll());
}

public Object get(String key) {
    return map.get().get(key); 
}

Upvotes: 1

Alex Suo
Alex Suo

Reputation: 3119

It's weird you need to clear() then addAll() for such a global map. I smell your problem needs to be resolved properly by a ReadWriteLock protected double buffering.

Anyway, from a pure performance point of view, on normal server boxes with total number of CPU core < 32, and much more read than write, ConcurrentHashMap is probably your best choice. Otherwise it needs to be studied case by case.

Upvotes: 0

assylias
assylias

Reputation: 328598

You could use a volatile map and reassign it after population:

public class MyService {

   private static volatile Map<String, Object> map = new HashMap<>();
   private MyDao dao;

   public void refresh(List<Object> objects) {
       Map<String, Object> newMap = new HashMap<>();
       newMap.addAll(dao.findAll()); //maybe long running routine
       map = newMap;
   }

   public Object get(String key) {
       map.get(key); //ensure this waits during a refresh??
   }
}

It is non blocking, the assignment from newMap to map is atomic and ensures visibility: any subsequent call to get will be based on the refreshed map.

Performance wise this should work well because volatile reads are almost as fast as normal reads. Volatile writes are a tiny bit slower but considering the refreshing frequency it should not be an issue. If performance matters you should run appropriate tests.

Note: you must make sure that no external code can get access to the map reference, otherwise that code could access stale data.

Upvotes: 4

Cihan Tek
Cihan Tek

Reputation: 5409

Using synchronized block or a ReadWriteLock would be a better choice here. This way, you wouldn't have to change anything in the calling code.

You could also use a concurrentHash, but in that case, for aggregate operations such as putAll and clear, concurrent retrievals may reflect insertion or removal of only some entries.

Upvotes: 0

Related Questions