[7u4] Request for approval for CR 7088989 - Improve the performance for T4 by utilizing the newly provided crypto APIs

[Andrew Hughes]

----- Original Message -----
> Thanks, Valerie, the change is now retroactively approved.
> 
> cheers,
> dalibor topic
> 
> On 2/9/12 12:18 AM, Valerie (Yu-Ching) Peng wrote:
> > 
> > First, I apologize for missing this request approval process before
> > proceeding with the putback.
> > I mistakenly thought that this request approval process is not
> > necessary until a later stage.
> > 
> > The majority of changes for 7088989 utilizes Oracle internal APIs
> > and thus are under the src/closed/solaris tree which can't be
> > included in the following webrev. Thus, the webrev here only
> > contains the open part, i.e. public makefiles, as well as testing
> > sources.
> > The thread of review comments also aren't included here since they
> > are relevant to the closed sources, rather than the build and
> > testing files here.
> > 
> > Webrev:
> > http://cr.openjdk.java.net/~valeriep/7088989_7u/webrev.00/jdk/
> > 
> > Reviewer:
> > Max Weijun Wang
> > 
> > Thanks,
> > Valerie
> 
> 
> --
> 

I ran across this code today, after being asked on the IcedTea7 2.1 
release blog ([1]) about the differences between OpenJDK and the proprietary
Oracle JDK, and grepping through the Makefiles for ifndef/ifdef OPENJDK chunks.

I must say I'm very disappointed to see more features being added which aren't
being open sourced.  In the past, our aim has been to reduce the differences
between OpenJDK and the proprietary JDK, whereas this increases them.  It's
also disappointing to see that this patch wasn't publicly reviewed.  For jdk7u,
this seems to be in contravention of rule 4 [2]:

"Code reviews SHOULD take place on that list - if they take place somewhere else, as part
of the approval request a URL for the public code review MUST be provided."

I do have a number of technical questions about this too, which I would have asked
during the public review, had there been one:

1.  java.security is altered to prioritise com.oracle.security.ucrypto.UcryptoProvider
on Solaris builds.  What does this mean for OpenJDK builds on Solaris that don't have
this provider?  Will the provider code simply ignore absent providers?

+security.provider.1=com.oracle.security.ucrypto.UcryptoProvider ${java.home}/lib/security/ucrypto-solaris.cfg
+security.provider.2=sun.security.pkcs11.SunPKCS11 ${java.home}/lib/security/sunpkcs11-solaris.cfg

2.  The addition of the SunPKCS11 provider is of interest to us as we've been looking at
using this as the primary provider on IcedTea builds, following observed performance benefits:
see my recent blogs [3,4,5].  Was there a reason the testcases in this commit were locked down
to only work with the proprietary ucrypto provider?  Are they not suitable for all providers?
They would be very useful for testing the SunPKCS11/NSS provider.

[1] http://blog.fuseyism.com/index.php/2012/02/15/icedtea-2-1-released-openjdk7-u3-release/#comments
[2] http://openjdk.java.net/projects/jdk7u/groundrules.html
[3] http://blog.fuseyism.com/index.php/2012/03/09/openjdk-icedtea-nss/
[4] http://blog.fuseyism.com/index.php/2012/03/13/openjdk-icedtea-nss-icedtea7-results/
[5] http://blog.fuseyism.com/index.php/2012/03/14/openjdk-icedtea-nss-results-without-hardware-aes/

Thanks,
-- 
Andrew :)

Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)

PGP Key: 248BDC07 (https://keys.indymedia.org/)
Fingerprint = EC5A 1F5E C0AD 1D15 8F1F  8F91 3B96 A578 248B DC07