Re: [jdk16] RFR: JDK-8259732: JDK 16 L10n resource file update - msg drop 10 [v2]
On Fri, 15 Jan 2021 01:59:07 GMT, Leo Jiang wrote:
>> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/resources/standard.properties
>> line 518:
>>
>>> 516: doclet.footer_specified=\
>>> 517: The -footer option is no longer supported and will be ignored.\n\
>>> 518: It may be removed in a future release.
>>
>> I believe this is to fix no newline at the end (unrelated to l10n changes).
>> Do we need to change the copyright year for this?
>
> Actually I was correcting the L217, changed {) -> {}. But 2 days ago,
> Jonathan Gibbons fixed it in another commit 6bb6093. I found his fix after
> running the merge. Looks both of us forgot to update the copyright year. Any
> suggestion?
> doclet.help.systemProperties.body=\
> The {0} page lists references to system properties.
I believe your PR still contains the fix to add the newline at the end of the
file (at L518). So I think you can simply change the copyright year in
`standard.properties` file.
-
PR: https://git.openjdk.java.net/jdk16/pull/123
Re: RFR: 8259498: Reduce overhead of MD5 and SHA digests [v2]
On Fri, 15 Jan 2021 23:36:35 GMT, Claes Redestad wrote: >> - The MD5 intrinsics added by >> [JDK-8250902](https://bugs.openjdk.java.net/browse/JDK-8250902) shows that >> the `int[] x` isn't actually needed. This also applies to the SHA intrinsics >> from which the MD5 intrinsic takes inspiration >> - Using VarHandles we can simplify the code in `ByteArrayAccess` enough to >> make it acceptable to use inline and replace the array in MD5 wholesale. >> This improves performance both in the presence and the absence of the >> intrinsic optimization. >> - Doing the exact same thing in the SHA impls would be unwieldy (64+ element >> arrays), but allocating the array lazily gets most of the speed-up in the >> presence of an intrinsic while being neutral in its absence. >> >> Baseline: >> (digesterName) (length)Cnt Score >> Error Units >> MessageDigests.digestMD516 15 >> 2714.307 ± 21.133 ops/ms >> MessageDigests.digestMD5 1024 15 >> 318.087 ±0.637 ops/ms >> MessageDigests.digest SHA-116 15 >> 1387.266 ± 40.932 ops/ms >> MessageDigests.digest SHA-1 1024 15 >> 109.273 ±0.149 ops/ms >> MessageDigests.digestSHA-25616 15 >> 995.566 ± 21.186 ops/ms >> MessageDigests.digestSHA-256 1024 15 >> 89.104 ±0.079 ops/ms >> MessageDigests.digestSHA-51216 15 >> 803.030 ± 15.722 ops/ms >> MessageDigests.digestSHA-512 1024 15 >> 115.611 ±0.234 ops/ms >> MessageDigests.getAndDigest MD516 15 >> 2190.367 ± 97.037 ops/ms >> MessageDigests.getAndDigest MD5 1024 15 >> 302.903 ±1.809 ops/ms >> MessageDigests.getAndDigestSHA-116 15 >> 1262.656 ± 43.751 ops/ms >> MessageDigests.getAndDigestSHA-1 1024 15 >> 104.889 ±3.554 ops/ms >> MessageDigests.getAndDigest SHA-25616 15 >> 914.541 ± 55.621 ops/ms >> MessageDigests.getAndDigest SHA-256 1024 15 >> 85.708 ±1.394 ops/ms >> MessageDigests.getAndDigest SHA-51216 15 >> 737.719 ± 53.671 ops/ms >> MessageDigests.getAndDigest SHA-512 1024 15 >> 112.307 ±1.950 ops/ms >> >> GC: >> MessageDigests.getAndDigest:·gc.alloc.rate.norm MD516 15 >> 312.011 ±0.005B/op >> MessageDigests.getAndDigest:·gc.alloc.rate.normSHA-116 15 >> 584.020 ±0.006B/op >> MessageDigests.getAndDigest:·gc.alloc.rate.norm SHA-25616 15 >> 544.019 ±0.016B/op >> MessageDigests.getAndDigest:·gc.alloc.rate.norm SHA-51216 15 >> 1056.037 ±0.003B/op >> >> Target: >> Benchmark (digesterName) (length)Cnt >> Score Error Units >> MessageDigests.digestMD516 15 >> 3134.462 ± 43.685 ops/ms >> MessageDigests.digestMD5 1024 15 >> 323.667 ±0.633 ops/ms >> MessageDigests.digest SHA-116 15 >> 1418.742 ± 38.223 ops/ms >> MessageDigests.digest SHA-1 1024 15 >> 110.178 ±0.788 ops/ms >> MessageDigests.digestSHA-25616 15 >> 1037.949 ± 21.214 ops/ms >> MessageDigests.digestSHA-256 1024 15 >> 89.671 ±0.228 ops/ms >> MessageDigests.digestSHA-51216 15 >> 812.028 ± 39.489 ops/ms >> MessageDigests.digestSHA-512 1024 15 >> 116.738 ±0.249 ops/ms >> MessageDigests.getAndDigest MD516 15 >> 2314.379 ± 229.294 ops/ms >> MessageDigests.getAndDigest MD5 1024 15 >> 307.835 ±5.730 ops/ms >> MessageDigests.getAndDigestSHA-116 15 >> 1326.887 ± 63.263 ops/ms >> MessageDigests.getAndDigestSHA-1 1024 15 >> 106.611 ±2.292 ops/ms >> MessageDigests.getAndDigest SHA-25616 15 >> 961.589 ± 82.052 ops/ms >> MessageDigests.getAndDigest SHA-256 1024 15 >> 88.646 ±0.194 ops/ms >> MessageDigests.getAndDigest SHA-51216 15 >> 775.417 ± 56.775 ops/ms >> MessageDigests.getAndDigest SHA-512 1024 15 >> 112.904 ±2.014 ops/ms >>
Re: RFR: 8259498: Reduce overhead of MD5 and SHA digests [v2]
> - The MD5 intrinsics added by > [JDK-8250902](https://bugs.openjdk.java.net/browse/JDK-8250902) shows that > the `int[] x` isn't actually needed. This also applies to the SHA intrinsics > from which the MD5 intrinsic takes inspiration > - Using VarHandles we can simplify the code in `ByteArrayAccess` enough to > make it acceptable to use inline and replace the array in MD5 wholesale. This > improves performance both in the presence and the absence of the intrinsic > optimization. > - Doing the exact same thing in the SHA impls would be unwieldy (64+ element > arrays), but allocating the array lazily gets most of the speed-up in the > presence of an intrinsic while being neutral in its absence. > > Baseline: > (digesterName) (length)Cnt Score > Error Units > MessageDigests.digestMD516 15 > 2714.307 ± 21.133 ops/ms > MessageDigests.digestMD5 1024 15 > 318.087 ±0.637 ops/ms > MessageDigests.digest SHA-116 15 > 1387.266 ± 40.932 ops/ms > MessageDigests.digest SHA-1 1024 15 > 109.273 ±0.149 ops/ms > MessageDigests.digestSHA-25616 15 > 995.566 ± 21.186 ops/ms > MessageDigests.digestSHA-256 1024 15 > 89.104 ±0.079 ops/ms > MessageDigests.digestSHA-51216 15 > 803.030 ± 15.722 ops/ms > MessageDigests.digestSHA-512 1024 15 > 115.611 ±0.234 ops/ms > MessageDigests.getAndDigest MD516 15 > 2190.367 ± 97.037 ops/ms > MessageDigests.getAndDigest MD5 1024 15 > 302.903 ±1.809 ops/ms > MessageDigests.getAndDigestSHA-116 15 > 1262.656 ± 43.751 ops/ms > MessageDigests.getAndDigestSHA-1 1024 15 > 104.889 ±3.554 ops/ms > MessageDigests.getAndDigest SHA-25616 15 > 914.541 ± 55.621 ops/ms > MessageDigests.getAndDigest SHA-256 1024 15 > 85.708 ±1.394 ops/ms > MessageDigests.getAndDigest SHA-51216 15 > 737.719 ± 53.671 ops/ms > MessageDigests.getAndDigest SHA-512 1024 15 > 112.307 ±1.950 ops/ms > > GC: > MessageDigests.getAndDigest:·gc.alloc.rate.norm MD516 15 > 312.011 ±0.005B/op > MessageDigests.getAndDigest:·gc.alloc.rate.normSHA-116 15 > 584.020 ±0.006B/op > MessageDigests.getAndDigest:·gc.alloc.rate.norm SHA-25616 15 > 544.019 ±0.016B/op > MessageDigests.getAndDigest:·gc.alloc.rate.norm SHA-51216 15 > 1056.037 ±0.003B/op > > Target: > Benchmark (digesterName) (length)Cnt > Score Error Units > MessageDigests.digestMD516 15 > 3134.462 ± 43.685 ops/ms > MessageDigests.digestMD5 1024 15 > 323.667 ±0.633 ops/ms > MessageDigests.digest SHA-116 15 > 1418.742 ± 38.223 ops/ms > MessageDigests.digest SHA-1 1024 15 > 110.178 ±0.788 ops/ms > MessageDigests.digestSHA-25616 15 > 1037.949 ± 21.214 ops/ms > MessageDigests.digestSHA-256 1024 15 > 89.671 ±0.228 ops/ms > MessageDigests.digestSHA-51216 15 > 812.028 ± 39.489 ops/ms > MessageDigests.digestSHA-512 1024 15 > 116.738 ±0.249 ops/ms > MessageDigests.getAndDigest MD516 15 > 2314.379 ± 229.294 ops/ms > MessageDigests.getAndDigest MD5 1024 15 > 307.835 ±5.730 ops/ms > MessageDigests.getAndDigestSHA-116 15 > 1326.887 ± 63.263 ops/ms > MessageDigests.getAndDigestSHA-1 1024 15 > 106.611 ±2.292 ops/ms > MessageDigests.getAndDigest SHA-25616 15 > 961.589 ± 82.052 ops/ms > MessageDigests.getAndDigest SHA-256 1024 15 > 88.646 ±0.194 ops/ms > MessageDigests.getAndDigest SHA-51216 15 > 775.417 ± 56.775 ops/ms > MessageDigests.getAndDigest SHA-512 1024 15 > 112.904 ±2.014 ops/ms > > GC > MessageDigests.getAndDigest:·gc.alloc.rate.norm MD516 15 > 232.009 ±0.006B/op >
Re: RFR: 8259498: Reduce overhead of MD5 and SHA digests [v2]
On Fri, 15 Jan 2021 23:21:00 GMT, Valerie Peng wrote: >> Claes Redestad has updated the pull request with a new target base due to a >> merge or a rebase. The incremental webrev excludes the unrelated changes >> brought in by the merge/rebase. The pull request contains 20 additional >> commits since the last revision: >> >> - Copyrights >> - Merge branch 'master' into improve_md5 >> - Remove unused Unsafe import >> - Harmonize MD4 impl, remove now-redundant checks from ByteArrayAccess (VHs >> do bounds checks, most of which will be optimized away) >> - Merge branch 'master' into improve_md5 >> - Apply allocation avoiding optimizations to all SHA versions sharing >> structural similarities with MD5 >> - Remove unused reverseBytes imports >> - Copyrights >> - Fix copy-paste error >> - Various fixes (IDE stopped IDEing..) >> - ... and 10 more: >> https://git.openjdk.java.net/jdk/compare/6e03c8d3...cafa3e49 > > test/micro/org/openjdk/bench/java/util/UUIDBench.java line 2: > >> 1: /* >> 2: * Copyright (c) 2020, 2021, Oracle and/or its affiliates. All rights >> reserved. > > nit: other files should also have this 2021 update. It seems most of them are > not updated and still uses 2020. fixed - PR: https://git.openjdk.java.net/jdk/pull/1855
Re: RFR: 8259498: Reduce overhead of MD5 and SHA digests
On Sun, 20 Dec 2020 20:27:03 GMT, Claes Redestad wrote: > - The MD5 intrinsics added by > [JDK-8250902](https://bugs.openjdk.java.net/browse/JDK-8250902) shows that > the `int[] x` isn't actually needed. This also applies to the SHA intrinsics > from which the MD5 intrinsic takes inspiration > - Using VarHandles we can simplify the code in `ByteArrayAccess` enough to > make it acceptable to use inline and replace the array in MD5 wholesale. This > improves performance both in the presence and the absence of the intrinsic > optimization. > - Doing the exact same thing in the SHA impls would be unwieldy (64+ element > arrays), but allocating the array lazily gets most of the speed-up in the > presence of an intrinsic while being neutral in its absence. > > Baseline: > (digesterName) (length)Cnt Score > Error Units > MessageDigests.digestMD516 15 > 2714.307 ± 21.133 ops/ms > MessageDigests.digestMD5 1024 15 > 318.087 ±0.637 ops/ms > MessageDigests.digest SHA-116 15 > 1387.266 ± 40.932 ops/ms > MessageDigests.digest SHA-1 1024 15 > 109.273 ±0.149 ops/ms > MessageDigests.digestSHA-25616 15 > 995.566 ± 21.186 ops/ms > MessageDigests.digestSHA-256 1024 15 > 89.104 ±0.079 ops/ms > MessageDigests.digestSHA-51216 15 > 803.030 ± 15.722 ops/ms > MessageDigests.digestSHA-512 1024 15 > 115.611 ±0.234 ops/ms > MessageDigests.getAndDigest MD516 15 > 2190.367 ± 97.037 ops/ms > MessageDigests.getAndDigest MD5 1024 15 > 302.903 ±1.809 ops/ms > MessageDigests.getAndDigestSHA-116 15 > 1262.656 ± 43.751 ops/ms > MessageDigests.getAndDigestSHA-1 1024 15 > 104.889 ±3.554 ops/ms > MessageDigests.getAndDigest SHA-25616 15 > 914.541 ± 55.621 ops/ms > MessageDigests.getAndDigest SHA-256 1024 15 > 85.708 ±1.394 ops/ms > MessageDigests.getAndDigest SHA-51216 15 > 737.719 ± 53.671 ops/ms > MessageDigests.getAndDigest SHA-512 1024 15 > 112.307 ±1.950 ops/ms > > GC: > MessageDigests.getAndDigest:·gc.alloc.rate.norm MD516 15 > 312.011 ±0.005B/op > MessageDigests.getAndDigest:·gc.alloc.rate.normSHA-116 15 > 584.020 ±0.006B/op > MessageDigests.getAndDigest:·gc.alloc.rate.norm SHA-25616 15 > 544.019 ±0.016B/op > MessageDigests.getAndDigest:·gc.alloc.rate.norm SHA-51216 15 > 1056.037 ±0.003B/op > > Target: > Benchmark (digesterName) (length)Cnt > Score Error Units > MessageDigests.digestMD516 15 > 3134.462 ± 43.685 ops/ms > MessageDigests.digestMD5 1024 15 > 323.667 ±0.633 ops/ms > MessageDigests.digest SHA-116 15 > 1418.742 ± 38.223 ops/ms > MessageDigests.digest SHA-1 1024 15 > 110.178 ±0.788 ops/ms > MessageDigests.digestSHA-25616 15 > 1037.949 ± 21.214 ops/ms > MessageDigests.digestSHA-256 1024 15 > 89.671 ±0.228 ops/ms > MessageDigests.digestSHA-51216 15 > 812.028 ± 39.489 ops/ms > MessageDigests.digestSHA-512 1024 15 > 116.738 ±0.249 ops/ms > MessageDigests.getAndDigest MD516 15 > 2314.379 ± 229.294 ops/ms > MessageDigests.getAndDigest MD5 1024 15 > 307.835 ±5.730 ops/ms > MessageDigests.getAndDigestSHA-116 15 > 1326.887 ± 63.263 ops/ms > MessageDigests.getAndDigestSHA-1 1024 15 > 106.611 ±2.292 ops/ms > MessageDigests.getAndDigest SHA-25616 15 > 961.589 ± 82.052 ops/ms > MessageDigests.getAndDigest SHA-256 1024 15 > 88.646 ±0.194 ops/ms > MessageDigests.getAndDigest SHA-51216 15 > 775.417 ± 56.775 ops/ms > MessageDigests.getAndDigest SHA-512 1024 15 > 112.904 ±2.014 ops/ms > > GC > MessageDigests.getAndDigest:·gc.alloc.rate.norm MD516 15 > 232.009 ±
Re: RFR: 8259498: Reduce overhead of MD5 and SHA digests
On Fri, 15 Jan 2021 22:54:32 GMT, Valerie Peng wrote: >> - The MD5 intrinsics added by >> [JDK-8250902](https://bugs.openjdk.java.net/browse/JDK-8250902) shows that >> the `int[] x` isn't actually needed. This also applies to the SHA intrinsics >> from which the MD5 intrinsic takes inspiration >> - Using VarHandles we can simplify the code in `ByteArrayAccess` enough to >> make it acceptable to use inline and replace the array in MD5 wholesale. >> This improves performance both in the presence and the absence of the >> intrinsic optimization. >> - Doing the exact same thing in the SHA impls would be unwieldy (64+ element >> arrays), but allocating the array lazily gets most of the speed-up in the >> presence of an intrinsic while being neutral in its absence. >> >> Baseline: >> (digesterName) (length)Cnt Score >> Error Units >> MessageDigests.digestMD516 15 >> 2714.307 ± 21.133 ops/ms >> MessageDigests.digestMD5 1024 15 >> 318.087 ±0.637 ops/ms >> MessageDigests.digest SHA-116 15 >> 1387.266 ± 40.932 ops/ms >> MessageDigests.digest SHA-1 1024 15 >> 109.273 ±0.149 ops/ms >> MessageDigests.digestSHA-25616 15 >> 995.566 ± 21.186 ops/ms >> MessageDigests.digestSHA-256 1024 15 >> 89.104 ±0.079 ops/ms >> MessageDigests.digestSHA-51216 15 >> 803.030 ± 15.722 ops/ms >> MessageDigests.digestSHA-512 1024 15 >> 115.611 ±0.234 ops/ms >> MessageDigests.getAndDigest MD516 15 >> 2190.367 ± 97.037 ops/ms >> MessageDigests.getAndDigest MD5 1024 15 >> 302.903 ±1.809 ops/ms >> MessageDigests.getAndDigestSHA-116 15 >> 1262.656 ± 43.751 ops/ms >> MessageDigests.getAndDigestSHA-1 1024 15 >> 104.889 ±3.554 ops/ms >> MessageDigests.getAndDigest SHA-25616 15 >> 914.541 ± 55.621 ops/ms >> MessageDigests.getAndDigest SHA-256 1024 15 >> 85.708 ±1.394 ops/ms >> MessageDigests.getAndDigest SHA-51216 15 >> 737.719 ± 53.671 ops/ms >> MessageDigests.getAndDigest SHA-512 1024 15 >> 112.307 ±1.950 ops/ms >> >> GC: >> MessageDigests.getAndDigest:·gc.alloc.rate.norm MD516 15 >> 312.011 ±0.005B/op >> MessageDigests.getAndDigest:·gc.alloc.rate.normSHA-116 15 >> 584.020 ±0.006B/op >> MessageDigests.getAndDigest:·gc.alloc.rate.norm SHA-25616 15 >> 544.019 ±0.016B/op >> MessageDigests.getAndDigest:·gc.alloc.rate.norm SHA-51216 15 >> 1056.037 ±0.003B/op >> >> Target: >> Benchmark (digesterName) (length)Cnt >> Score Error Units >> MessageDigests.digestMD516 15 >> 3134.462 ± 43.685 ops/ms >> MessageDigests.digestMD5 1024 15 >> 323.667 ±0.633 ops/ms >> MessageDigests.digest SHA-116 15 >> 1418.742 ± 38.223 ops/ms >> MessageDigests.digest SHA-1 1024 15 >> 110.178 ±0.788 ops/ms >> MessageDigests.digestSHA-25616 15 >> 1037.949 ± 21.214 ops/ms >> MessageDigests.digestSHA-256 1024 15 >> 89.671 ±0.228 ops/ms >> MessageDigests.digestSHA-51216 15 >> 812.028 ± 39.489 ops/ms >> MessageDigests.digestSHA-512 1024 15 >> 116.738 ±0.249 ops/ms >> MessageDigests.getAndDigest MD516 15 >> 2314.379 ± 229.294 ops/ms >> MessageDigests.getAndDigest MD5 1024 15 >> 307.835 ±5.730 ops/ms >> MessageDigests.getAndDigestSHA-116 15 >> 1326.887 ± 63.263 ops/ms >> MessageDigests.getAndDigestSHA-1 1024 15 >> 106.611 ±2.292 ops/ms >> MessageDigests.getAndDigest SHA-25616 15 >> 961.589 ± 82.052 ops/ms >> MessageDigests.getAndDigest SHA-256 1024 15 >> 88.646 ±0.194 ops/ms >> MessageDigests.getAndDigest SHA-51216 15 >> 775.417 ± 56.775 ops/ms >> MessageDigests.getAndDigest SHA-512 1024 15 >> 112.904 ±2.014 ops/ms >> >>
Re: RFR: 8256895: Add support for RFC 8954: Online Certificate Status Protocol (OCSP) Nonce Extension [v2]
On Thu, 14 Jan 2021 14:35:25 GMT, Sean Mullan wrote:
>> Hai-May Chao has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> update to use List.of() and typo changes
>
> src/java.base/share/classes/sun/security/provider/certpath/RevocationChecker.java
> line 88:
>
>> 86: boolean ocspNonce;
>> 87: }
>> 88: private RevocationProperties rp;
>
> I think this field could be `final`.
No change made due to getting an error: cannot assign a value to final variable
rp.
> src/java.base/share/classes/sun/security/provider/certpath/RevocationChecker.java
> line 107:
>
>> 105:
>> 106: private void setDefaultNonce() {
>> 107: byte[] nonce = null;
>
> This variable looks like it is not used and can be removed.
The setDefaultNonce() method is removed.
> src/java.base/share/classes/sun/security/provider/certpath/RevocationChecker.java
> line 109:
>
>> 107: byte[] nonce = null;
>> 108:
>> 109: // Set the nonce by default in OCSP request extension when the
>> sytem property
>
> Typo: s/sytem/system/
The setDefaultNonce() method is removed as creating nonce is done in
checkOCSP() method now.
> src/java.base/share/classes/sun/security/provider/certpath/RevocationChecker.java
> line 113:
>
>> 111: if (rp.ocspNonce) {
>> 112: try {
>> 113: setOcspExtensions(List.of(new
>> OCSPNonceExtension(DEFAULT_NONCE_BYTES)));
>
> I don't think we should use the `PKIXRevocationChecker.setOcspExtensions()`
> API to add an OCSP Nonce extension. That API is intended to be called by
> applications. I think the Nonce extension should be set by the implementation
> only and not exposed via the standard API. Also, a nonce should be unique for
> each OCSP request, but setting it here means that it could re-use the same
> nonce for different OCSP requests.
>
> I think a better place to create/add the OCSPExtension is in the `checkOCSP`
> method, and the extension should be created each time that method is called
> (if the system property is enabled), so a new nonce is created for each OCSP
> request.
Creating the nonce is moved to checkOCSP() method.
-
PR: https://git.openjdk.java.net/jdk/pull/2039
Re: RFR: 8023980: JCE doesn't provide any class to handle RSA private key in PKCS#1 [v3]
On Thu, 14 Jan 2021 20:04:33 GMT, Valerie Peng wrote:
>> src/java.base/share/classes/sun/security/rsa/RSAKeyFactory.java line 344:
>>
>>> 342: if (keySpec instanceof PKCS8EncodedKeySpec) {
>>> 343: return RSAPrivateCrtKeyImpl.newKey(type, "PKCS#8",
>>> 344: ((PKCS8EncodedKeySpec)keySpec).getEncoded());
>>
>> Will you clean up the `getEncoded()` output or shall I?
>
> Maybe it's better that you do it this time? Just so that the backport won't
> miss it.
Or if you integrated before me, I will manually merge the changes and clean up
the getEncoded() also.
-
PR: https://git.openjdk.java.net/jdk/pull/1787
Re: RFR: 8256895: Add support for RFC 8954: Online Certificate Status Protocol (OCSP) Nonce Extension [v4]
> This enhancement adds support for the nonce extension in OCSP request > extensions by system property jdk.security.certpath.ocspNonce. > > Please review the CSR at: > https://bugs.openjdk.java.net/browse/JDK-8257766 Hai-May Chao has updated the pull request incrementally with one additional commit since the last revision: Nonce creation is done in checkOCSP method - Changes: - all: https://git.openjdk.java.net/jdk/pull/2039/files - new: https://git.openjdk.java.net/jdk/pull/2039/files/949ee2be..da6e5bab Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2039=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2039=02-03 Stats: 68 lines in 1 file changed: 36 ins; 31 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/2039.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2039/head:pull/2039 PR: https://git.openjdk.java.net/jdk/pull/2039
Re: RFR: 8259498: Reduce overhead of MD5 and SHA digests
On Sun, 20 Dec 2020 20:27:03 GMT, Claes Redestad wrote: > - The MD5 intrinsics added by > [JDK-8250902](https://bugs.openjdk.java.net/browse/JDK-8250902) shows that > the `int[] x` isn't actually needed. This also applies to the SHA intrinsics > from which the MD5 intrinsic takes inspiration > - Using VarHandles we can simplify the code in `ByteArrayAccess` enough to > make it acceptable to use inline and replace the array in MD5 wholesale. This > improves performance both in the presence and the absence of the intrinsic > optimization. > - Doing the exact same thing in the SHA impls would be unwieldy (64+ element > arrays), but allocating the array lazily gets most of the speed-up in the > presence of an intrinsic while being neutral in its absence. > > Baseline: > (digesterName) (length)Cnt Score > Error Units > MessageDigests.digestMD516 15 > 2714.307 ± 21.133 ops/ms > MessageDigests.digestMD5 1024 15 > 318.087 ±0.637 ops/ms > MessageDigests.digest SHA-116 15 > 1387.266 ± 40.932 ops/ms > MessageDigests.digest SHA-1 1024 15 > 109.273 ±0.149 ops/ms > MessageDigests.digestSHA-25616 15 > 995.566 ± 21.186 ops/ms > MessageDigests.digestSHA-256 1024 15 > 89.104 ±0.079 ops/ms > MessageDigests.digestSHA-51216 15 > 803.030 ± 15.722 ops/ms > MessageDigests.digestSHA-512 1024 15 > 115.611 ±0.234 ops/ms > MessageDigests.getAndDigest MD516 15 > 2190.367 ± 97.037 ops/ms > MessageDigests.getAndDigest MD5 1024 15 > 302.903 ±1.809 ops/ms > MessageDigests.getAndDigestSHA-116 15 > 1262.656 ± 43.751 ops/ms > MessageDigests.getAndDigestSHA-1 1024 15 > 104.889 ±3.554 ops/ms > MessageDigests.getAndDigest SHA-25616 15 > 914.541 ± 55.621 ops/ms > MessageDigests.getAndDigest SHA-256 1024 15 > 85.708 ±1.394 ops/ms > MessageDigests.getAndDigest SHA-51216 15 > 737.719 ± 53.671 ops/ms > MessageDigests.getAndDigest SHA-512 1024 15 > 112.307 ±1.950 ops/ms > > GC: > MessageDigests.getAndDigest:·gc.alloc.rate.norm MD516 15 > 312.011 ±0.005B/op > MessageDigests.getAndDigest:·gc.alloc.rate.normSHA-116 15 > 584.020 ±0.006B/op > MessageDigests.getAndDigest:·gc.alloc.rate.norm SHA-25616 15 > 544.019 ±0.016B/op > MessageDigests.getAndDigest:·gc.alloc.rate.norm SHA-51216 15 > 1056.037 ±0.003B/op > > Target: > Benchmark (digesterName) (length)Cnt > Score Error Units > MessageDigests.digestMD516 15 > 3134.462 ± 43.685 ops/ms > MessageDigests.digestMD5 1024 15 > 323.667 ±0.633 ops/ms > MessageDigests.digest SHA-116 15 > 1418.742 ± 38.223 ops/ms > MessageDigests.digest SHA-1 1024 15 > 110.178 ±0.788 ops/ms > MessageDigests.digestSHA-25616 15 > 1037.949 ± 21.214 ops/ms > MessageDigests.digestSHA-256 1024 15 > 89.671 ±0.228 ops/ms > MessageDigests.digestSHA-51216 15 > 812.028 ± 39.489 ops/ms > MessageDigests.digestSHA-512 1024 15 > 116.738 ±0.249 ops/ms > MessageDigests.getAndDigest MD516 15 > 2314.379 ± 229.294 ops/ms > MessageDigests.getAndDigest MD5 1024 15 > 307.835 ±5.730 ops/ms > MessageDigests.getAndDigestSHA-116 15 > 1326.887 ± 63.263 ops/ms > MessageDigests.getAndDigestSHA-1 1024 15 > 106.611 ±2.292 ops/ms > MessageDigests.getAndDigest SHA-25616 15 > 961.589 ± 82.052 ops/ms > MessageDigests.getAndDigest SHA-256 1024 15 > 88.646 ±0.194 ops/ms > MessageDigests.getAndDigest SHA-51216 15 > 775.417 ± 56.775 ops/ms > MessageDigests.getAndDigest SHA-512 1024 15 > 112.904 ±2.014 ops/ms > > GC > MessageDigests.getAndDigest:·gc.alloc.rate.norm MD516 15 > 232.009 ±
Re: RFR: 8258833: Cancel multi-part cipher operations in SunPKCS11 after failures [v6]
On Thu, 14 Jan 2021 20:29:54 GMT, Valerie Peng wrote: >> The update fails because the native mechanism (CKM_AES_ECB) has no padding >> and OpenJDK does not buffer data in the Java side for encryption [1] (this >> is a bug that I'll address soon). As a result, there is a PKCS#11 call with >> an invalid length and we get the error that ends up returning the session to >> the Session Manager. I just realized that when we fix the previous >> padding-bug, this test path won't work anymore. CKR_BUFFER_TOO_SMALL errors >> on updates do not lead to a reset call in the OpenJDK side (contrary to >> doFinal), so they wouldn't be useful for the test. I'll investigate if there >> is a way to trigger the path. Otherwise we should keep the doFinal path >> only. I'd still force a reset if there is an error other than >> CKR_BUFFER_TOO_SMALL in the update. >> >> -- >> [1] - >> https://github.com/openjdk/jdk/blob/1cc09ccaef9a3695dd2862e3ee121e141e0a8a13/src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11Cipher.java#L583 > > It's an update call, isn't padding occur when doFinal() is called for > encryption? > In any case, it's best for the test case to not have this bug dependency. I > am ok if you can only test doFinal path only. Yes, makes sense to remove the bug dependency and the whole encrypt-update path. I'll keep the test extensible, though; so we can include new paths eventually. - PR: https://git.openjdk.java.net/jdk/pull/1901
Re: RFR: 8258833: Cancel multi-part cipher operations in SunPKCS11 after failures [v6]
> When a multi-part cipher operation fails in SunPKCS11 (i.e. because of an > invalid block size), we now cancel the operation before returning the > underlying Session to the Session Manager. This allows to use the returned > Session for a different purpose. Otherwise, an CKR_OPERATION_ACTIVE error > would be raised from the PKCS#11 library. > > The jdk/sun/security/pkcs11/Cipher/CancelMultipart.java regression test is > introduced as part of this PR. > > No regressions found in jdk/sun/security/pkcs11. Martin Balao has updated the pull request incrementally with one additional commit since the last revision: Removing the encryption-update path in CancelMultipart test as it depends on a know bug to cause a PKCS#11 error. - Changes: - all: https://git.openjdk.java.net/jdk/pull/1901/files - new: https://git.openjdk.java.net/jdk/pull/1901/files/0f96ddf1..4c892a44 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=1901=05 - incr: https://webrevs.openjdk.java.net/?repo=jdk=1901=04-05 Stats: 22 lines in 1 file changed: 0 ins; 20 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/1901.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1901/head:pull/1901 PR: https://git.openjdk.java.net/jdk/pull/1901
Re: RFR: 8258833: Cancel multi-part cipher operations in SunPKCS11 after failures [v5]
> When a multi-part cipher operation fails in SunPKCS11 (i.e. because of an > invalid block size), we now cancel the operation before returning the > underlying Session to the Session Manager. This allows to use the returned > Session for a different purpose. Otherwise, an CKR_OPERATION_ACTIVE error > would be raised from the PKCS#11 library. > > The jdk/sun/security/pkcs11/Cipher/CancelMultipart.java regression test is > introduced as part of this PR. > > No regressions found in jdk/sun/security/pkcs11. Martin Balao has updated the pull request incrementally with one additional commit since the last revision: Minor documentation improvement in P11Mac regarding Cancel Operation - Changes: - all: https://git.openjdk.java.net/jdk/pull/1901/files - new: https://git.openjdk.java.net/jdk/pull/1901/files/ee90166e..0f96ddf1 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=1901=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk=1901=03-04 Stats: 2 lines in 1 file changed: 1 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/1901.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1901/head:pull/1901 PR: https://git.openjdk.java.net/jdk/pull/1901
Re: RFR: 8258833: Cancel multi-part cipher operations in SunPKCS11 after failures
On Wed, 13 Jan 2021 00:53:01 GMT, Valerie Peng wrote: >>> For cipher impls, there are more than just P11Cipher, there are also >>> P11AEADCipher and P11RSACipher. It looks like they should be updated with >>> this defensive cancellation change unless the non-compliant NSS impl is >>> algorithm-specific and does not apply to AES/GCM and RSA. >> >> Sure, I was going to go through each of them. Only P11Cipher and >> P11Signature so far but I'm working on this. > >> >> >> > For cipher impls, there are more than just P11Cipher, there are also >> > P11AEADCipher and P11RSACipher. It looks like they should be updated with >> > this defensive cancellation change unless the non-compliant NSS impl is >> > algorithm-specific and does not apply to AES/GCM and RSA. >> >> Sure, I was going to go through each of them. Only P11Cipher and >> P11Signature so far but I'm working on this. > > Wonderful, thanks! @valeriepeng let me know your thoughts. Nothing else from my side now, unless you want me to revisit something. - PR: https://git.openjdk.java.net/jdk/pull/1901
Re: RFR: 8258833: Cancel multi-part cipher operations in SunPKCS11 after failures [v4]
> When a multi-part cipher operation fails in SunPKCS11 (i.e. because of an > invalid block size), we now cancel the operation before returning the > underlying Session to the Session Manager. This allows to use the returned > Session for a different purpose. Otherwise, an CKR_OPERATION_ACTIVE error > would be raised from the PKCS#11 library. > > The jdk/sun/security/pkcs11/Cipher/CancelMultipart.java regression test is > introduced as part of this PR. > > No regressions found in jdk/sun/security/pkcs11. Martin Balao has updated the pull request incrementally with six additional commits since the last revision: - More consistent documentation about Cancel Operation in P11-services. - Consistent Cancel Operation behavior across P11-services: do not fail when the operation being cancelled was not initialized. - Better error handling in P11PSSSignature Cancel Operation and documentation improvements. - C_DecryptFinal/C_EncryptFinal failures do not need a Cancel Operation; only C_DecryptUpdate/C_EncryptUpdate ones. - Documentation note explaining why Cancel Operation is not required in P11AEADCipher - Documentation note explaining why Cancel Operation is not required in P11PSSSignature - Changes: - all: https://git.openjdk.java.net/jdk/pull/1901/files - new: https://git.openjdk.java.net/jdk/pull/1901/files/5bf00de0..ee90166e Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=1901=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=1901=02-03 Stats: 60 lines in 5 files changed: 53 ins; 3 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/1901.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1901/head:pull/1901 PR: https://git.openjdk.java.net/jdk/pull/1901
Re: RFR: 8258833: Cancel multi-part cipher operations in SunPKCS11 after failures
On Wed, 13 Jan 2021 00:53:01 GMT, Valerie Peng wrote: >>> For cipher impls, there are more than just P11Cipher, there are also >>> P11AEADCipher and P11RSACipher. It looks like they should be updated with >>> this defensive cancellation change unless the non-compliant NSS impl is >>> algorithm-specific and does not apply to AES/GCM and RSA. >> >> Sure, I was going to go through each of them. Only P11Cipher and >> P11Signature so far but I'm working on this. > >> >> >> > For cipher impls, there are more than just P11Cipher, there are also >> > P11AEADCipher and P11RSACipher. It looks like they should be updated with >> > this defensive cancellation change unless the non-compliant NSS impl is >> > algorithm-specific and does not apply to AES/GCM and RSA. >> >> Sure, I was going to go through each of them. Only P11Cipher and >> P11Signature so far but I'm working on this. > > Wonderful, thanks! P11PSSSignature - The P11PSSSignature case is analogous to PSSSignature, because the PKCS#11 primitives and their use from OpenJDK are similar. Added a note explaining why doCancel = false is safe even after C_Sign or C_SignFinal (so anyone reading the PKCS#11 standard and wondering about cancellation after CKR_BUFFER_TOO_SMALL errors or successful calls to determine the output length will have an answer there). In summary, this is handled at libj2pkcs native level. The only theoretical way I see to get an error here is that the output of the signature is greater than MAX_STACK_BUFFER_LEN -which, of course, will cause more critical issues-. Added a check in cancelOperation, so we have a consistent behavior with other P11 services. Cancel Operation should not fail if the operation being cancelled was not initialized. P11AEADCipher - In P11AEADCipher C_EncryptUpdate/C_DecryptUpdate/C_EncryptFinal/C_DecryptFinal PKCS#11 calls are not used but C_Encrypt/C_Decrypt only. According to the PKCS#11 standard [1], C_Encrypt does not cancel the operation in these cases: 1) CKR_BUFFER_TOO_SMALL error; and, 2) Successful call to determine the length of the output buffer. The NSS Software Token implementation seems to honor this specification in NSC_Encrypt [2]. These paths are not triggerable from OpenJDK because the output length is checked against an expected value before making the PKCS#11 call [3]. So it should be safe to assume that the operation will always be terminated in P11AEADCipher::implDoFinal. As a result, I conclude that no code changes are require in P11AEADCipher::implDoFinal but a documentation note explaining why doCancel = false is safe there [4]. The C_Decrypt case is analogous to C_Encrypt. Added a check in cancelOperation as in P11PSSSignature. P11RSACipher - C_WrapKey/C_UnwrapKey/C_GenerateKey: these calls are atomic, instead of multi-part operations; so there shouldn't be an issue there. In the case of C_Encrypt, C_Decrypt, C_Sign and C_VerifyRecover (P11RSACipher::implDoFinal), a reset with doCancel = true is not done. C_Encrypt/C_Decrypt can theoretically keep an operation active as listed for P11AEADCipher. However, I was unable to find in the NSS Software Token a path leading to a CKR_BUFFER_TOO_SMALL error from P11RSACipher::implDoFinal (CKM_RSA_PKCS or CKM_RSA_X_509 schemes): if the output buffer length is too short, we get a CKR_DEVICE_ERROR error coming from here [5]. I was unable to trigger a successful call with output length equal to 0 (to get the output length from the library) because direct buffers are not used [6] -see argument #6-, length-0 buffers do not return a null pointer here [7] (which is checked here [8] in the NSS side) and null buffers do not make the PKCS#11 call [9]. Looks to me that C_Encrypt may return CKR_BUFFER_TOO_SMALL for some algorithms such as RSA-OAEP, but that's not what P11RSACipher is currently using. C_Sign may return CKR_BUFFER_TOO_SMALL from NSC_SignFinal. However, the user does seem to have enough influence on the PKCS#11 call here [10] to trigger this path. I don't have evidence that the bug could be triggered in P11RSACipher; but the static analysis I've done is limited/error-prone. I'm inclined not to make any change unless we have real evidence that a cancel is required. Added a check in cancelOperation as in P11PSSSignature. P11Mac - Not cancelling from OpenJDK when C_SignFinal is called is safe as described before in this thread. In the case of a C_SignUpdate error, P11Mac::reset is not called so the Session is not returned to the Session Manager. Added a check in cancelOperation as in P11PSSSignature. P11Cipher - Revisiting the P11Cipher case, I realised that the bug can be triggered from C_EncryptUpdate/C_DecryptUpdate calls only and not from C_EncryptFinal/C_DecryptFinal. Thus, in P11Cipher::implDoFinal we might be cancelling when not necessary. Note that
Re: RFR: 8257733: Move module-specific data from make to respective module [v4]
On Fri, 15 Jan 2021 14:58:14 GMT, Alan Bateman wrote: >> This PR is not stale; it's just still waiting for input from @AlanBateman. > > @magicus Can the CharacterDataXXX.template files move to > src/java.base/share/classes/java/lang? @AlanBateman That sounds like an excellent idea. I'll update the PR first thing next week. :) - PR: https://git.openjdk.java.net/jdk/pull/1611
Re: RFR: 8257733: Move module-specific data from make to respective module
2020/12/4 6:08:13 -0800, er...@openjdk.java.net: > On Fri, 4 Dec 2020 12:30:02 GMT, Alan Bateman wrote: >>> And I can certainly move jdwp.spec to java.base instead. That's the >>> reason I need input on this: All I know is that is definitely not >>> the responsibility of the Build Group to maintain that document, and >>> I made my best guess at where to place it. >> >>> And I can certainly move jdwp.spec to java.base instead. >> >> If jdwp.spec has to move to the src tree then src/java.se is probably >> the most suitable home because Java SE specifies JDWP as an optional >> interface. So nothing to do with java.base and the build will need to >> continue to generate the sources for the front-end (jdk.jdi) and >> back-end (jdk.jdwp.agent) as they implement the protocol. > > My understanding of JEPs is that they should be viewed as living > documents. In this case, I think it's perfectly valid to update JEP > 201 with additional source code layout. Both for this and for the > other missing dirs. Feature JEPs are living documents until such time as they are delivered. In this case it would not be appropriate to update JEP 201, which is as much about the transition from the old source-code layout as it is about the new layout as of 2014. At this point, and given that we’d already gone beyond JEP 201 prior to this change (with `man` and `lib` subdirectories), what’d make the most sense is a new informational JEP that documents the source-code layout. Informational JEPs can, within reason, be updated over time. - Mark
Re: RFR: 8256895: Add support for RFC 8954: Online Certificate Status Protocol (OCSP) Nonce Extension [v3]
> This enhancement adds support for the nonce extension in OCSP request > extensions by system property jdk.security.certpath.ocspNonce. > > Please review the CSR at: > https://bugs.openjdk.java.net/browse/JDK-8257766 Hai-May Chao has updated the pull request incrementally with one additional commit since the last revision: Revert changes to UnitTest.java - Changes: - all: https://git.openjdk.java.net/jdk/pull/2039/files - new: https://git.openjdk.java.net/jdk/pull/2039/files/26254d59..949ee2be Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2039=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2039=01-02 Stats: 43 lines in 1 file changed: 0 ins; 41 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/2039.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2039/head:pull/2039 PR: https://git.openjdk.java.net/jdk/pull/2039
Re: RFR: 8257733: Move module-specific data from make to respective module [v4]
On Mon, 11 Jan 2021 09:20:07 GMT, Magnus Ihse Bursie wrote: >> Marked as reviewed by prr (Reviewer). > > This PR is not stale; it's just still waiting for input from @AlanBateman. @magicus Can the CharacterDataXXX.template files move to src/java.base/share/classes/java/lang? - PR: https://git.openjdk.java.net/jdk/pull/1611
Re: RFR: JDK-8259786: initialize last parameter of getpwuid_r
On Fri, 15 Jan 2021 13:54:15 GMT, Harold Seigel wrote:
>> We have a couple of calls to getpwuid_r in the codebase, like
>> g= getpwuid_r(getuid(), , pwd_buf, sizeof(pwd_buf), );
>>
>> Usually we NULL-check pwd after the call because we do not fully trust the
>> return code of the function (it is documented in the codebase why we do not
>> fully trust the return code) . However we miss to initialize pwd at some
>> places before the call, which might we a little problematic and should be
>> improved (at other places we already initialize it).
>>
>> This triggers also Sonar warnings like :
>> https://sonarcloud.io/project/issues?id=jdk=AXaE0dsA8L9hkQskGEbA=false=BUG
>>
>>
>> Aside from this issue , should we in other issue , unify the OS versions
>> of static char* get_user_name(uid_t uid)in posix code (currently we
>> have it for bsd, linux, aix but the functions look very similar ?
>
> Hi Matthias,
> These changes look good, but could you hold off on making these changes until
> pull request https://github.com/openjdk/jdk/pull/2037, which consolidates the
> perfMemory_{aix,bsd,linux].cpp files into one perfMemory_posix.cpp file, has
> been integrated?
> Thanks, Harold
> Hi Matthias,
> These changes look good, but could you hold off on making these changes until
> pull request #2037, which consolidates the perfMemory_{aix,bsd,linux].cpp
> files into one perfMemory_posix.cpp file, has been integrated?
> Thanks, Harold
Hi Harold, thanks for reviewing . I will wait until your change is in .
Good to see that you already started consolidating the perfmem code into
posix, my question
>Aside from this issue , should we in other issue , unify the OS versions of
>static char* get_user_name(uid_t uid) in posix code
>(currently we have it for bsd, linux, aix but the functions look very similar ?
was pointing into the same direction but looks like you already started the
effort !
Thanks, Matthias
-
PR: https://git.openjdk.java.net/jdk/pull/2098
Re: RFR: JDK-8259786: initialize last parameter of getpwuid_r
On Fri, 15 Jan 2021 11:24:49 GMT, Matthias Baesken wrote:
> We have a couple of calls to getpwuid_r in the codebase, like
> g= getpwuid_r(getuid(), , pwd_buf, sizeof(pwd_buf), );
>
> Usually we NULL-check pwd after the call because we do not fully trust the
> return code of the function (it is documented in the codebase why we do not
> fully trust the return code) . However we miss to initialize pwd at some
> places before the call, which might we a little problematic and should be
> improved (at other places we already initialize it).
>
> This triggers also Sonar warnings like :
> https://sonarcloud.io/project/issues?id=jdk=AXaE0dsA8L9hkQskGEbA=false=BUG
>
>
> Aside from this issue , should we in other issue , unify the OS versions of
> static char* get_user_name(uid_t uid)in posix code (currently we have it
> for bsd, linux, aix but the functions look very similar ?
Hi Matthias,
These changes look good, but could you hold off on making these changes until
pull request https://github.com/openjdk/jdk/pull/2037, which consolidates the
perfMemory_{aix,bsd,linux].cpp files into one perfMemory_posix.cpp file, has
been integrated?
Thanks, Harold
-
PR: https://git.openjdk.java.net/jdk/pull/2098
RFR: JDK-8259786: initialize last parameter of getpwuid_r
We have a couple of calls to getpwuid_r in the codebase, like g= getpwuid_r(getuid(), , pwd_buf, sizeof(pwd_buf), ); Usually we NULL-check pwd after the call because we do not fully trust the return code of the function (it is documented in the codebase why we do not fully trust the return code) . However we miss to initialize pwd at some places before the call, which might we a little problematic and should be improved (at other places we already initialize it). This triggers also Sonar warnings like : https://sonarcloud.io/project/issues?id=jdk=AXaE0dsA8L9hkQskGEbA=false=BUG Aside from this issue , should we in other issue , unify the OS versions of static char* get_user_name(uid_t uid)in posix code (currently we have it for bsd, linux, aix but the functions look very similar ? - Commit messages: - JDK-8259786 Changes: https://git.openjdk.java.net/jdk/pull/2098/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2098=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8259786 Stats: 13 lines in 4 files changed: 0 ins; 4 del; 9 mod Patch: https://git.openjdk.java.net/jdk/pull/2098.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2098/head:pull/2098 PR: https://git.openjdk.java.net/jdk/pull/2098
