What is the preferred way to modify a value in ConcurrentHashMap?

Question

Let's say I have a Concurrent Map that is high-read, low-write, and needs to store application data:

ConcurrentMap<UUID, Data> map = new ConcurrentHashMap<UUID, Data>();

Then, during startup and through user input, data is added to the map:

public void createData(Data newData) {
    map.put(newId, newData); // etc...
}

If I then need to change the data, should I:

A) Make the Data class objects immutable, and then conduct a put operation every time a change is needed for a Data object:

public void changeData(UUID oldId, Foo newInfo) {
    Data oldData = map.get(oldId);
    Data newData = new Data(oldData, newInfo); // Constructor for demo only
    map.put(newData);
    saveToDatabase(newData);
}

B) Make the Data class objects mutable yet thread-safe with volatile fields, atomic references or final concurrent fields, and simply modify the object as needed:

public void changeData(UUID oldId, Foo newInfo) {
    Data data = map.get(id);
    data.changeSomething(newInfo);
    saveToDatabase(data);
}

C) None of the above

Answer 1

A) is the better option, for two reasons:

1. Since in your scenario reads are more frequent, you should reduce the amount of overhead for them. Adding additional synchronization (such as volatile) works against you in this case.
2. By using mutable objects with additional custom safeguards (which may have bugs) you're pretty much defeating the point of making your life easier by using ConcurrentHashMap.

Answer 2

If you have an option of making an immutable class, you would be much better off with your implementation #A: in-place modifications are significantly harder to implement and maintain.

Sometimes going the immutable route may not be an option, because of the need to make frequent modifications to a relatively large object. In this case you may want to reconsider the application of the concurrent hash map to your design, because the fact that it is synchronized does not give you too much an advantage.

Answer 3

Just a thought. You stated that the write rate is low, but for the sake of the argument let's suppose multiple concurrent writes / calls of the changeData method. It is then possible that the thread that called the method the last, finishes first (in both approaches).

If your application logic assumes that the order of insertion will be honored it may yield wrong results. In that case, the body of the method changeData is your critical section which per definition means that it should not be executed concurrently.

Critical section definition is highly sensitive to application domain semantics and the code structure, so I can't really tell if that method is to be considered a critical section. Guessing by the names of the variables, and supposing that your map is a user data cache from database, I'd guess that you can disregard this answer. But do carefully think of it, though :)

If all the writes go through this method, this would be a sketch of the code (you can use non thread safe map implementation then):

public void changeData(UUID oldId, Foo newInfo) {
    synchronized(SomeClass.class) { // global lock
        //update logic
    }
}

This is just a sketch to illustrate the point of course. You can most probably use some of the Java concurrent constructs, if this is the problem.