RFR: 8072692: Improve performance of SecurityManager.checkPackageAccess

[Daniel Fuchs]

Hi Max,

On 6/17/15 2:42 AM, Weijun Wang wrote:
> 1478 final int plast = restrictedPkg.length() - 1;
>
> Why is it named plast?

Right. That's a bad name. Maybe 'rlast' would be more appropriate.
Any better name for 'the index of the last character in restrictedPkg'?

>
> 1494    //    - we check that restrictedPkg.length is pkg.length + 1,
> 1495    //    - we check that restrictedPkg starts with pkg,
> 1496    //    - and we check that the last character in restrictedPkg
> 1497    //      is '.'
>
> Seems redundant. The check below is not difficult to read.
>
> Also, is checking the "." at the end of restrictedPkg useful? On the 
> one hand we know every item in package.access should always end with 
> it. On the other hand, if someone really adds a "sun" there, the 1st 
> part of the check could go wrong (For example, "sunw" matches). IMHO, 
> either we don't check it at all (hoping property is always set 
> correctly), or we always check for it (cover both "sun.tail" and "sun").

Possibly - but that would be a behavioral change. The current test:

plast == plen && restrictedPkg.startsWith(pkg) &&  restrictedPkg.charAt(plast) == '.'


is strictly equivalent to the old test:

restrictedPkg.equals(pkg + ".")


(side note: pkg + "." was the root of the perf issue).


best regards,

-- daniel

>
> Thanks
> Max
>
> On 06/16/2015 10:54 PM, Sean Mullan wrote:
>> This is the sixth in a series of fixes for JEP 232 (Improve Secure
>> Application Performance) [1].
>>
>> webrev: http://cr.openjdk.java.net/~mullan/webrevs/8072692/webrev.00/
>> bug: https://bugs.openjdk.java.net/browse/JDK-8072692
>>
>> This fix adds several optimizations to the package matching algorithm
>> used by the SecurityManager.checkPackageAcccess method. These
>> improvements result in a 5-7x increase in throughput of this method. A
>> performance chart has been attached to the bug with more information.
>>
>> A new test is included which uses a state machine to verify that the
>> matching algorithm is working correctly.
>>
>> Special thanks to Daniel Fuchs for contributing this fix and the test.
>>
>> Thanks,
>> Sean
>>
>> [1] http://openjdk.java.net/jeps/232

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/security-dev/attachments/20150617/950db004/attachment.htm>