Re: RFR: 8073108: GHASH Intrinsics [need second reviewer]

2015-06-17 Thread John Rose
On Jun 17, 2015, at 10:40 AM, Anthony Scarpino anthony.scarp...@oracle.com wrote: On 06/15/2015 05:20 PM, John Rose wrote: Thanks for taking this on. It looks good, except for one thing. The intrinsic does not need to be an instance method, and doing so creates an undesirable coupling

Re: RFR 8064890: SecureClassLoader should use a ConcurrentHashMap

2015-06-17 Thread Sean Mullan
On line 222, I would probably name the parameter key instead of dummy since that is what it is, even though it is not used. Looks good otherwise. Thanks, Sean On 06/17/2015 12:25 PM, Weijun Wang wrote: This fix is also a part for JEP 232 (Improve Secure Application Performance) [1]. webrev:

Re: RFR: 8073108: GHASH Intrinsics [need second reviewer]

2015-06-17 Thread Anthony Scarpino
On 06/15/2015 05:20 PM, John Rose wrote: Thanks for taking this on. It looks good, except for one thing. The intrinsic does not need to be an instance method, and doing so creates an undesirable coupling between the JVM and JDK. Specifically, the JDK should not need to know about subkeyH and

RFR: 8060103: CheckBlacklistedCerts.java thinks its openjdk build

2015-06-17 Thread Rajan Halade
May I request you to review small fix to CheckBlacklistedCerts test to correctly determine if OpenJDK build is used. Webrev: http://cr.openjdk.java.net/~rhalade/8060103/webrev.00/ Bug: https://bugs.openjdk.java.net/browse/JDK-8060103 Thanks, Rajan

Re: RFR: 8073108: GHASH Intrinsics [need second reviewer]

2015-06-17 Thread John Rose
Thumbs up. Thanks! — John On Jun 17, 2015, at 3:31 PM, Anthony Scarpino anthony.scarp...@oracle.com wrote: On 06/15/2015 05:20 PM, John Rose wrote: Thanks for taking this on. It looks good, except for one thing. The intrinsic does not need to be an instance method, and doing so

Re: RFR: 8073108: GHASH Intrinsics [need second reviewer]

2015-06-17 Thread Vladimir Kozlov
Looks good to me too. Please, push into jdk9/hs-comp so we get testing of intrinsic in Hotspot nightly testing. Thanks, Vladimir On 6/17/15 3:31 PM, Anthony Scarpino wrote: On 06/15/2015 05:20 PM, John Rose wrote: Thanks for taking this on. It looks good, except for one thing. The

Re: RFR: 8060103: CheckBlacklistedCerts.java thinks its openjdk build

2015-06-17 Thread Weijun Wang
Change looks fine. Thanks Max On 6/18/2015 7:00 AM, Rajan Halade wrote: May I request you to review small fix to CheckBlacklistedCerts test to correctly determine if OpenJDK build is used. Webrev: http://cr.openjdk.java.net/~rhalade/8060103/webrev.00/ Bug:

Re: RFR: 8073108: GHASH Intrinsics [need second reviewer]

2015-06-17 Thread Anthony Scarpino
On 06/15/2015 05:20 PM, John Rose wrote: Thanks for taking this on. It looks good, except for one thing. The intrinsic does not need to be an instance method, and doing so creates an undesirable coupling between the JVM and JDK. Specifically, the JDK should not need to know about subkeyH and

Re: RFR: 8072692: Improve performance of SecurityManager.checkPackageAccess

2015-06-17 Thread Sean Mullan
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

Re: RFR: 8072692: Improve performance of SecurityManager.checkPackageAccess

2015-06-17 Thread 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

Re: RFR 7191662: JCE providers should be located via ServiceLoader

2015-06-17 Thread Alan Bateman
On 16/06/2015 00:58, Valerie Peng wrote: It seems that the jimage refresh change may still take some time, so we will end up removing the makefile related changes and then deferring the ServiceLoader related changes to Jake workspace. Here is the latest webrev (Security source/test changes

Re: RFR: 8072692: Improve performance of SecurityManager.checkPackageAccess

2015-06-17 Thread Weijun Wang
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 + .) Yes, I understand. Right, and this is an interesting observation.

RFR 8064890: SecureClassLoader should use a ConcurrentHashMap

2015-06-17 Thread Weijun Wang
This fix is also a part for JEP 232 (Improve Secure Application Performance) [1]. webrev: http://cr.openjdk.java.net/~weijun/8064890/webrev.00/ bug: https://bugs.openjdk.java.net/browse/JDK-8064890 The fix uses a ConcurrentHashMap to avoid synchronization. Note: 1. ConcurrentHashMap does not