Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8145539

(coll) AbstractMap.keySet and .values should not be volatile

    Details

    • Type: Enhancement
    • Status: Resolved
    • Priority: P4
    • Resolution: Fixed
    • Affects Version/s: 8u66, 9
    • Fix Version/s: 9
    • Component/s: core-libs

      Backports

        Description

        A DESCRIPTION OF THE REQUEST :
        AbstractMap has two fields which are declared volatile:

        public abstract class AbstractMap<K,V> implements Map<K,V> {
        .....
            /**
             * Each of these fields are initialized to contain an instance of the
             * appropriate view the first time this view is requested. The views are
             * stateless, so there's no reason to create more than one of each.
             */
            transient volatile Set<K> keySet;
            transient volatile Collection<V> values;
        ....
        }

        This is unnecessary since there is nothing in the API requiring these fields to be volatile.

        JUSTIFICATION :
        Removing volatile will reduce the cost of creating instances extending AbstractMap, as well as invoking e.g. ::values() and ::keySet() on a HashMap. In the application I'm working in hundreds of thousands HashMaps are created each second.


          Issue Links

            Activity

            Hide
            psonal Pallavi Sonal added a comment -
            Moving across to dev-team for evaluation.
            Show
            psonal Pallavi Sonal added a comment - Moving across to dev-team for evaluation.
            Hide
            shade Aleksey Shipilev added a comment -
            This seems to predate OpenJDK:
             http://hg.openjdk.java.net/jdk9/jdk9/jdk/file/37a05a11f281/src/share/classes/java/util/AbstractMap.java#l294

            I agree with submitter, there seem to be no obvious reason to keep these fields volatile. Concurrent collections are expected to override values() and keySet() with their own thread-safe implementations.
            Show
            shade Aleksey Shipilev added a comment - This seems to predate OpenJDK:   http://hg.openjdk.java.net/jdk9/jdk9/jdk/file/37a05a11f281/src/share/classes/java/util/AbstractMap.java#l294 I agree with submitter, there seem to be no obvious reason to keep these fields volatile. Concurrent collections are expected to override values() and keySet() with their own thread-safe implementations.
            Hide
            shade Aleksey Shipilev added a comment -
            It is probably not that bad for HashMap initialization after JDK-8035284, but acquiring keySet/values should experience problems.
            Show
            shade Aleksey Shipilev added a comment - It is probably not that bad for HashMap initialization after JDK-8035284 , but acquiring keySet/values should experience problems.
            Show
            shade Aleksey Shipilev added a comment - RFR: http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-December/037674.html
            Hide
            hgupdate HG Updates added a comment -
            URL: http://hg.openjdk.java.net/jdk9/dev/jdk/rev/ce72c7641f38
            User: shade
            Date: 2015-12-17 22:45:16 +0000
            Show
            hgupdate HG Updates added a comment - URL: http://hg.openjdk.java.net/jdk9/dev/jdk/rev/ce72c7641f38 User: shade Date: 2015-12-17 22:45:16 +0000
            Hide
            hgupdate HG Updates added a comment -
            URL: http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/ce72c7641f38
            User: lana
            Date: 2015-12-23 23:04:19 +0000
            Show
            hgupdate HG Updates added a comment - URL: http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/ce72c7641f38 User: lana Date: 2015-12-23 23:04:19 +0000

              People

              • Assignee:
                shade Aleksey Shipilev
                Reporter:
                webbuggrp Webbug Group
              • Votes:
                0 Vote for this issue
                Watchers:
                5 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: