CHM replaceAll

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

CHM replaceAll

Ben Manes
ConcurrentHashMap#replaceAll(bifunction) performs the computation optimistically per key, retrying until successful. This is a valid implementation, but slightly surprising due to the map's ability to perform the per-key operations pessimistically. In the case of a map holding a resource that should be explicitly closed, these races could result in a leak. That of course is a user error, but one that I think is easily corrected without a negative loss for the hash table. This of course is more important for my case of a cache, where I provide a custom implementation for a variety of reasons.

The request is to use computeIfPresent per key. The function would have to be decorated with a lambda that disallows null values. I suspect this single allocation is preferable to the cost of retrying on a conflict. That allocation could be removed with a more complex replacement, but that is probably unnecessary.

Cheers,
Ben



_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

Re: CHM replaceAll

Doug Lea

Sorry; I let this sit as an issue about which I didn't have much
of an opinion, to see if anyone else did, but then forgot to
revisit it until now...

On 02/22/2016 12:29 PM, Ben Manes wrote:
> ConcurrentHashMap#replaceAll(bifunction) performs the computation
> optimistically per key, retrying until successful. This is a valid
> implementation, but slightly surprising due to the map's ability to perform
> the per-key operations pessimistically. In the case of a map holding a
> resource that should be explicitly closed, these races could result in a
> leak. That of course is a user error, but one that I think is easily
> corrected without a negative loss for the hash table. This of course is more
> important for my case of a cache, where I provide a custom implementation for
> a variety of reasons.

One minor argument against this is that doing so would make it subtly
incompatible with similar operations on Streams (java.util.stream).

>
> The request is to use computeIfPresent per key. The function would have to be
>  decorated with a lambda that disallows null values.

So, if you care about this issue, you could instead use compute or
computeIfPresent in a loop instead of replaceAll.

> I suspect this single allocation is preferable to the cost of retrying on a
> conflict. That allocation could be removed with a more complex replacement,
> but that is probably unnecessary.
>

If there were a compelling case to do this, we'd probably want to do it
as efficiently as possible, unlayering it as another variant of the "compute"
code.

-Doug

_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest