changeset 59885:cc4aad546d9a

8247995: Avoid use of a mapping function in Permissions.getPermissionCollection Reviewed-by: rriggs
author redestad
date Tue, 23 Jun 2020 10:07:10 +0200
parents 0b6107622de4
children f37be32c7662
files src/java.base/share/classes/java/security/Permissions.java test/micro/org/openjdk/bench/java/security/PermissionsImplies.java
diffstat 2 files changed, 115 insertions(+), 26 deletions(-) [+]
line wrap: on
line diff
--- a/src/java.base/share/classes/java/security/Permissions.java	Tue Jun 23 09:38:21 2020 +0200
+++ b/src/java.base/share/classes/java/security/Permissions.java	Tue Jun 23 10:07:10 2020 +0200
@@ -32,7 +32,6 @@
 import java.io.ObjectStreamField;
 import java.io.Serializable;
 import java.util.Enumeration;
-import java.util.HashMap;
 import java.util.Hashtable;
 import java.util.Iterator;
 import java.util.List;
@@ -230,38 +229,50 @@
      */
     private PermissionCollection getPermissionCollection(Permission p,
                                                          boolean createEmpty) {
-        Class<?> c = p.getClass();
+        PermissionCollection pc = permsMap.get(p.getClass());
+        if ((!hasUnresolved && !createEmpty) || pc != null) {
+            // Collection not to be created, or already created
+            return pc;
+        }
+        return createPermissionCollection(p, createEmpty);
+    }
 
-        if (!hasUnresolved && !createEmpty) {
-            return permsMap.get(c);
-        }
+    private PermissionCollection createPermissionCollection(Permission p,
+                                                            boolean createEmpty) {
+        synchronized (permsMap) {
+            // Re-read under lock
+            Class<?> c = p.getClass();
+            PermissionCollection pc = permsMap.get(c);
 
-        // Create and add permission collection to map if it is absent.
-        // NOTE: cannot use lambda for mappingFunction parameter until
-        // JDK-8076596 is fixed.
-        return permsMap.computeIfAbsent(c,
-            new java.util.function.Function<>() {
-                @Override
-                public PermissionCollection apply(Class<?> k) {
-                    // Check for unresolved permissions
-                    PermissionCollection pc =
-                        (hasUnresolved ? getUnresolvedPermissions(p) : null);
+            // Collection already created
+            if (pc != null) {
+                return pc;
+            }
 
-                    // if still null, create a new collection
-                    if (pc == null && createEmpty) {
+            // Create and add permission collection to map if it is absent.
+            // Check for unresolved permissions
+            pc = (hasUnresolved ? getUnresolvedPermissions(p) : null);
 
-                        pc = p.newPermissionCollection();
+            // if still null, create a new collection
+            if (pc == null && createEmpty) {
 
-                        // still no PermissionCollection?
-                        // We'll give them a PermissionsHash.
-                        if (pc == null) {
-                            pc = new PermissionsHash();
-                        }
-                    }
-                    return pc;
+                pc = p.newPermissionCollection();
+
+                // still no PermissionCollection?
+                // We'll give them a PermissionsHash.
+                if (pc == null) {
+                    pc = new PermissionsHash();
                 }
             }
-        );
+            if (pc != null) {
+                // Add pc, resolving any race
+                PermissionCollection oldPc = permsMap.putIfAbsent(c, pc);
+                if (oldPc != null) {
+                    pc = oldPc;
+                }
+            }
+            return pc;
+        }
     }
 
     /**
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/micro/org/openjdk/bench/java/security/PermissionsImplies.java	Tue Jun 23 10:07:10 2020 +0200
@@ -0,0 +1,78 @@
+/*
+ * Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+package org.openjdk.bench.java.security;
+
+import org.openjdk.jmh.annotations.Benchmark;
+import org.openjdk.jmh.annotations.BenchmarkMode;
+import org.openjdk.jmh.annotations.Fork;
+import org.openjdk.jmh.annotations.Measurement;
+import org.openjdk.jmh.annotations.Mode;
+import org.openjdk.jmh.annotations.OutputTimeUnit;
+import org.openjdk.jmh.annotations.Scope;
+import org.openjdk.jmh.annotations.Setup;
+import org.openjdk.jmh.annotations.State;
+import org.openjdk.jmh.annotations.Warmup;
+
+import java.security.Permissions;
+import java.security.UnresolvedPermission;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * Benchmark measuring Permissions.implies
+ */
+@BenchmarkMode(Mode.AverageTime)
+@OutputTimeUnit(TimeUnit.NANOSECONDS)
+@Warmup(iterations = 10, time = 500, timeUnit = TimeUnit.MILLISECONDS)
+@Measurement(iterations = 5, time = 1000, timeUnit = TimeUnit.MILLISECONDS)
+@Fork(3)
+@State(Scope.Thread)
+public class PermissionsImplies {
+
+    private Permissions withPermission = new Permissions();
+    private Permissions withoutPermission = new Permissions();
+    private Permissions withUnresolvedPermission = new Permissions();
+
+    private RuntimePermission permission = new RuntimePermission("exitVM");
+
+    @Setup
+    public void setup() {
+        withPermission.add(permission);
+        withUnresolvedPermission.add(permission);
+        withUnresolvedPermission.add(new UnresolvedPermission("java.lang.FilePermission", "foo", "write", null));
+    }
+
+    @Benchmark
+    public boolean withoutPermission() {
+        return withoutPermission.implies(permission);
+    }
+
+    @Benchmark
+    public boolean withPermission() {
+        return withPermission.implies(permission);
+    }
+
+    @Benchmark
+    public boolean withUnresolvedPermission() {
+        return withUnresolvedPermission.implies(permission);
+    }
+}