Re: RFR: 8350459: MontgomeryIntegerPolynomialP256 multiply intrinsic with AVX2 on x86_64 [v8]
On Fri, 28 Mar 2025 18:20:31 GMT, Andrey Turbanov wrote:
>> Volodymyr Paprotski has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Fix copyright stmt
>
> src/java.base/share/classes/sun/security/util/math/intpoly/MontgomeryIntegerPolynomialP256.java
> line 164:
>
>> 162: protected void mult(long[] a, long[] b, long[] r) {
>> 163: multImpl(a, b, r);
>> 164: reducePositive(r);
>
> `reducePositive` is now seems unused
oh.. hmm.. I had a second PR that I decided wasnt worth it that was going to
reuse this code..
Will create a second JBS and remove
-
PR Review Comment: https://git.openjdk.org/jdk/pull/23719#discussion_r2019286778
Re: RFR: 8346129: Simplify EdDSA & XDH curve name usage [v7]
On Thu, 27 Mar 2025 23:08:54 GMT, Anthony Scarpino wrote: >> Hi, >> >> I need a review for the following change. Naming conventions for EdDSA and >> XDH have inconsistencies between DisabledAlgorithms and KeyPairGenerator. >> These internal changes help make it more consistent when parsing the actual >> curve being used vs the broader algorithm name. >> >> thanks >> >> Tony > > Anthony Scarpino has updated the pull request incrementally with one > additional commit since the last revision: > > code review comments LGTM. - Marked as reviewed by weijun (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/23647#pullrequestreview-2725456861
Re: RFR: 8339280: jarsigner -verify performs cross-checking between CEN and LOC [v22]
> The jarsigner -verify command currently performs verification by reading from > JarFile to navigate the central directory (CEN) headers. It is now enhanced > to include cross-validation of entries between JarFile (CEN-based) and > JarInputStream (stream-based) representations of the JAR. It emits earnings > when detecting discrepancies between a JAR file’s central directory and its > local file entries. Hai-May Chao has updated the pull request incrementally with one additional commit since the last revision: Update with Weijun's comments - Changes: - all: https://git.openjdk.org/jdk/pull/23532/files - new: https://git.openjdk.org/jdk/pull/23532/files/4c801be1..78093d4e Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=23532&range=21 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=23532&range=20-21 Stats: 17 lines in 2 files changed: 0 ins; 10 del; 7 mod Patch: https://git.openjdk.org/jdk/pull/23532.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/23532/head:pull/23532 PR: https://git.openjdk.org/jdk/pull/23532
Re: RFR: 8339280: jarsigner -verify performs cross-checking between CEN and LOC [v21]
On Thu, 27 Mar 2025 21:29:05 GMT, Hai-May Chao wrote: >> The jarsigner -verify command currently performs verification by reading >> from JarFile to navigate the central directory (CEN) headers. It is now >> enhanced to include cross-validation of entries between JarFile (CEN-based) >> and JarInputStream (stream-based) representations of the JAR. It emits >> earnings when detecting discrepancies between a JAR file’s central directory >> and its local file entries. > > Hai-May Chao has updated the pull request incrementally with one additional > commit since the last revision: > > Fix typo Final comments: 1. The warning message "Manifest attribute %s is present..." is not 100% precise, should be "Manifest main attribute %s is present...". 2. The manifest does not need to be at the first position when reading as a `JarFile` (especially we are not comparing the order now), so suggest changing the last lines in `crossCheckEntries` to jarFile.stream() .map(JarEntry::getName) .filter(n -> !locEntries.contains(n) && !n.equals(JarFile.MANIFEST_NAME)) .forEach(n -> crossChkWarnings.add(String.format(rb.getString( "entry.1.present.when.reading.jarfile.but.missing.via.jarinputstream"), n))); - PR Comment: https://git.openjdk.org/jdk/pull/23532#issuecomment-2761397907
Integrated: 8350459: MontgomeryIntegerPolynomialP256 multiply intrinsic with AVX2 on x86_64
On Thu, 20 Feb 2025 21:49:42 GMT, Volodymyr Paprotski wrote: > Add AVX2 montgomery multiplication intrinsic. (About 60-80% gain) > > Also add reduction to existing AVX512 multiplication (this was left-over from > https://github.com/openjdk/jdk/pull/19893 where a quick fix was required). > This is mostly for cleanup, but there is about 1-2% gain. > > Before (no AVX512) > > Benchmark(algorithm) (dataSize) (keyLength) > (provider) Mode Cnt Score Error Units > SignatureBench.ECDSA.signSHA256withECDSA1024 256 > thrpt 40 3720.589 ± 17.879 ops/s > SignatureBench.ECDSA.signSHA256withECDSA 16384 256 > thrpt 40 3605.940 ± 15.807 ops/s > SignatureBench.ECDSA.verify SHA256withECDSA1024 256 > thrpt 40 1076.502 ± 4.190 ops/s > SignatureBench.ECDSA.verify SHA256withECDSA 16384 256 > thrpt 40 1069.624 ± 2.484 ops/s > Benchmark (algorithm) (keyLength) > (kpgAlgorithm) (provider) Mode Cnt Score Error Units > KeyAgreementBench.EC.generateSecret ECDH 256 > EC thrpt 40 830.448 ± 2.285 ops/s > > After (with AVX2) > > Benchmark(algorithm) (dataSize) (keyLength) > (provider) Mode Cnt Score Error Units > SignatureBench.ECDSA.signSHA256withECDSA1024 256 > thrpt 40 6000.496 ± 39.923 ops/s > SignatureBench.ECDSA.signSHA256withECDSA 16384 256 > thrpt 40 5739.878 ± 34.838 ops/s > SignatureBench.ECDSA.verify SHA256withECDSA1024 256 > thrpt 40 1942.437 ± 12.179 ops/s > SignatureBench.ECDSA.verify SHA256withECDSA 16384 256 > thrpt 40 1921.770 ± 8.992 ops/s > Benchmark (algorithm) (keyLength) > (kpgAlgorithm) (provider) Mode Cnt Score Error Units > KeyAgreementBench.EC.generateSecret ECDH 256 > EC thrpt 40 1399.761 ± 6.238 ops/s > > > Before (with AVX512): > > Benchmark(algorithm) (dataSize) (keyLength) > (provider) Mode Cnt Score Error Units > SignatureBench.ECDSA.signSHA256withECDSA1024 256 > thrpt 409621.950 ± 27.260 ops/s > SignatureBench.ECDSA.signSHA256withECDSA 16384 256 > thrpt 408975.654 ± 26.707 ops/s > SignatureBench.ECDSA.verify SHA256withECDSA102... This pull request has now been integrated. Changeset: a269bef0 Author:Volodymyr Paprotski URL: https://git.openjdk.org/jdk/commit/a269bef04cf3c9c8b731edcbf7618624f7571a2d Stats: 760 lines in 9 files changed: 641 ins; 16 del; 103 mod 8350459: MontgomeryIntegerPolynomialP256 multiply intrinsic with AVX2 on x86_64 Reviewed-by: ascarpino, sviswanathan - PR: https://git.openjdk.org/jdk/pull/23719
Integrated: 8341775: Duplicate manifest files are removed by jarsigner after signing
On Mon, 18 Nov 2024 22:40:40 GMT, Kevin Driver wrote: > JDK-8341775: In the case where there is a *single* META-INF directory but > potentially *multiple* manifest files of different cases, print a warning > before selecting the first one and ignoring the rest (the current behavior > should be maintained). This pull request has now been integrated. Changeset: d8090337 Author:Kevin Driver URL: https://git.openjdk.org/jdk/commit/d8090337ee8ea763bca4e8e3baaf8ee4dd3d6214 Stats: 118 lines in 4 files changed: 117 ins; 0 del; 1 mod 8341775: Duplicate manifest files are removed by jarsigner after signing Reviewed-by: weijun, hchao - PR: https://git.openjdk.org/jdk/pull/2
Re: RFR: 8346129: Simplify EdDSA & XDH curve name usage [v7]
On Thu, 27 Mar 2025 23:08:54 GMT, Anthony Scarpino wrote: >> Hi, >> >> I need a review for the following change. Naming conventions for EdDSA and >> XDH have inconsistencies between DisabledAlgorithms and KeyPairGenerator. >> These internal changes help make it more consistent when parsing the actual >> curve being used vs the broader algorithm name. >> >> thanks >> >> Tony > > Anthony Scarpino has updated the pull request incrementally with one > additional commit since the last revision: > > code review comments LGTM - Marked as reviewed by abarashev (Author). PR Review: https://git.openjdk.org/jdk/pull/23647#pullrequestreview-2725964720
Re: RFR: 8350459: MontgomeryIntegerPolynomialP256 multiply intrinsic with AVX2 on x86_64 [v8]
On Thu, 27 Mar 2025 19:13:59 GMT, Volodymyr Paprotski
wrote:
>> Add AVX2 montgomery multiplication intrinsic. (About 60-80% gain)
>>
>> Also add reduction to existing AVX512 multiplication (this was left-over
>> from https://github.com/openjdk/jdk/pull/19893 where a quick fix was
>> required). This is mostly for cleanup, but there is about 1-2% gain.
>>
>> Before (no AVX512)
>>
>> Benchmark(algorithm) (dataSize) (keyLength)
>> (provider) Mode Cnt Score Error Units
>> SignatureBench.ECDSA.signSHA256withECDSA1024 256
>> thrpt 40 3720.589 ± 17.879 ops/s
>> SignatureBench.ECDSA.signSHA256withECDSA 16384 256
>> thrpt 40 3605.940 ± 15.807 ops/s
>> SignatureBench.ECDSA.verify SHA256withECDSA1024 256
>> thrpt 40 1076.502 ± 4.190 ops/s
>> SignatureBench.ECDSA.verify SHA256withECDSA 16384 256
>> thrpt 40 1069.624 ± 2.484 ops/s
>> Benchmark (algorithm) (keyLength)
>> (kpgAlgorithm) (provider) Mode Cnt Score Error Units
>> KeyAgreementBench.EC.generateSecret ECDH 256
>> EC thrpt 40 830.448 ± 2.285 ops/s
>>
>> After (with AVX2)
>>
>> Benchmark(algorithm) (dataSize) (keyLength)
>> (provider) Mode Cnt Score Error Units
>> SignatureBench.ECDSA.signSHA256withECDSA1024 256
>> thrpt 40 6000.496 ± 39.923 ops/s
>> SignatureBench.ECDSA.signSHA256withECDSA 16384 256
>> thrpt 40 5739.878 ± 34.838 ops/s
>> SignatureBench.ECDSA.verify SHA256withECDSA1024 256
>> thrpt 40 1942.437 ± 12.179 ops/s
>> SignatureBench.ECDSA.verify SHA256withECDSA 16384 256
>> thrpt 40 1921.770 ± 8.992 ops/s
>> Benchmark (algorithm) (keyLength)
>> (kpgAlgorithm) (provider) Mode Cnt Score Error Units
>> KeyAgreementBench.EC.generateSecret ECDH 256
>> EC thrpt 40 1399.761 ± 6.238 ops/s
>>
>>
>> Before (with AVX512):
>>
>> Benchmark(algorithm) (dataSize) (keyLength)
>> (provider) Mode Cnt Score Error Units
>> SignatureBench.ECDSA.signSHA256withECDSA1024 256
>> thrpt 409621.950 ± 27.260 ops/s
>> SignatureBench.ECDSA.signSHA256withECDSA 16384 256
>> thrpt 408975.654 ± 26.707 o...
>
> Volodymyr Paprotski has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Fix copyright stmt
src/java.base/share/classes/sun/security/util/math/intpoly/MontgomeryIntegerPolynomialP256.java
line 164:
> 162: protected void mult(long[] a, long[] b, long[] r) {
> 163: multImpl(a, b, r);
> 164: reducePositive(r);
`reducePositive` is now seems unused
-
PR Review Comment: https://git.openjdk.org/jdk/pull/23719#discussion_r2019135976
Re: RFR: 8349583: Add mechanism to disable signature schemes based on their TLS scope [v21]
On Fri, 28 Mar 2025 03:12:40 GMT, Artur Barashev wrote:
>> Currently when a signature scheme constraint is specified with
>> "jdk.tls.disabledAlgorithms" property we don't differentiate between
>> signatures used to sign a TLS handshake exchange and the signatures used in
>> TLS certificates:
>> https://datatracker.ietf.org/doc/html/rfc8446#section-4.2.3
>
> Artur Barashev has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Update copyright year
A final review, just a couple of minor comments.
src/java.base/share/classes/sun/security/ssl/SignatureScheme.java line 221:
> 219:
> 220: // Handshake signature scope.
> 221: public static final Set HANDSHAKE_SCOPE =
I think this constant and `CERTIFICATE_SCOPE` can be package-private.
src/java.base/share/classes/sun/security/ssl/SignatureScheme.java line 554:
> 552: // Returns true if this signature scheme is supported for the given
> 553: // protocol version and SSL scopes.
> 554: boolean isSupportedProtocol(ProtocolVersion version, Set
> scopes) {
I think this method and `isAllowed` can be private.
-
PR Review: https://git.openjdk.org/jdk/pull/23681#pullrequestreview-2726622461
PR Review Comment: https://git.openjdk.org/jdk/pull/23681#discussion_r2019284852
PR Review Comment: https://git.openjdk.org/jdk/pull/23681#discussion_r2019286085
Re: RFR: 8350459: MontgomeryIntegerPolynomialP256 multiply intrinsic with AVX2 on x86_64 [v8]
On Fri, 28 Mar 2025 14:39:23 GMT, Sean Mullan wrote: > I think it would also be useful to write a release note describing the > approximate performance improvement gains for the crypto algorithms as > displayed in your chart. Thanks. @seanjmullan I think I only done that once, cant find the 'instructions'.. I think Jamil had helped me, but.. (https://bugs.openjdk.org/browse/JDK-8297970) "Create subtask with 'release-note' label?" - PR Comment: https://git.openjdk.org/jdk/pull/23719#issuecomment-2762362675
Re: RFR: 8350459: MontgomeryIntegerPolynomialP256 multiply intrinsic with AVX2 on x86_64 [v8]
On Thu, 27 Mar 2025 19:13:59 GMT, Volodymyr Paprotski wrote: >> Add AVX2 montgomery multiplication intrinsic. (About 60-80% gain) >> >> Also add reduction to existing AVX512 multiplication (this was left-over >> from https://github.com/openjdk/jdk/pull/19893 where a quick fix was >> required). This is mostly for cleanup, but there is about 1-2% gain. >> >> Before (no AVX512) >> >> Benchmark(algorithm) (dataSize) (keyLength) >> (provider) Mode Cnt Score Error Units >> SignatureBench.ECDSA.signSHA256withECDSA1024 256 >> thrpt 40 3720.589 ± 17.879 ops/s >> SignatureBench.ECDSA.signSHA256withECDSA 16384 256 >> thrpt 40 3605.940 ± 15.807 ops/s >> SignatureBench.ECDSA.verify SHA256withECDSA1024 256 >> thrpt 40 1076.502 ± 4.190 ops/s >> SignatureBench.ECDSA.verify SHA256withECDSA 16384 256 >> thrpt 40 1069.624 ± 2.484 ops/s >> Benchmark (algorithm) (keyLength) >> (kpgAlgorithm) (provider) Mode Cnt Score Error Units >> KeyAgreementBench.EC.generateSecret ECDH 256 >> EC thrpt 40 830.448 ± 2.285 ops/s >> >> After (with AVX2) >> >> Benchmark(algorithm) (dataSize) (keyLength) >> (provider) Mode Cnt Score Error Units >> SignatureBench.ECDSA.signSHA256withECDSA1024 256 >> thrpt 40 6000.496 ± 39.923 ops/s >> SignatureBench.ECDSA.signSHA256withECDSA 16384 256 >> thrpt 40 5739.878 ± 34.838 ops/s >> SignatureBench.ECDSA.verify SHA256withECDSA1024 256 >> thrpt 40 1942.437 ± 12.179 ops/s >> SignatureBench.ECDSA.verify SHA256withECDSA 16384 256 >> thrpt 40 1921.770 ± 8.992 ops/s >> Benchmark (algorithm) (keyLength) >> (kpgAlgorithm) (provider) Mode Cnt Score Error Units >> KeyAgreementBench.EC.generateSecret ECDH 256 >> EC thrpt 40 1399.761 ± 6.238 ops/s >> >> >> Before (with AVX512): >> >> Benchmark(algorithm) (dataSize) (keyLength) >> (provider) Mode Cnt Score Error Units >> SignatureBench.ECDSA.signSHA256withECDSA1024 256 >> thrpt 409621.950 ± 27.260 ops/s >> SignatureBench.ECDSA.signSHA256withECDSA 16384 256 >> thrpt 408975.654 ± 26.707 o... > > Volodymyr Paprotski has updated the pull request incrementally with one > additional commit since the last revision: > > Fix copyright stmt Thanks Tony! - PR Comment: https://git.openjdk.org/jdk/pull/23719#issuecomment-2761529451
Re: RFR: 8350459: MontgomeryIntegerPolynomialP256 multiply intrinsic with AVX2 on x86_64 [v8]
On Thu, 27 Mar 2025 19:13:59 GMT, Volodymyr Paprotski wrote: >> Add AVX2 montgomery multiplication intrinsic. (About 60-80% gain) >> >> Also add reduction to existing AVX512 multiplication (this was left-over >> from https://github.com/openjdk/jdk/pull/19893 where a quick fix was >> required). This is mostly for cleanup, but there is about 1-2% gain. >> >> Before (no AVX512) >> >> Benchmark(algorithm) (dataSize) (keyLength) >> (provider) Mode Cnt Score Error Units >> SignatureBench.ECDSA.signSHA256withECDSA1024 256 >> thrpt 40 3720.589 ± 17.879 ops/s >> SignatureBench.ECDSA.signSHA256withECDSA 16384 256 >> thrpt 40 3605.940 ± 15.807 ops/s >> SignatureBench.ECDSA.verify SHA256withECDSA1024 256 >> thrpt 40 1076.502 ± 4.190 ops/s >> SignatureBench.ECDSA.verify SHA256withECDSA 16384 256 >> thrpt 40 1069.624 ± 2.484 ops/s >> Benchmark (algorithm) (keyLength) >> (kpgAlgorithm) (provider) Mode Cnt Score Error Units >> KeyAgreementBench.EC.generateSecret ECDH 256 >> EC thrpt 40 830.448 ± 2.285 ops/s >> >> After (with AVX2) >> >> Benchmark(algorithm) (dataSize) (keyLength) >> (provider) Mode Cnt Score Error Units >> SignatureBench.ECDSA.signSHA256withECDSA1024 256 >> thrpt 40 6000.496 ± 39.923 ops/s >> SignatureBench.ECDSA.signSHA256withECDSA 16384 256 >> thrpt 40 5739.878 ± 34.838 ops/s >> SignatureBench.ECDSA.verify SHA256withECDSA1024 256 >> thrpt 40 1942.437 ± 12.179 ops/s >> SignatureBench.ECDSA.verify SHA256withECDSA 16384 256 >> thrpt 40 1921.770 ± 8.992 ops/s >> Benchmark (algorithm) (keyLength) >> (kpgAlgorithm) (provider) Mode Cnt Score Error Units >> KeyAgreementBench.EC.generateSecret ECDH 256 >> EC thrpt 40 1399.761 ± 6.238 ops/s >> >> >> Before (with AVX512): >> >> Benchmark(algorithm) (dataSize) (keyLength) >> (provider) Mode Cnt Score Error Units >> SignatureBench.ECDSA.signSHA256withECDSA1024 256 >> thrpt 409621.950 ± 27.260 ops/s >> SignatureBench.ECDSA.signSHA256withECDSA 16384 256 >> thrpt 408975.654 ± 26.707 o... > > Volodymyr Paprotski has updated the pull request incrementally with one > additional commit since the last revision: > > Fix copyright stmt @vpaprotsk Your change (at version 0a4230aa41f7cb5ddf5e2978f89ad4f0ec231e88) is now ready to be sponsored by a Committer. - PR Comment: https://git.openjdk.org/jdk/pull/23719#issuecomment-2761532342
Re: RFR: 8350459: MontgomeryIntegerPolynomialP256 multiply intrinsic with AVX2 on x86_64 [v8]
On Thu, 27 Mar 2025 19:13:59 GMT, Volodymyr Paprotski wrote: >> Add AVX2 montgomery multiplication intrinsic. (About 60-80% gain) >> >> Also add reduction to existing AVX512 multiplication (this was left-over >> from https://github.com/openjdk/jdk/pull/19893 where a quick fix was >> required). This is mostly for cleanup, but there is about 1-2% gain. >> >> Before (no AVX512) >> >> Benchmark(algorithm) (dataSize) (keyLength) >> (provider) Mode Cnt Score Error Units >> SignatureBench.ECDSA.signSHA256withECDSA1024 256 >> thrpt 40 3720.589 ± 17.879 ops/s >> SignatureBench.ECDSA.signSHA256withECDSA 16384 256 >> thrpt 40 3605.940 ± 15.807 ops/s >> SignatureBench.ECDSA.verify SHA256withECDSA1024 256 >> thrpt 40 1076.502 ± 4.190 ops/s >> SignatureBench.ECDSA.verify SHA256withECDSA 16384 256 >> thrpt 40 1069.624 ± 2.484 ops/s >> Benchmark (algorithm) (keyLength) >> (kpgAlgorithm) (provider) Mode Cnt Score Error Units >> KeyAgreementBench.EC.generateSecret ECDH 256 >> EC thrpt 40 830.448 ± 2.285 ops/s >> >> After (with AVX2) >> >> Benchmark(algorithm) (dataSize) (keyLength) >> (provider) Mode Cnt Score Error Units >> SignatureBench.ECDSA.signSHA256withECDSA1024 256 >> thrpt 40 6000.496 ± 39.923 ops/s >> SignatureBench.ECDSA.signSHA256withECDSA 16384 256 >> thrpt 40 5739.878 ± 34.838 ops/s >> SignatureBench.ECDSA.verify SHA256withECDSA1024 256 >> thrpt 40 1942.437 ± 12.179 ops/s >> SignatureBench.ECDSA.verify SHA256withECDSA 16384 256 >> thrpt 40 1921.770 ± 8.992 ops/s >> Benchmark (algorithm) (keyLength) >> (kpgAlgorithm) (provider) Mode Cnt Score Error Units >> KeyAgreementBench.EC.generateSecret ECDH 256 >> EC thrpt 40 1399.761 ± 6.238 ops/s >> >> >> Before (with AVX512): >> >> Benchmark(algorithm) (dataSize) (keyLength) >> (provider) Mode Cnt Score Error Units >> SignatureBench.ECDSA.signSHA256withECDSA1024 256 >> thrpt 409621.950 ± 27.260 ops/s >> SignatureBench.ECDSA.signSHA256withECDSA 16384 256 >> thrpt 408975.654 ± 26.707 o... > > Volodymyr Paprotski has updated the pull request incrementally with one > additional commit since the last revision: > > Fix copyright stmt I think it would also be useful to write a release note describing the approximate performance improvement gains for the crypto algorithms as displayed in your chart. Thanks. - PR Comment: https://git.openjdk.org/jdk/pull/23719#issuecomment-2761566550
