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

Store PermissionCollection entries in a ConcurrentHashMap instead of a HashMap in Permissions class

    Details

      Backports

        Description

        Currently, the code has several synchronized blocks around get/put operations to the HashMap. The policy provider caches a Permissions object per ProtectionDomain, so this can easily become a thread contention point when multiple threads are checking permissions.

        My benchmark with 8 threads which tests the performance of implies against a Permissions object containing a collection of different Permission objects shows about a 2x throughput improvement of Permissions.implies after I made this change. Prior to my change, a profiler showed that Permissions.implies was blocked 72% of the time. Afterwards, 0 %.

          Issue Links

            Activity

            Hide
            mullan Sean Mullan added a comment -
            The next point of thread contention to work on after this fix will be the various subclasses of Permission that implement their own PermissionCollections. These all show that threads are blocked on the implies method of the PermissionCollection subclasses implemented by these classes.

            The fix for this issue causes one new regression: test/java/security/PermissionCollection/Concurrent.java now fails with a ConcurrentModificationException, which is thrown by one of the PermissionCollection subclasses of the Permission subclasses. However, this is not really a regression. Removing the synchronization in Permissions has exposed an underlying bug in these Permission subclasses, in that the PermissionCollection.elements method is not supposed to be fail-fast (see the end of the PermissionCollection class summary for more info). It should not throw ConcurrentModificationException.
            Show
            mullan Sean Mullan added a comment - The next point of thread contention to work on after this fix will be the various subclasses of Permission that implement their own PermissionCollections. These all show that threads are blocked on the implies method of the PermissionCollection subclasses implemented by these classes. The fix for this issue causes one new regression: test/java/security/PermissionCollection/Concurrent.java now fails with a ConcurrentModificationException, which is thrown by one of the PermissionCollection subclasses of the Permission subclasses. However, this is not really a regression. Removing the synchronization in Permissions has exposed an underlying bug in these Permission subclasses, in that the PermissionCollection.elements method is not supposed to be fail-fast (see the end of the PermissionCollection class summary for more info). It should not throw ConcurrentModificationException.
            Hide
            mullan Sean Mullan added a comment -
            Throughput performance improvement of benchmark is roughly as follows (4 CPU system):

            1 thread : 8%
            8 threads : 89%
            Show
            mullan Sean Mullan added a comment - Throughput performance improvement of benchmark is roughly as follows (4 CPU system): 1 thread : 8% 8 threads : 89%
            Hide
            mullan Sean Mullan added a comment -
            Attached is a chart comparing the performance of Permissions.getPermissions(Permission) before and after the fix with various numbers of threads. The system under test was Intel® Core™ i5-2520M CPU @ 2.50GHz × 4 running Ubuntu 14.04 LTS. Single thread performance is better (by about 9%) and multiple-thread throughput is increased 2-4 times depending on the number of threads.
            Show
            mullan Sean Mullan added a comment - Attached is a chart comparing the performance of Permissions.getPermissions(Permission) before and after the fix with various numbers of threads. The system under test was Intel® Core™ i5-2520M CPU @ 2.50GHz × 4 running Ubuntu 14.04 LTS. Single thread performance is better (by about 9%) and multiple-thread throughput is increased 2-4 times depending on the number of threads.
            Hide
            hgupdate HG Updates added a comment -
            URL: http://hg.openjdk.java.net/jdk9/dev/jdk/rev/467094c6081b
            User: mullan
            Date: 2015-06-09 13:35:49 +0000
            Show
            hgupdate HG Updates added a comment - URL: http://hg.openjdk.java.net/jdk9/dev/jdk/rev/467094c6081b User: mullan Date: 2015-06-09 13:35:49 +0000
            Hide
            hgupdate HG Updates added a comment -
            URL: http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/467094c6081b
            User: lana
            Date: 2015-06-17 03:53:08 +0000
            Show
            hgupdate HG Updates added a comment - URL: http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/467094c6081b User: lana Date: 2015-06-17 03:53:08 +0000

              People

              • Assignee:
                mullan Sean Mullan
                Reporter:
                mullan Sean Mullan
              • Votes:
                0 Vote for this issue
                Watchers:
                2 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: