[8] code review request for 7165807: Non optimized initialization of NSS crypto library leads to scalability issues (original) (raw)

Valerie (Yu-Ching) Peng valerie.peng at oracle.com
Wed Jun 26 21:24:52 UTC 2013

Sure, zapping the nssInit() for only JDK 8 is fine. Being a private static method, I doubt that its removal will break anything though. Thanks, Valerie

On 06/26/13 11:38, Vincent Ryan wrote:

Thanks for the review Valerie. Comments below.

On 26 Jun 2013, at 00:23, Valerie (Yu-Ching) Peng wrote: <Secmod.java> - The private static native boolean nssInit(...) method should be removed? It seems obsolete by nssInitWithOptions(...) and I didn't see it being used anywhere. Same goes for the corresponding JNI method impl, i.e. Javasunsecuritypkcs11SecmodnssInit, in the j2secmod.c file. I considered zapping nssInit() but I wanted to ensure I didn't break anything in the JDK 7 Update release. I can remove it from the JDK 8 version.

<Config.java> - Particular reason to use different variable name, i.e. "nssUseOptimizeSpace" and property/option name "nssOptimizeSpace"? It seems inconsistent as most config variable name matches the property/option name. Can't we just use "nssOptimizeSpace"across board? The previous name of the SunPKCS11 configuration flag was 'nssUseOptimizeSpace'. I'll update all the internal references. <j2secmod.c> - line 127, empty string is used instead of the passed configDir. I am not sure if this correct. Why not just pass the configDir to this call? To be consistent with the NSS native code: it passes an empty string. Lastly, is the "NSSInitialize(...)" method always available for the supported NSS library versions, i.e. 3.7+? Is this a newer method meant to replace "NSSInit(...)"? It's available in addition to the NSSInit* functions (not a replacement).

Thanks, Valerie On 06/19/13 10:38, Vincent Ryan wrote: Thanks for the review. I've simplified the name of the NSS flag, updated the bug report and filed a doc bug, as you suggest.

On 19 Jun 2013, at 18:21, Sean Mullan wrote: Looks good, just a couple of comments: 1. I think the name "nssOptimizeSpace" is clearer. The "Use" part seems a bit odd in the property name. 2. Add the appropriate noreg label to the bug. 3. File a followup doc bug to document the attribute in the PKCS11 guide. --Sean On 06/19/2013 08:49 AM, Vincent Ryan wrote: I've made some corrections to the native method that initializes NSS. The new webrev is at: http://cr.openjdk.java.net/~vinnie/7165807/webrev.01

On 14 Jun 2013, at 18:38, Vincent Ryan wrote: Please review the following fix: http://cr.openjdk.java.net/~vinnie/7165807/webrev.00/ http://bugs.sun.com/viewbug.do?bugid=7165807 NSS may be initialized to favour performance or to favour memory footprint. This fix introduces a new configuration flag to allow Java applications to choose. By default, NSS will be initialized for performance. Thanks.

More information about the security-dev mailing list