Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

ConcurrentHashMap Conditional Replace

I'd like to be able to conditionally replace a value in a ConcurrentHashMap. That is, given:

public class PriceTick {
   final String instrumentId;
   ...
   final long timestamp;
   ...

And a class (let's call it TickHolder) which owns a ConcurrentHashMap (let's just call it map).

I wish to be able to implement a conditional put method, so that if there's no entry for the key, the new one is inserted, but if there is an existing entry, the new one is inserted only if the timestamp value in the new PriceTick is greater than the existing one.

For an old-school HashMap solution, TickHolder would have a put method:

public void add(PriceTick tick) {
   synchronized(map) {
      if ((map.get(tick.instrumentId) == null)
         || (tick.getTimestamp() > map.get(tick.instrumentId).getTimestamp()) )
         map.put(tick.instrumentId, tick);
      }
}

With a ConcurrentHashMap, one would want to drop the synchronization and use some atomic method like replace, but that's unconditional. So clearly the "conditional replace" method must be written.

However, since the test-and-replace operation is non-atomic, in order to be thread safe, it would have to be synchronized - but my initial reading of the ConcurrentHashMap source leads me to think that external synchronization and their internal locks will not work very well, so at a very minimum, every Map method which performs structural changes and the containing class performs would have to be synchronized by the containing class... and even then, I'm going to be fairly uneasy.

I thought about subclassing ConcurrentHashMap, but that seems to be impossible. It makes use of an inner final class HashEntry with default access, so although ConcurrentHashMap is not final, it's not extensible.

Which seems to mean that I have to fall back to implementing TickHolder as containing an old-school HashMap in order to write my conditional replace method.

So, the questions: am I right about the above? Have I (hopefully) missed something, whether obvious or subtle, which would lead to a different conclusion? I'd really like to be able to make use of that lovely striped locking mechanism here.

like image 261
CPerkins Avatar asked Jan 23 '23 05:01

CPerkins


2 Answers

The non-deterministic solution is to loop replace():

do {
  PriceTick oldTick = map.get(newTick.getInstrumentId());
} while ((oldTick == null || oldTick.before(newTick)) && !map.replace(newTick.getInstrumentId(), oldTick, newTick);

Odd though it may seem, that is a commonly suggested pattern for this kind of thing.

like image 120
cletus Avatar answered Jan 30 '23 13:01

cletus


@cletus solution formed the base for my solution to an almost identical problem. I think a couple of changes are needed though as if oldTick is null then replace throws a NullPointerException as stated by @hotzen

PriceTick oldTick;
do {
  oldTick = map.putIfAbsent(newTick.getInstrumentId());
} while (oldTick != null && oldTick.before(newTick) && !map.replace(newTick.getInstrumentId(), oldTick, newTick);
like image 44
robSE13 Avatar answered Jan 30 '23 13:01

robSE13