java concurrency in practice 5.19

Question

In JCIP book, Listing 5.19 Final Implementation of Memorizer. My questions are:

1. The endless while loop is here because of atomic putIfAbsent()?
2. should the while loop just inside impl of putIfAbsent() instead of client code?
3. should the while loop be in smaller scope just wrapping putIfAbsent()?
4. while loop looks bad on readability

Code:

public class Memorizer<A, V> implements Computable<A, V> {
    private final ConcurrentMap<A, Future<V>> cache
            = new ConcurrentHashMap<A, Future<V>>();
    private final Computable<A, V> c;
    public Memorizer(Computable<A, V> c) { this.c = c; }
    public V compute(final A arg) throws InterruptedException {
    while (true) { //<==== WHY?
        Future<V> f = cache.get(arg);
        if (f == null) {
           Callable<V> eval = new Callable<V>() {
               public V call() throws InterruptedException {
                    return c.compute(arg);
               }
           };
           FutureTask<V> ft = new FutureTask<V>(eval);
           f = cache.putIfAbsent(arg, ft);
           if (f == null) { f = ft; ft.run(); }
        }
        try {
           return f.get();
        } catch (CancellationException e) {
           cache.remove(arg, f);
        } catch (ExecutionException e) {
           throw launderThrowable(e.getCause());
        }
     }
   }
}

Answer 1

1) The endless while loop is here because of atomic putIfAbsent()?

The while loop here is for repeating computation when a computation was cancelled (first case in try).

2) Should the while loop just inside impl of putIfAbsent() instead of client code?

No, please, read what putIfAbsent does. It just tries to put an object once only.

3) Should the while loop be in smaller scope just wrapping putIfAbsent()?

No, it shouldn't. See #1.

4) While loop looks bad on readability.

You are free to offer something better. In fact, this construction suites perfect for situation when you have to try to do something until it proceeds successfully.

Answer 2

No, you cannot reduce the scope of the while loop. You want to do f.get() on the value that is in the cache. If there was no value for arg in the map, you want to execute get() on your result, otherwise you want to get the existing value for arg in the map and get() that one.

The problem is that there are no locks in this implementation, so between you checking if there is a value and trying to insert a value, another thread could have inserted its own value. Equally, it could be the case that between the insertion failing and the retrieval, the value could have been removed from the cache (due to an CancellationException). Because of these failure cases, you spin in the while(true) until either you can get the canonical value out of the map or you insert a new value into the map (making your value canonical).

It would seem that you could try to more the f.get() out of the loop, but that is kept in due to the risk of an CancellationException, where you want to keep trying.