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

[Claes Redestad]

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
>