RFR (XS) 8145539: (coll) AbstractMap.keySet and .values should not be volatile

[Peter Levart]

Hi Aleksey,

Wouldn't that change make a possible outcome of keySet() returning null 
in case it was invoked concurrently? Wouldn't you have to use a local 
variable to prevent that?

Regards, Peter

On 12/16/2015 11:53 PM, Aleksey Shipilev wrote:
> Hi,
>
> Since the dawn of OpenJDK, AbstractMap.keySet and .value were defined as
> package-private volatile fields. Their only use is to cache keySet and
> valueSet implementations from java.util collections.
>
> However, all relevant java.util collections are not having any declared
> fields (an opaque reference to enclosing class is stored in final
> field), and they delegate straight to the backing collection. Therefore,
> any race on cache field is benign, and we can drop "volatile" from the
> fields:
>    https://bugs.openjdk.java.net/browse/JDK-8145539
>    http://cr.openjdk.java.net/~shade/8145539/webrev.02/
>
> This improves performance for keySet()/values() on AbstractMap
> implementations, because we don't emit barriers (volatile write in x86
> case). This does not affect AbstractMap subclasses allocation
> performance, because the volatile field values were default since
> JDK-8035284.
>
> See:
>   http://cr.openjdk.java.net/~shade/8145539/HashMapBench.java
>
> Testing: microbenchmarks, java/util jtreg on Linux x86_64
>
> Thanks,
> -Aleksey
>