Populating map from multiple threads

Question

I have a ConcurrentHashMap which I am populating from multiple threads as shown below:

private static Map<ErrorData, Long> holder = new ConcurrentHashMap<ErrorData, Long>();

public static void addError(ErrorData error) {
    if (holder.keySet().contains(error)) {
        holder.put(error, holder.get(error) + 1);
    } else {
        holder.put(error, 1L);
    }
}

Is there any possibility of race condition in above code and it can skip updates? Also how can I use Guava AtomicLongMap here if that can give better performance?

I am on Java 7.

Answer 1

Yes, there is a possibility of a race because you are not checking contains and putting atomically.

You can use AtomicLongMap as follows, which does this check atomically:

private static final AtomicLongMap<ErrorData> holder = AtomicLongMap.create();

public static void addError(ErrorData error) {
  holder.getAndIncrement(error);
}

As described in the javadoc:

[T]he typical mechanism for writing to this map is addAndGet(K, long), which adds a long to the value currently associated with K. If a key has not yet been associated with a value, its implicit value is zero.

and

All operations are atomic unless otherwise noted.