RFR: 8247995: Avoid use of a mapping function in Permissions.getPermissionCollection

Claes Redestad claes.redestad at oracle.com
Mon Jun 22 22:29:21 UTC 2020


Hi,

inlining the new method actually messed up microbenchmark scores a
lot (15-80% overhead). I settled for simplifying the duplicate
permsMap.get in the outer method in a way that keeps scores neutral:

http://cr.openjdk.java.net/~redestad/8247995/open.01/

Benchmark                                    Mode  Cnt   Score   Error 
Units
PermissionsImplies.withPermission            avgt   15  27.857 ± 0.137 
ns/op
PermissionsImplies.withUnresolvedPermission  avgt   15  27.918 ± 0.178 
ns/op
PermissionsImplies.withoutPermission         avgt   15   4.559 ± 0.115 
ns/op

Thanks!

/Claes

On 2020-06-22 23:54, Claes Redestad wrote:
> Hi Roger,
> 
> I prototyped it as such, and saw a 0.4ns/op overhead in the
> PermissionImplies.withoutPermission. Thinking about it a bit now, this
> should be the rare path taken, since it means the permission is not
> implied and the performance largely irrelevant. I'll simplify as you 
> suggested.
> 
> /Claes
> 
> On 2020-06-22 23:03, Roger Riggs wrote:
>> Hi Claes,
>>
>> Its correct as is but I would have written it without duplicating the 
>> 'permsMap.get(p.getClass())' invocation
>> and as a single method (unless the inlining of 
>> getPermissionCollection(p,create)) is important.
>>
>> A patch on top of yours:
>>
>> diff --git 
>> a/src/java.base/share/classes/java/security/Permissions.java 
>> b/src/java.base/share/classes/java/security/Permissions.java
>> --- a/src/java.base/share/classes/java/security/Permissions.java
>> +++ b/src/java.base/share/classes/java/security/Permissions.java
>> @@ -229,20 +229,16 @@ implements Serializable
>>        */
>>       private PermissionCollection getPermissionCollection(Permission p,
>> boolean createEmpty) {
>> +        PermissionCollection pc = permsMap.get(p.getClass());
>>           if (!hasUnresolved && !createEmpty) {
>>               // Collection not to be created
>> -            return permsMap.get(p.getClass());
>> +            return pc;
>>           }
>> -        PermissionCollection pc = permsMap.get(p.getClass());
>>           if (pc != null) {
>>               // Collection already created
>>               return pc;
>>           }
>> -        return createPermissionCollection(p, createEmpty);
>> -    }
>>
>> -    private PermissionCollection 
>> createPermissionCollection(Permission p,
>> - boolean createEmpty) {
>>           synchronized (permsMap) {
>>               Class<?> c = p.getClass();
>>               PermissionCollection pc = permsMap.get(c);
>>
>> Thanks, Roger
>>
>>
>> On 6/22/20 11:04 AM, Claes Redestad wrote:
>>> Hi,
>>>
>>> this patch fixes a corner-case performance issue with
>>> Permissions.implies(Permission) by not needing to allocate a mapper
>>> function (or lambda) on each invocation of getPermissionCollection
>>> when there are unresolved permissions present.
>>>
>>> Bug:   https://bugs.openjdk.java.net/browse/JDK-8247995
>>> Patch: http://cr.openjdk.java.net/~redestad/8247995/open.00/
>>>
>>> Testing: tier1-2
>>>
>>> Thanks!
>>>
>>> /Claes
>>



More information about the security-dev mailing list