Re: RFR: 8256523: Streamline Java SHA2 implementation
On Fri, 20 Nov 2020 00:14:55 GMT, Valerie Peng wrote: >> Current `sun/security/provider/SHA2` implementation is written with lots of >> small method invocations on the fastpath in `implCompress0`. Normally it >> does not matter much, because compilers are able to inline through it, and >> then some compilers even intrinsify the entire `implCompress0`. >> >> But it comes as major downside for platforms that do not have SHA2 >> intrinsics, or those VM configs that blindly interpret the bytecode. Zero, >> for example, keeps re-entering the small methods when computing SHA2 digests >> during jmod/jlink generation during the build, and spends significant time >> there. Linux x86_64 Zero fastdebug builds **improve from 18.5 minutes to >> 11.0 minutes** with this change, which is a major win for development >> experience. >> >> Note that SHA1 is already written in similar streamlined style. SHA5 is >> written with helper functions. This patch moves SHA2 to be closer to SHA1 >> style-wise. >> >> Performance improvement in interpreted modes is substantial, about 4x..5x, >> while compiler modes are not affected. As measured by: `make test >> TEST="micro:MessageDigests" >> MICRO="FORK=1;ITER=5;WARMUP_ITER=5;WARMUP_TIME=1s;TIME=1s;OPTIONS=-p >> digesterName=sha256 -p provider=SUN"` >> >> Before: >> >> Benchmark (digesterName) (length) (provider) Mode Cnt >> Score ErrorUnits >> >> # Server, Default >> MessageDigests.digest sha25664 SUN thrpt5 >> 8585.532 ± 219.896 ops/ms >> MessageDigests.digest sha256 1024 SUN thrpt5 >> 1545.994 ± 73.325 ops/ms >> MessageDigests.digest sha256 16384 SUN thrpt5 >> 110.550 ± 1.576 ops/ms >> >> # Server, -XX:+UnlockDiagnosticVMOptions -XX:-UseSHA256Intrinsics >> MessageDigests.digest sha25664 SUN thrpt5 >> 1426.117 ± 48.320 ops/ms >> MessageDigests.digest sha256 1024 SUN thrpt5 >> 188.779 ± 0.097 ops/ms >> MessageDigests.digest sha256 16384 SUN thrpt5 >> 12.512 ± 1.371 ops/ms >> >> # Server, -Xint >> MessageDigests.digest sha25664 SUN thrpt5 >> 6.143 ± 0.126 ops/ms >> MessageDigests.digest sha256 1024 SUN thrpt5 >> 0.769 ± 0.008 ops/ms >> MessageDigests.digest sha256 16384 SUN thrpt5 >> 0.051 ± 0.001 ops/ms >> >> # Zero >> MessageDigests.digest sha25664 SUN thrpt5 >> 5.358 ± 0.664 ops/ms >> MessageDigests.digest sha256 1024 SUN thrpt5 >> 0.686 ± 0.003 ops/ms >> MessageDigests.digest sha256 16384 SUN thrpt5 >> 0.046 ± 0.001 ops/ms >> >> After: >> >> Benchmark (digesterName) (length) (provider) Mode Cnt >> Score Error Units >> >> # Server, Default >> MessageDigests.digest sha25664 SUN thrpt5 >> 8564.689 ± 1459.552 ops/ms >> MessageDigests.digest sha256 1024 SUN thrpt5 >> 1548.582 ± 78.888 ops/ms >> MessageDigests.digest sha256 16384 SUN thrpt5 >> 110.800 ±0.057 ops/ms >> >> # Server, -XX:+UnlockDiagnosticVMOptions -XX:-UseSHA256Intrinsics >> MessageDigests.digest sha25664 SUN thrpt5 >> 1471.399 ± 66.622 ops/ms >> MessageDigests.digest sha256 1024 SUN thrpt5 >> 186.297 ±0.127 ops/ms >> MessageDigests.digest sha256 16384 SUN thrpt5 >> 12.448 ±0.099 ops/ms >> >> # Server, -Xint >> MessageDigests.digest sha25664 SUN thrpt5 >> 27.046 ±0.304 ops/ms >> MessageDigests.digest sha256 1024 SUN thrpt5 >> 3.538 ±0.060 ops/ms >> MessageDigests.digest sha256 16384 SUN thrpt5 >> 0.238 ±0.002 ops/ms >> >> # Zero >> MessageDigests.digest sha25664 SUN thrpt5 >> 26.696 ±0.968 ops/ms >> MessageDigests.digest sha256 1024 SUN thrpt5 >> 3.655 ±0.024 ops/ms >> MessageDigests.digest sha256 16384 SUN thrpt5 >> 0.241 ±0.002 ops/ms >> >> Addtional testing: >> - [x] `jdk/security` with Linux x86_64 server fastdebug >> - [x] `jdk/security` with Linux x86_64 zero fastdebug > > Changes look good. Thank you, @valeriepeng. I am not sure what is the review policy for this code, should I wait for another reviewer? - PR: https://git.openjdk.java.net/jdk/pull/1283
Re: RFR: 8256523: Streamline Java SHA2 implementation
On Wed, 18 Nov 2020 08:29:49 GMT, Aleksey Shipilev wrote: > Current `sun/security/provider/SHA2` implementation is written with lots of > small method invocations on the fastpath in `implCompress0`. Normally it does > not matter much, because compilers are able to inline through it, and then > some compilers even intrinsify the entire `implCompress0`. > > But it comes as major downside for platforms that do not have SHA2 > intrinsics, or those VM configs that blindly interpret the bytecode. Zero, > for example, keeps re-entering the small methods when computing SHA2 digests > during jmod/jlink generation during the build, and spends significant time > there. Linux x86_64 Zero fastdebug builds **improve from 18.5 minutes to 11.0 > minutes** with this change, which is a major win for development experience. > > Note that SHA1 is already written in similar streamlined style. SHA5 is > written with helper functions. This patch moves SHA2 to be closer to SHA1 > style-wise. > > Performance improvement in interpreted modes is substantial, about 4x..5x, > while compiler modes are not affected. As measured by: `make test > TEST="micro:MessageDigests" > MICRO="FORK=1;ITER=5;WARMUP_ITER=5;WARMUP_TIME=1s;TIME=1s;OPTIONS=-p > digesterName=sha256 -p provider=SUN"` > > Before: > > Benchmark (digesterName) (length) (provider) Mode Cnt > Score ErrorUnits > > # Server, Default > MessageDigests.digest sha25664 SUN thrpt5 > 8585.532 ± 219.896 ops/ms > MessageDigests.digest sha256 1024 SUN thrpt5 > 1545.994 ± 73.325 ops/ms > MessageDigests.digest sha256 16384 SUN thrpt5 > 110.550 ± 1.576 ops/ms > > # Server, -XX:+UnlockDiagnosticVMOptions -XX:-UseSHA256Intrinsics > MessageDigests.digest sha25664 SUN thrpt5 > 1426.117 ± 48.320 ops/ms > MessageDigests.digest sha256 1024 SUN thrpt5 > 188.779 ± 0.097 ops/ms > MessageDigests.digest sha256 16384 SUN thrpt5 > 12.512 ± 1.371 ops/ms > > # Server, -Xint > MessageDigests.digest sha25664 SUN thrpt5 > 6.143 ± 0.126 ops/ms > MessageDigests.digest sha256 1024 SUN thrpt5 > 0.769 ± 0.008 ops/ms > MessageDigests.digest sha256 16384 SUN thrpt5 > 0.051 ± 0.001 ops/ms > > # Zero > MessageDigests.digest sha25664 SUN thrpt5 > 5.358 ± 0.664 ops/ms > MessageDigests.digest sha256 1024 SUN thrpt5 > 0.686 ± 0.003 ops/ms > MessageDigests.digest sha256 16384 SUN thrpt5 > 0.046 ± 0.001 ops/ms > > After: > > Benchmark (digesterName) (length) (provider) Mode Cnt > Score Error Units > > # Server, Default > MessageDigests.digest sha25664 SUN thrpt5 > 8564.689 ± 1459.552 ops/ms > MessageDigests.digest sha256 1024 SUN thrpt5 > 1548.582 ± 78.888 ops/ms > MessageDigests.digest sha256 16384 SUN thrpt5 > 110.800 ±0.057 ops/ms > > # Server, -XX:+UnlockDiagnosticVMOptions -XX:-UseSHA256Intrinsics > MessageDigests.digest sha25664 SUN thrpt5 > 1471.399 ± 66.622 ops/ms > MessageDigests.digest sha256 1024 SUN thrpt5 > 186.297 ±0.127 ops/ms > MessageDigests.digest sha256 16384 SUN thrpt5 > 12.448 ±0.099 ops/ms > > # Server, -Xint > MessageDigests.digest sha25664 SUN thrpt5 > 27.046 ±0.304 ops/ms > MessageDigests.digest sha256 1024 SUN thrpt5 > 3.538 ±0.060 ops/ms > MessageDigests.digest sha256 16384 SUN thrpt5 > 0.238 ±0.002 ops/ms > > # Zero > MessageDigests.digest sha25664 SUN thrpt5 > 26.696 ±0.968 ops/ms > MessageDigests.digest sha256 1024 SUN thrpt5 > 3.655 ±0.024 ops/ms > MessageDigests.digest sha256 16384 SUN thrpt5 > 0.241 ±0.002 ops/ms > > Addtional testing: > - [x] `jdk/security` with Linux x86_64 server fastdebug > - [x] `jdk/security` with Linux x86_64 zero fastdebug Changes look good. - Marked as reviewed by valeriep (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/1283
Re: RFR: 8253821: Improve ByteBuffer performance with GCM [v4]
On Wed, 18 Nov 2020 05:14:11 GMT, Anthony Scarpino wrote: >> src/java.base/share/classes/com/sun/crypto/provider/GaloisCounterMode.java >> line 560: >> >>> 558: System.arraycopy(buffer, 0, block, 0, blockSize); >>> 559: buflen -= block.length; >>> 560: return 0; >> >> Will bufLen > blockSize? Judging from the context of this method, >> buffer.length should always <= blockSize and never be larger? If bufLen == >> blockSize, perhaps we can just use buffer directly and no need to copy. > > This method is being removed Ok. - PR: https://git.openjdk.java.net/jdk/pull/411
Re: RFR: 8253821: Improve ByteBuffer performance with GCM [v4]
On Wed, 18 Nov 2020 04:28:38 GMT, Anthony Scarpino wrote: >> The current impl of constructBlock() seems to have code handling ibuffer >= >> blocksize scenario. Could you fix that and also pass the input offset into >> constructBlock() for this RFE? > > I redid the whole code Sure, I will be waiting for the next commit then. - PR: https://git.openjdk.java.net/jdk/pull/411
Integrated: JDK-8256682: JDK-8202343 is incomplete
On Thu, 19 Nov 2020 18:49:07 GMT, Sean Mullan wrote: > The fix for disabling TLS 1.0 and 1.1 is causing intermittent failures of > this test which depends on TLSv1 and v1.1. This test needs to run its own VM > because it now needs to modify a security property to re-enable TLSv1 and > v1.1. This pull request has now been integrated. Changeset: b9db002f Author:Sean Mullan URL: https://git.openjdk.java.net/jdk/commit/b9db002f Stats: 4 lines in 1 file changed: 0 ins; 0 del; 4 mod 8256682: JDK-8202343 is incomplete Reviewed-by: dfuchs - PR: https://git.openjdk.java.net/jdk/pull/1327
Re: RFR: JDK-8256682: JDK-8202343 is incomplete
On Thu, 19 Nov 2020 18:49:07 GMT, Sean Mullan wrote: > The fix for disabling TLS 1.0 and 1.1 is causing intermittent failures of > this test which depends on TLSv1 and v1.1. This test needs to run its own VM > because it now needs to modify a security property to re-enable TLSv1 and > v1.1. Marked as reviewed by dfuchs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/1327
RFR: JDK-8256682: JDK-8202343 is incomplete
The fix for disabling TLS 1.0 and 1.1 is causing intermittent failures of this test which depends on TLSv1 and v1.1. This test needs to run its own VM because it now needs to modify a security property to re-enable TLSv1 and v1.1. - Commit messages: - JDK-8256682: JDK-8202343 is incomplete Changes: https://git.openjdk.java.net/jdk/pull/1327/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=1327=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8256682 Stats: 4 lines in 1 file changed: 0 ins; 0 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/1327.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1327/head:pull/1327 PR: https://git.openjdk.java.net/jdk/pull/1327
Re: RFR: JDK-8166596: TLS support for the EdDSA signature algorithm [v3]
> Hello all, > This change brings in support for certificates with EdDSA keys (both Ed25519 > and Ed448) allowing those signature algorithms to be used both on the > certificates themselves and used during the handshaking process for messages > like CertificateVerify, ServerKeyExchange and so forth. Jamil Nimeh 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 seven additional commits since the last revision: - Update test to account for JDK-8202343 fix - Merge - Merge - Applied code review comments to tests - Fix cut/paste error with ECDH-RSA key exchange - Merge - Initial EdDSA/TLS solution - Changes: - all: https://git.openjdk.java.net/jdk/pull/1197/files - new: https://git.openjdk.java.net/jdk/pull/1197/files/59aced35..dee70944 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=1197=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=1197=01-02 Stats: 6780 lines in 253 files changed: 3650 ins; 1976 del; 1154 mod Patch: https://git.openjdk.java.net/jdk/pull/1197.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1197/head:pull/1197 PR: https://git.openjdk.java.net/jdk/pull/1197
Integrated: 8253299: Manifest bytes are read twice when verifying a signed JAR
On Wed, 18 Nov 2020 21:59:01 GMT, Hai-May Chao wrote: > Small change to retrieve the raw bytes of manifest during verifying signed > JAR. This pull request has now been integrated. Changeset: 9bb82232 Author:Hai-May Chao Committer: Lance Andersen URL: https://git.openjdk.java.net/jdk/commit/9bb82232 Stats: 6 lines in 1 file changed: 5 ins; 0 del; 1 mod 8253299: Manifest bytes are read twice when verifying a signed JAR Reviewed-by: redestad, lancea, alanb - PR: https://git.openjdk.java.net/jdk/pull/1299
Re: RFR: 8253299: Manifest bytes are read twice when verifying a signed JAR
On Thu, 19 Nov 2020 17:13:00 GMT, Lance Andersen wrote: >> Small change to retrieve the raw bytes of manifest during verifying signed >> JAR. > > Marked as reviewed by lancea (Reviewer). Thank you all for the review. I added the noreg-trivial label to the bug. - PR: https://git.openjdk.java.net/jdk/pull/1299
Re: RFR: 8253299: Manifest bytes are read twice when verifying a signed JAR
On Thu, 19 Nov 2020 17:20:58 GMT, Hai-May Chao wrote: >> Marked as reviewed by lancea (Reviewer). > > Thank you all for the review. I added the noreg-trivial label to the bug. Lance, I've entered /integrate. Thank you for sponsoring this! - PR: https://git.openjdk.java.net/jdk/pull/1299
Re: RFR: 8253299: Manifest bytes are read twice when verifying a signed JAR
On Thu, 19 Nov 2020 17:08:21 GMT, Alan Bateman wrote: >> Small change to retrieve the raw bytes of manifest during verifying signed >> JAR. > > Marked as reviewed by alanb (Reviewer). I can sponsor once you integrate - PR: https://git.openjdk.java.net/jdk/pull/1299
Re: RFR: 8253299: Manifest bytes are read twice when verifying a signed JAR
On Wed, 18 Nov 2020 21:59:01 GMT, Hai-May Chao wrote: > Small change to retrieve the raw bytes of manifest during verifying signed > JAR. Marked as reviewed by lancea (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/1299
Re: RFR: 8253299: Manifest bytes are read twice when verifying a signed JAR
On Wed, 18 Nov 2020 21:59:01 GMT, Hai-May Chao wrote: > Small change to retrieve the raw bytes of manifest during verifying signed > JAR. Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/1299
Integrated: 8202343: Disable TLS 1.0 and 1.1
On Mon, 16 Nov 2020 20:18:16 GMT, Sean Mullan wrote: > This change disables the TLSv1 and TLSv1.1 protocols by adding them to the > jdk.tls.disabledAlgorithms security property in the java.security file. These > protocols use weak algorithms and are being deprecated by the IETF. They > should be disabled by default to improve the default security configuration > of the JDK. See the CSR for more rationale: > https://bugs.openjdk.java.net/browse/JDK-8254713 > > The fix mostly involves changes to existing tests that for one reason or > another depend on the TLSv1 and TLSv1.1 protocols being enabled. There is a > new test specifically for this issue: > test/jdk/sun/security/ssl/SSLContextImpl/SSLContextDefault.java This pull request has now been integrated. Changeset: 3a4b90f0 Author:Sean Mullan URL: https://git.openjdk.java.net/jdk/commit/3a4b90f0 Stats: 396 lines in 21 files changed: 273 ins; 97 del; 26 mod 8202343: Disable TLS 1.0 and 1.1 Reviewed-by: xuelei, dfuchs, coffeys - PR: https://git.openjdk.java.net/jdk/pull/1235
Re: RFR: 8256507: Add a micro benchmark for JDK-8153005 [v2]
On Thu, 19 Nov 2020 07:58:36 GMT, Aleksey Shipilev wrote:
>> Weijun Wang has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> not static anymore
>
> test/micro/org/openjdk/bench/java/security/PKCS12KeyStores.java line 46:
>
>> 44: public class PKCS12KeyStores {
>> 45:
>> 46: static char[] pass = "changeit".toCharArray();
>
> This is still a bit weird: is it a constant? Then this should probably be:
> static final char[] PASS = "changeit".toCharArray();
Yes, it should be. I'll include the change in the next push. Thanks for
reviewing.
-
PR: https://git.openjdk.java.net/jdk/pull/1277
Re: RFR: 8256507: Add a micro benchmark for JDK-8153005 [v2]
On Wed, 18 Nov 2020 18:53:21 GMT, Weijun Wang wrote:
>> This is a micro benchmark for various algorithm settings of PKCS keystores.
>> Strong for new algorithms and weak for old ones. Different iteration counts
>> are tried. The result should show that the current setting (strong1) is
>> more efficient than old setting (weak5).
>
> Weijun Wang has updated the pull request incrementally with one additional
> commit since the last revision:
>
> not static anymore
I like it, thanks. Minor nit follows.
test/micro/org/openjdk/bench/java/security/PKCS12KeyStores.java line 46:
> 44: public class PKCS12KeyStores {
> 45:
> 46: static char[] pass = "changeit".toCharArray();
This is still a bit weird: is it a constant? Then this should probably be:
static final char[] PASS = "changeit".toCharArray();
-
Marked as reviewed by shade (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/1277
