On 06/17/2015 03:27 AM, Daniel Fuchs wrote:
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'?
rlast seems ok to me. I will change it.
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.
Ok, I will remove those lines.
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 + ".")
Right, and this is an interesting observation. However, because it is a
behavior change and there may be potential compatibility issues, I think
we should open a separate issue to do that.
Any other comments, Max?
--Sean
(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
