HashMap.computeIfAbsent() behavior in Java 9

If in your code you are using computeIfAbsent(), be aware that it has changed its behavior in Java 9. Now you get a ConcurrentModificationException if the mapping function changes the map.

So, for instance, this Fibonacci calculator works fine in Java 8:
class Fibonacci {
    private static final Map<Integer, Long> cache = new HashMap<>();
    static {
        cache.put(0, 0L);
        cache.put(1, 1L);
    }

    public long calculate(int x) {
        return cache.computeIfAbsent(x, n -> calculate(n - 1) + calculate(n - 2));
    }
}
But not if you try to run it in Java 9.

As the computeIfAbsent()'s javadoc states quite clearly, "This method will, on a best-effort basis, throw a ConcurrentModificationException if it is detected that the mapping function modifies this map during computation". If you have a look at the code, you will see that line 1139 is:
if (mc != modCount) { throw new ConcurrentModificationException(); }
And the mc counter is increased a bit below to keep track of any changes in the map due to mappingFunction.

My way out to this issue has been refactoring the Fibonacci calculation to get rid of computeIfAbsent(). Something like that:
public long calculate(int x) {
    if (cache.containsKey(x)) {
        return cache.get(x);
    }

    long lhs = cache.containsKey(x - 1) ? cache.get(x - 1) : calculate(x - 1);
    long rhs = cache.containsKey(x - 2) ? cache.get(x - 2) : calculate(x - 2);
    long result = lhs + rhs;

    cache.put(x, result);
    return result;
}
Well, it's not a beauty. At least it works.

5 comments:

  1. Can't you use ConcurrentHashMap instead?

    ReplyDelete
    Replies
    1. Yes, Klitos, you are right, thank you. Now I would more precisely say that if our code should support Java 9, and we spot a reference to computeIfAbsent(), we should ensure that it is called from a concurrent hash map or patch the code to avoid that method call.

      In the current case I would stick to my not-so-nice patch, since I don't want to pay for the map synchronization when I know for sure there is no need for that here.

      Delete
    2. You don't need to change all uses of computeIfAbsent() to use a ConcurrentHashMap. HashMap.conputeIfAbsent is still a very useful method, as long as you don't concurrently modify the map.

      Delete
    3. Wait a minute, Klitos. In the provided example there is no concurrent access to the cache, still computeIfAbsent() throws an exception. And its source code shows that it is thrown after checking just the modification count. Am I missing anything?

      Delete
    4. It's the API designer's definition of "concurrent" for this particular method: a modification to the map that happens between the call of the method and the return from the method. In this example, that's exactly what happens - even though it all happens in the same thread.

      Delete