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

Roger Riggs Roger.Riggs at oracle.com
Mon Jun 22 22:30:50 UTC 2020


Hi Claes,

Looks good.

Thanks for the followup.

Roger

On 6/22/20 6:29 PM, Claes Redestad wrote:
> 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