Re: RFR: 8273646: Add openssl from path variable also in to Default System Openssl Path in OpensslArtifactFetcher

2021-09-19 Thread Thejasvi Voniadka
On Wed, 15 Sep 2021 21:49:01 GMT, Weijun Wang  wrote:

>> The test "sun/security/pkcs12/KeytoolOpensslInteropTest.java" performs 
>> interoperability checks between JDK and openssl with respect to certain 
>> keystore operations. The test requires a suitable version of openssl to be 
>> available on the machine it runs. Some mechanisms are used to discover 
>> openssl, among which searching through a predefined set of paths is also 
>> one. The test at present looks only at the following absolute paths (in that 
>> order):
>
> My understanding is that the $PATH variable usually already contains 
> `/usr/bin` and `/usr/local/bin`. If you switch to $PATH, maybe it's no more 
> necessary to try out those 2 anymore?
> 
> Just curious, what is the full path of openssl on your Cygwin?

@wangweij, just a quick follow-up to make sure this change makes sense given 
the value of PATH variable on some of those machines (I mentioned it in the 
comment above). It appears it is true that openssl is indeed found inside 
/usr/local/bin directory, but on Cygwin-based systems, this actually translates 
to something like "C:\Cygwin\usr\local\bin" system path. Since we are 
explicitly passing "/usr/local/bin/openssl" to the process launcher, it fails 
to launch on such machines. We could have probably passed "C:\Cygwin...", but 
that would indicate some sort of prior knowledge towards where Cygwin is 
installed exactly. Hence, just an additional attempt with no absolute paths 
could succeed.

-

PR: https://git.openjdk.java.net/jdk/pull/5523


Re: RFR: 8273297: AES/GCM non-AVX512+VAES CPUs suffer after 8267125 [v2]

2021-09-19 Thread Smita Kamath
> Performance dropped up to 10% for 1k data after 8267125 for CPUs that do not 
> support the new intrinsic. Tests run were crypto.full.AESGCMBench and 
> crypto.full.AESGCMByteBuffer from the jmh micro benchmarks.
> 
> The problem is each instance of GHASH allocates 96 extra longs for the 
> AVX512+VAES intrinsic regardless if the intrinsic is used. This extra table 
> space should be allocated differently so that non-supporting CPUs do not 
> suffer this penalty. This issue also affects non-Intel CPUs too.

Smita Kamath has updated the pull request incrementally with one additional 
commit since the last revision:

  Added a wrapper around aes-gcm intrinsic, changed data size in TestAESMain 
and added a new constant for htbl entries

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/5402/files
  - new: https://git.openjdk.java.net/jdk/pull/5402/files/4628dc3a..7ea464ae

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5402&range=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5402&range=00-01

  Stats: 42 lines in 5 files changed: 28 ins; 1 del; 13 mod
  Patch: https://git.openjdk.java.net/jdk/pull/5402.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/5402/head:pull/5402

PR: https://git.openjdk.java.net/jdk/pull/5402