Re: RFR: 8259886 : Improve SSL session cache performance and scalability [v2]

2021-02-01 Thread Xue-Lei Andrew Fan
On Mon, 1 Feb 2021 18:49:04 GMT, djelinski 
 wrote:

>> Under certain load, MemoryCache operations take a substantial fraction of 
>> the time needed to complete SSL handshakes. This series of patches improves 
>> performance characteristics of MemoryCache, at the cost of a functional 
>> change: expired entries are no longer guaranteed to be removed before live 
>> ones. Unused entries are still removed before used ones, and cache 
>> performance no longer depends on its capacity.
>> 
>> First patch in the series contains a benchmark that can be run with `make 
>> test TEST="micro:CacheBench"`.
>> Baseline results before any MemoryCache changes:
>> Benchmark   (size)  (timeout)  Mode  Cnt ScoreError  Units
>> CacheBench.put   20480  86400  avgt   2583.653 ?  6.269  us/op
>> CacheBench.put   20480  0  avgt   25 0.107 ?  0.001  us/op
>> CacheBench.put  204800  86400  avgt   25  2057.781 ? 35.942  us/op
>> CacheBench.put  204800  0  avgt   25 0.108 ?  0.001  us/op
>> there's a nonlinear performance drop between 20480 and 204800 entries, 
>> probably attributable to CPU cache thrashing. Beyond 204800 entries the 
>> cache scales more linearly.
>> 
>> Benchmark results after the 2nd and 3rd patches are pretty similar, so I'll 
>> only copy one:
>> Benchmark   (size)  (timeout)  Mode  Cnt  Score   Error  Units
>> CacheBench.put   20480  86400  avgt   25  0.146 ? 0.002  us/op
>> CacheBench.put   20480  0  avgt   25  0.108 ? 0.002  us/op
>> CacheBench.put  204800  86400  avgt   25  0.150 ? 0.001  us/op
>> CacheBench.put  204800  0  avgt   25  0.106 ? 0.001  us/op
>> The third patch improves worst-case times on a mostly idle cache by 
>> scattering removal of expired entries over multiple `put` calls. It does not 
>> affect performance of an overloaded cache.
>> 
>> The 4th patch removes all code that clears cached values before handing them 
>> over to the GC. [This 
>> comment](https://github.com/openjdk/jdk/commit/5859a0320334bfb6b46b62eb16b4c387641f4a2a#diff-c6bd583a97fbc4f471621fee7eab37c63718cdb6932ce357fa403cfda4b32b6fL346)
>>  stated that clearing values was supposed to be a GC performance 
>> optimization. It wasn't. Benchmark results after that commit:
>> Benchmark   (size)  (timeout)  Mode  Cnt  Score   Error  Units
>> CacheBench.put   20480  86400  avgt   25  0.113 ? 0.001  us/op
>> CacheBench.put   20480  0  avgt   25  0.075 ? 0.002  us/op
>> CacheBench.put  204800  86400  avgt   25  0.116 ? 0.001  us/op
>> CacheBench.put  204800  0  avgt   25  0.072 ? 0.001  us/op
>> I wasn't expecting that much of an improvement, and don't know how to 
>> explain it.
>> 
>> The 40ns difference between cache with and without a timeout can be 
>> attributed to 2 `System.currentTimeMillis()` calls; they were pretty slow on 
>> my VM.
>
> djelinski has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Simplify makefile

If I get the patch right, the benchmark performance improvement is a trade-off 
between CPU and memory, by keeping expired entries while putting a new entry in 
the cache.  I'm not very sure of the performance impact on memory and GC 
collections.  Would you mind add two more types of benchmark: get the entries 
and remove the entries, for cases that there are 1/10, 1/4, 1/3 and 1/2 expired 
entries in the cache?  And increase the size to some big scales, like 2M and 
20M.

It looks like a spec update as it may change the behavior of a few JDK 
components (TLS session cache, OCSP stapling response cache, cert store cache, 
certificate factory, etc), because of "expired entries are no longer guaranteed 
to be removed before live ones".  I'm not very sure of the impact. I may 
suggest to file a CSR and have more eyes to check the compatibility impact 
before moving forward.

-

Changes requested by xuelei (Reviewer).

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


Re: RFR: 8259886 : Improve SSL session cache performance and scalability [v2]

2021-02-01 Thread Xue-Lei Andrew Fan
On Mon, 1 Feb 2021 22:45:17 GMT, Claes Redestad  wrote:

>> Build change looks good, but I would like to hear from @cl4es too.
>
> Adding an `--add-exports` to `JAVAC_FLAGS` is a bit iffy, but should be OK. 
> Yes, all benchmarks will now be compiled with that package exported and 
> visible, but that should have no unintentional effect on other compilations.

The impact could beyond the JSSE implementation, andI will have a look as well.

-

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


Re: RFR: 8259662: Don't wrap SocketExceptions into SSLExceptions in SSLSocketImpl [v7]

2021-02-01 Thread Clive Verghese
On Fri, 29 Jan 2021 00:50:04 GMT, Xue-Lei Andrew Fan  wrote:

>> Clive Verghese has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Address review comments
>
> Please make sure all tier1 and tier2 tests passed on all supported platforms. 
>  Except the failed I commented in the update, 
> javax/net/ssl/SSLSession/TestEnabledProtocols.java is also failed with a 
> similar stack on Windows.  Maybe, the pull request command "/test tier1" and  
> "/test tier2" could help speed up the testing.

Hi, 

Thank you for the feedback. 

I have verified that tier1 and tier2 tests pass on Windows, MacOS and Linux.

-

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


Re: RFR: 8259662: Don't wrap SocketExceptions into SSLExceptions in SSLSocketImpl [v9]

2021-02-01 Thread Clive Verghese
> Redo for 8237578: JDK-8214339 (SSLSocketImpl wraps SocketException) appears 
> to not be fully fixed
> 
> This also fixes JDK-8259516: Alerts sent by peer may not be received 
> correctly during TLS handshake

Clive Verghese has updated the pull request incrementally with one additional 
commit since the last revision:

  Fix test failures

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2057/files
  - new: https://git.openjdk.java.net/jdk/pull/2057/files/573406dd..43cb77d1

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=2057=08
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2057=07-08

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2057.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2057/head:pull/2057

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


RFR: 8257497: Key identifier compliance issue

2021-02-01 Thread Hai-May Chao
This change is made for compliance with RFC 5280 section 4.2.1.1 for Authority 
Key Identifier extension.

-

Commit messages:
 - 8257497: Key identifier compliance issue

Changes: https://git.openjdk.java.net/jdk/pull/2343/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2343=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8257497
  Stats: 32 lines in 2 files changed: 22 ins; 0 del; 10 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2343.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2343/head:pull/2343

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


Re: RFR: 8260864: ProblemList two security/krb5 tests on Linux

2021-02-01 Thread Weijun Wang
On Mon, 1 Feb 2021 22:51:06 GMT, Daniel D. Daugherty  wrote:

> @wangweij - just a heads up that I'm ProblemListing the two tests you said to 
> "ignore".

No problem! Good idea!! Well done!!!

-

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


Re: RFR: 8260864: ProblemList two security/krb5 tests on Linux

2021-02-01 Thread David Holmes
On Mon, 1 Feb 2021 22:41:16 GMT, Daniel D. Daugherty  wrote:

> A trivial fix to ProblemList these two tests on Linux:
> 
> sun/security/krb5/auto/ReplayCacheTestProcWithMD5.java
> sun/security/krb5/auto/ReplayCacheTestProc.java
> 
> We're rolling machines over to Linux 8.3 and these two tests are starting
> to fail in the CI in Tier2 (so far). So we need to reduce the noise.

LGTM!

-

Marked as reviewed by dholmes (Reviewer).

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


Integrated: 8260864: ProblemList two security/krb5 tests on Linux

2021-02-01 Thread Daniel D . Daugherty
On Mon, 1 Feb 2021 22:41:16 GMT, Daniel D. Daugherty  wrote:

> A trivial fix to ProblemList these two tests on Linux:
> 
> sun/security/krb5/auto/ReplayCacheTestProcWithMD5.java
> sun/security/krb5/auto/ReplayCacheTestProc.java
> 
> We're rolling machines over to Linux 8.3 and these two tests are starting
> to fail in the CI in Tier2 (so far). So we need to reduce the noise.

This pull request has now been integrated.

Changeset: a6d95058
Author:Daniel D. Daugherty 
URL:   https://git.openjdk.java.net/jdk/commit/a6d95058
Stats: 2 lines in 1 file changed: 2 ins; 0 del; 0 mod

8260864: ProblemList two security/krb5 tests on Linux

Reviewed-by: dholmes

-

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


Re: RFR: 8260864: ProblemList two security/krb5 tests on Linux

2021-02-01 Thread Daniel D . Daugherty
On Mon, 1 Feb 2021 22:50:37 GMT, David Holmes  wrote:

>> A trivial fix to ProblemList these two tests on Linux:
>> 
>> sun/security/krb5/auto/ReplayCacheTestProcWithMD5.java
>> sun/security/krb5/auto/ReplayCacheTestProc.java
>> 
>> We're rolling machines over to Linux 8.3 and these two tests are starting
>> to fail in the CI in Tier2 (so far). So we need to reduce the noise.
>
> LGTM!

@wangweij - just a heads up that I'm ProblemListing the two tests you said to 
"ignore".

@dholmes-ora - Thanks for the fast review!

-

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


RFR: 8260864: ProblemList two security/krb5 tests on Linux

2021-02-01 Thread Daniel D . Daugherty
A trivial fix to ProblemList these two tests on Linux:

sun/security/krb5/auto/ReplayCacheTestProcWithMD5.java
sun/security/krb5/auto/ReplayCacheTestProc.java

We're rolling machines over to Linux 8.3 and these two tests are starting
to fail in the CI in Tier2 (so far). So we need to reduce the noise.

-

Commit messages:
 - 8260864: ProblemList two security/krb5 tests on Linux

Changes: https://git.openjdk.java.net/jdk/pull/2342/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2342=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8260864
  Stats: 2 lines in 1 file changed: 2 ins; 0 del; 0 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2342.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2342/head:pull/2342

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


Re: RFR: 8259886 : Improve SSL session cache performance and scalability [v2]

2021-02-01 Thread Claes Redestad
On Mon, 1 Feb 2021 19:22:22 GMT, Erik Joelsson  wrote:

>> djelinski has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Simplify makefile
>
> Build change looks good, but I would like to hear from @cl4es too.

Adding an `--add-exports` to `JAVAC_FLAGS` is a bit iffy, but should be OK. 
Yes, all benchmarks will now be compiled with that package exported and 
visible, but that should have no unintentional effect on other compilations.

-

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


Re: RFR: 8260861: TrustStoreDescriptor log the same value

2021-02-01 Thread Davin Kevin
On Tue, 8 Dec 2020 09:35:26 GMT, Davin Kevin 
 wrote:

> If both jssecacerts and cacerts are not defined, the same log is printed 
> twice, talking about the jssecacerts instead of cacerts the second time. 

OCA has been signed and my name has been added into the OCA listing page: 
* https://www.oracle.com/technical-resources/oracle-contributor-agreement.html

What is the next step to make this contribution eligible to merge ?

-

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


Re: RFR: 8260861: TrustStoreDescriptor log the same value

2021-02-01 Thread Davin Kevin
On Thu, 31 Dec 2020 09:43:18 GMT, Davin Kevin 
 wrote:

>> If both jssecacerts and cacerts are not defined, the same log is printed 
>> twice, talking about the jssecacerts instead of cacerts the second time. 
>
> OCA has been signed and my name has been added into the OCA listing page: 
> * https://www.oracle.com/technical-resources/oracle-contributor-agreement.html
> 
> What is the next step to make this contribution eligible to merge ?

I've made all the tasks required, so I don't know why the process is blocked .

-

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


Re: RFR: 8260861: TrustStoreDescriptor log the same value

2021-02-01 Thread Xue-Lei Andrew Fan
On Tue, 8 Dec 2020 09:35:26 GMT, Davin Kevin 
 wrote:

> If both jssecacerts and cacerts are not defined, the same log is printed 
> twice, talking about the jssecacerts instead of cacerts the second time. 

Marked as reviewed by xuelei (Reviewer).

-

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


RFR: 8260861: TrustStoreDescriptor log the same value

2021-02-01 Thread Davin Kevin
If both jssecacerts and cacerts are not defined, the same log is printed twice, 
talking about the jssecacerts instead of cacerts the second time. 

-

Commit messages:
 - fix(truststore): log about truststore not found print everytime the same 
value

Changes: https://git.openjdk.java.net/jdk/pull/1690/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=1690=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8260861
  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/1690.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/1690/head:pull/1690

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


Re: RFR: 8211227: Inconsistent TLS protocol version in debug output

2021-02-01 Thread Rajan Halade
On Mon, 1 Feb 2021 10:37:35 GMT, Evan Whelan  wrote:

> Hi everyone,
> 
> Please review my fix for JDK-8211227
> 
> This supportability fix will result in a more consistent debug format when 
> reading and writing TLS protocol versions.
> 
> Thanks,
> Evan

Changes requested by rhalade (Reviewer).

test/jdk/sun/security/ssl/SSLLogger/LoggingFormatConsistency.java line 36:

> 34: /*
> 35:  * This test runs in another process so we can monitor the debug
> 36:  * results.  The OutputAnalyzer must see correct debug output to return a

Suggestion:

 * results. The OutputAnalyzer must see correct debug output to return a

test/jdk/sun/security/ssl/SSLLogger/LoggingFormatConsistency.java line 83:

> 81: // Re-enabling as test depends on these algorithms
> 82: SecurityUtils.removeFromDisabledTlsAlgs("TLSv1", "TLSv1.1");
> 83: var url = new URL("https://jpg-data.us.oracle.com/;);

This URL is not accessible outside.

test/jdk/sun/security/ssl/SSLLogger/LoggingFormatConsistency.java line 50:

> 48: public class LoggingFormatConsistency {
> 49: public static void main(String[] args) throws Exception {
> 50: if (args.length == 0){

Please add a comment to explain when the test should be run with parameter.

-

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


Re: RFR: 8259886 : Improve SSL session cache performance and scalability [v2]

2021-02-01 Thread Erik Joelsson
On Mon, 1 Feb 2021 18:49:04 GMT, djelinski 
 wrote:

>> Under certain load, MemoryCache operations take a substantial fraction of 
>> the time needed to complete SSL handshakes. This series of patches improves 
>> performance characteristics of MemoryCache, at the cost of a functional 
>> change: expired entries are no longer guaranteed to be removed before live 
>> ones. Unused entries are still removed before used ones, and cache 
>> performance no longer depends on its capacity.
>> 
>> First patch in the series contains a benchmark that can be run with `make 
>> test TEST="micro:CacheBench"`.
>> Baseline results before any MemoryCache changes:
>> Benchmark   (size)  (timeout)  Mode  Cnt ScoreError  Units
>> CacheBench.put   20480  86400  avgt   2583.653 ?  6.269  us/op
>> CacheBench.put   20480  0  avgt   25 0.107 ?  0.001  us/op
>> CacheBench.put  204800  86400  avgt   25  2057.781 ? 35.942  us/op
>> CacheBench.put  204800  0  avgt   25 0.108 ?  0.001  us/op
>> there's a nonlinear performance drop between 20480 and 204800 entries, 
>> probably attributable to CPU cache thrashing. Beyond 204800 entries the 
>> cache scales more linearly.
>> 
>> Benchmark results after the 2nd and 3rd patches are pretty similar, so I'll 
>> only copy one:
>> Benchmark   (size)  (timeout)  Mode  Cnt  Score   Error  Units
>> CacheBench.put   20480  86400  avgt   25  0.146 ? 0.002  us/op
>> CacheBench.put   20480  0  avgt   25  0.108 ? 0.002  us/op
>> CacheBench.put  204800  86400  avgt   25  0.150 ? 0.001  us/op
>> CacheBench.put  204800  0  avgt   25  0.106 ? 0.001  us/op
>> The third patch improves worst-case times on a mostly idle cache by 
>> scattering removal of expired entries over multiple `put` calls. It does not 
>> affect performance of an overloaded cache.
>> 
>> The 4th patch removes all code that clears cached values before handing them 
>> over to the GC. [This 
>> comment](https://github.com/openjdk/jdk/commit/5859a0320334bfb6b46b62eb16b4c387641f4a2a#diff-c6bd583a97fbc4f471621fee7eab37c63718cdb6932ce357fa403cfda4b32b6fL346)
>>  stated that clearing values was supposed to be a GC performance 
>> optimization. It wasn't. Benchmark results after that commit:
>> Benchmark   (size)  (timeout)  Mode  Cnt  Score   Error  Units
>> CacheBench.put   20480  86400  avgt   25  0.113 ? 0.001  us/op
>> CacheBench.put   20480  0  avgt   25  0.075 ? 0.002  us/op
>> CacheBench.put  204800  86400  avgt   25  0.116 ? 0.001  us/op
>> CacheBench.put  204800  0  avgt   25  0.072 ? 0.001  us/op
>> I wasn't expecting that much of an improvement, and don't know how to 
>> explain it.
>> 
>> The 40ns difference between cache with and without a timeout can be 
>> attributed to 2 `System.currentTimeMillis()` calls; they were pretty slow on 
>> my VM.
>
> djelinski has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Simplify makefile

Build change looks good, but I would like to hear from @cl4es too.

-

Marked as reviewed by erikj (Reviewer).

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


Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v8]

2021-02-01 Thread Andrew Haley
On Sun, 31 Jan 2021 20:14:01 GMT, Anton Kozlov  wrote:

>> Please review the implementation of JEP 391: macOS/AArch64 Port.
>> 
>> It's heavily based on existing ports to linux/aarch64, macos/x86_64, and 
>> windows/aarch64. 
>> 
>> Major changes are in:
>> * src/hotspot/cpu/aarch64: support of the new calling convention (subtasks 
>> JDK-8253817, JDK-8253818)
>> * src/hotspot/os_cpu/bsd_aarch64: copy of os_cpu/linux_aarch64 with 
>> necessary adjustments (JDK-8253819)
>> * src/hotspot/share, test/hotspot/gtest: support of write-xor-execute (W^X), 
>> required on macOS/AArch64 platform. It's implemented with 
>> pthread_jit_write_protect_np provided by Apple. The W^X mode is local to a 
>> thread, so W^X mode change relates to the java thread state change (for java 
>> threads). In most cases, JVM executes in write-only mode, except when 
>> calling a generated stub like SafeFetch, which requires a temporary switch 
>> to execute-only mode. The same execute-only mode is enabled when a java 
>> thread executes in java or native states. This approach of managing W^X mode 
>> turned out to be simple and efficient enough.
>> * src/jdk.hotspot.agent: serviceability agent implementation (JDK-8254941)
>
> Anton Kozlov has updated the pull request with a new target base due to a 
> merge or a rebase. The pull request now contains 62 commits:
> 
>  - Merge branch 'master' into jdk-macos
>  - Update copyright year for BsdAARCH64ThreadContext.java
>  - Fix inclusing of StubRoutines header
>  - Redo buildsys fix
>  - Revert harfbuzz changes, disable warnings for it
>  - Little adjustement of SlowSignatureHandler
>  - Partially bring previous commit
>  - Revert "Address feedback for signature generators"
>
>This reverts commit 50b55f6684cd21f8b532fa979b7b6fbb4613266d.
>  - Refactor CDS disabling
>  - Redo builsys support for aarch64-darwin
>  - ... and 52 more: 
> https://git.openjdk.java.net/jdk/compare/8a9004da...b421e0b4

src/hotspot/cpu/aarch64/interpreterRT_aarch64.cpp line 84:

> 82: // on stack. Natural alignment for types are still in place,
> 83: // for example double/long should be 8 bytes alligned
> 84: 

This comment is a bit confusing because it's no longer #ifdef APPLE. Better 
move it up to Line 41.

src/hotspot/cpu/aarch64/interpreterRT_aarch64.cpp line 352:

> 350: 
> 351: #ifdef __APPLE__
> 352:   virtual void pass_byte()

Please remove ```#ifdef __APPLE__``` around this region.

src/hotspot/cpu/aarch64/sharedRuntime_aarch64.cpp line 839:

> 837:   // The code unable to handle this, bailout.
> 838:   return -1;
> 839: #endif

This looks like a bug to me. The caller doesn't necessarily check the return 
value. See CallRuntimeNode::calling_convention.

-

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


Re: RFR: 8259886 : Improve SSL session cache performance and scalability [v2]

2021-02-01 Thread djelinski
> Under certain load, MemoryCache operations take a substantial fraction of the 
> time needed to complete SSL handshakes. This series of patches improves 
> performance characteristics of MemoryCache, at the cost of a functional 
> change: expired entries are no longer guaranteed to be removed before live 
> ones. Unused entries are still removed before used ones, and cache 
> performance no longer depends on its capacity.
> 
> First patch in the series contains a benchmark that can be run with `make 
> test TEST="micro:CacheBench"`.
> Baseline results before any MemoryCache changes:
> Benchmark   (size)  (timeout)  Mode  Cnt ScoreError  Units
> CacheBench.put   20480  86400  avgt   2583.653 ?  6.269  us/op
> CacheBench.put   20480  0  avgt   25 0.107 ?  0.001  us/op
> CacheBench.put  204800  86400  avgt   25  2057.781 ? 35.942  us/op
> CacheBench.put  204800  0  avgt   25 0.108 ?  0.001  us/op
> there's a nonlinear performance drop between 20480 and 204800 entries, 
> probably attributable to CPU cache thrashing. Beyond 204800 entries the cache 
> scales more linearly.
> 
> Benchmark results after the 2nd and 3rd patches are pretty similar, so I'll 
> only copy one:
> Benchmark   (size)  (timeout)  Mode  Cnt  Score   Error  Units
> CacheBench.put   20480  86400  avgt   25  0.146 ? 0.002  us/op
> CacheBench.put   20480  0  avgt   25  0.108 ? 0.002  us/op
> CacheBench.put  204800  86400  avgt   25  0.150 ? 0.001  us/op
> CacheBench.put  204800  0  avgt   25  0.106 ? 0.001  us/op
> The third patch improves worst-case times on a mostly idle cache by 
> scattering removal of expired entries over multiple `put` calls. It does not 
> affect performance of an overloaded cache.
> 
> The 4th patch removes all code that clears cached values before handing them 
> over to the GC. [This 
> comment](https://github.com/openjdk/jdk/commit/5859a0320334bfb6b46b62eb16b4c387641f4a2a#diff-c6bd583a97fbc4f471621fee7eab37c63718cdb6932ce357fa403cfda4b32b6fL346)
>  stated that clearing values was supposed to be a GC performance 
> optimization. It wasn't. Benchmark results after that commit:
> Benchmark   (size)  (timeout)  Mode  Cnt  Score   Error  Units
> CacheBench.put   20480  86400  avgt   25  0.113 ? 0.001  us/op
> CacheBench.put   20480  0  avgt   25  0.075 ? 0.002  us/op
> CacheBench.put  204800  86400  avgt   25  0.116 ? 0.001  us/op
> CacheBench.put  204800  0  avgt   25  0.072 ? 0.001  us/op
> I wasn't expecting that much of an improvement, and don't know how to explain 
> it.
> 
> The 40ns difference between cache with and without a timeout can be 
> attributed to 2 `System.currentTimeMillis()` calls; they were pretty slow on 
> my VM.

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

  Simplify makefile

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2255/files
  - new: https://git.openjdk.java.net/jdk/pull/2255/files/5859a032..34949970

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=2255=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2255=00-01

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2255.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2255/head:pull/2255

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


Re: RFR: 8259886 : Improve SSL session cache performance and scalability [v2]

2021-02-01 Thread djelinski
On Mon, 1 Feb 2021 18:35:56 GMT, djelinski 
 wrote:

>> Hm, maybe you just misunderstand how this makefile construct works. If you 
>> just want to add "--add-exports java.base/sun.security.util=ALL-UNNAMED", 
>> then that's all you should put in this assignment.
>
> yeah, I'm new to makefiles. Let me try that...

Removed. I could have sworn I tried this... but apparently I didn't. Thanks for 
the suggestion!

-

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


Re: RFR: 8259886 : Improve SSL session cache performance and scalability

2021-02-01 Thread djelinski
On Mon, 1 Feb 2021 18:31:39 GMT, Erik Joelsson  wrote:

>> Adding "--add-exports java.base/sun.security.util=ALL-UNNAMED" is fine, I'm 
>> asking about "$(JAVAC_FLAGS)".
>
> Hm, maybe you just misunderstand how this makefile construct works. If you 
> just want to add "--add-exports java.base/sun.security.util=ALL-UNNAMED", 
> then that's all you should put in this assignment.

yeah, I'm new to makefiles. Let me try that...

-

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


Re: RFR: 8257858: [macOS]: Remove JNF dependency from libosxsecurity/KeystoreImpl.m [v7]

2021-02-01 Thread Erik Joelsson
On Mon, 1 Feb 2021 16:51:12 GMT, Weijun Wang  wrote:

>> This fix covers both
>> 
>> - [[macOS]: Remove JNF dependency from 
>> libosxsecurity/KeystoreImpl.m](https://bugs.openjdk.java.net/browse/JDK-8257858)
>> - [[macOS]: Remove JNF dependency from 
>> libosxkrb5/SCDynamicStoreConfig.m](https://bugs.openjdk.java.net/browse/JDK-8257860)
>
> Weijun Wang has updated the pull request incrementally with two additional 
> commits since the last revision:
> 
>  - file attr error
>  - objc use .m

Build changes look good. Thanks for fixing the .m support in 
TestFilesCompilation.gmk!

-

Marked as reviewed by erikj (Reviewer).

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


Re: RFR: 8259886 : Improve SSL session cache performance and scalability

2021-02-01 Thread Erik Joelsson
On Mon, 1 Feb 2021 18:24:46 GMT, djelinski 
 wrote:

>> make/test/BuildMicrobenchmark.gmk line 97:
>> 
>>> 95: SRC := $(MICROBENCHMARK_SRC), \
>>> 96: BIN := $(MICROBENCHMARK_CLASSES), \
>>> 97: JAVAC_FLAGS := $(JAVAC_FLAGS) --add-exports 
>>> java.base/sun.security.util=ALL-UNNAMED, \
>> 
>> Why do you need to add $(JAVAC_FLAGS) here?
>
> I'm trying to benchmark a class that is in a non-exported package 
> `sun.security.util`. Without this line the benchmark doesn't compile. I 
> couldn't find any other benchmarks that access non-exported classes, so I 
> came up with my own implementation. 
> 
> Is there a better way to get the benchmark to compile?

Adding "--add-exports java.base/sun.security.util=ALL-UNNAMED" is fine, I'm 
asking about "$(JAVAC_FLAGS)".

-

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


Re: RFR: 8259886 : Improve SSL session cache performance and scalability

2021-02-01 Thread Erik Joelsson
On Mon, 1 Feb 2021 18:30:14 GMT, Erik Joelsson  wrote:

>> I'm trying to benchmark a class that is in a non-exported package 
>> `sun.security.util`. Without this line the benchmark doesn't compile. I 
>> couldn't find any other benchmarks that access non-exported classes, so I 
>> came up with my own implementation. 
>> 
>> Is there a better way to get the benchmark to compile?
>
> Adding "--add-exports java.base/sun.security.util=ALL-UNNAMED" is fine, I'm 
> asking about "$(JAVAC_FLAGS)".

Hm, maybe you just misunderstand how this makefile construct works. If you just 
want to add "--add-exports java.base/sun.security.util=ALL-UNNAMED", then 
that's all you should put in this assignment.

-

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


Re: RFR: 8259886 : Improve SSL session cache performance and scalability

2021-02-01 Thread djelinski
On Mon, 1 Feb 2021 18:18:51 GMT, Erik Joelsson  wrote:

>> Under certain load, MemoryCache operations take a substantial fraction of 
>> the time needed to complete SSL handshakes. This series of patches improves 
>> performance characteristics of MemoryCache, at the cost of a functional 
>> change: expired entries are no longer guaranteed to be removed before live 
>> ones. Unused entries are still removed before used ones, and cache 
>> performance no longer depends on its capacity.
>> 
>> First patch in the series contains a benchmark that can be run with `make 
>> test TEST="micro:CacheBench"`.
>> Baseline results before any MemoryCache changes:
>> Benchmark   (size)  (timeout)  Mode  Cnt ScoreError  Units
>> CacheBench.put   20480  86400  avgt   2583.653 ?  6.269  us/op
>> CacheBench.put   20480  0  avgt   25 0.107 ?  0.001  us/op
>> CacheBench.put  204800  86400  avgt   25  2057.781 ? 35.942  us/op
>> CacheBench.put  204800  0  avgt   25 0.108 ?  0.001  us/op
>> there's a nonlinear performance drop between 20480 and 204800 entries, 
>> probably attributable to CPU cache thrashing. Beyond 204800 entries the 
>> cache scales more linearly.
>> 
>> Benchmark results after the 2nd and 3rd patches are pretty similar, so I'll 
>> only copy one:
>> Benchmark   (size)  (timeout)  Mode  Cnt  Score   Error  Units
>> CacheBench.put   20480  86400  avgt   25  0.146 ? 0.002  us/op
>> CacheBench.put   20480  0  avgt   25  0.108 ? 0.002  us/op
>> CacheBench.put  204800  86400  avgt   25  0.150 ? 0.001  us/op
>> CacheBench.put  204800  0  avgt   25  0.106 ? 0.001  us/op
>> The third patch improves worst-case times on a mostly idle cache by 
>> scattering removal of expired entries over multiple `put` calls. It does not 
>> affect performance of an overloaded cache.
>> 
>> The 4th patch removes all code that clears cached values before handing them 
>> over to the GC. [This 
>> comment](https://github.com/openjdk/jdk/commit/5859a0320334bfb6b46b62eb16b4c387641f4a2a#diff-c6bd583a97fbc4f471621fee7eab37c63718cdb6932ce357fa403cfda4b32b6fL346)
>>  stated that clearing values was supposed to be a GC performance 
>> optimization. It wasn't. Benchmark results after that commit:
>> Benchmark   (size)  (timeout)  Mode  Cnt  Score   Error  Units
>> CacheBench.put   20480  86400  avgt   25  0.113 ? 0.001  us/op
>> CacheBench.put   20480  0  avgt   25  0.075 ? 0.002  us/op
>> CacheBench.put  204800  86400  avgt   25  0.116 ? 0.001  us/op
>> CacheBench.put  204800  0  avgt   25  0.072 ? 0.001  us/op
>> I wasn't expecting that much of an improvement, and don't know how to 
>> explain it.
>> 
>> The 40ns difference between cache with and without a timeout can be 
>> attributed to 2 `System.currentTimeMillis()` calls; they were pretty slow on 
>> my VM.
>
> make/test/BuildMicrobenchmark.gmk line 97:
> 
>> 95: SRC := $(MICROBENCHMARK_SRC), \
>> 96: BIN := $(MICROBENCHMARK_CLASSES), \
>> 97: JAVAC_FLAGS := $(JAVAC_FLAGS) --add-exports 
>> java.base/sun.security.util=ALL-UNNAMED, \
> 
> Why do you need to add $(JAVAC_FLAGS) here?

I'm trying to benchmark a class that is in a non-exported package 
`sun.security.util`. Without this line the benchmark doesn't compile. I 
couldn't find any other benchmarks that access non-exported classes, so I came 
up with my own implementation. 

Is there a better way to get the benchmark to compile?

-

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


Re: RFR: 8259886 : Improve SSL session cache performance and scalability

2021-02-01 Thread Erik Joelsson
On Wed, 27 Jan 2021 11:32:08 GMT, djelinski 
 wrote:

> Under certain load, MemoryCache operations take a substantial fraction of the 
> time needed to complete SSL handshakes. This series of patches improves 
> performance characteristics of MemoryCache, at the cost of a functional 
> change: expired entries are no longer guaranteed to be removed before live 
> ones. Unused entries are still removed before used ones, and cache 
> performance no longer depends on its capacity.
> 
> First patch in the series contains a benchmark that can be run with `make 
> test TEST="micro:CacheBench"`.
> Baseline results before any MemoryCache changes:
> Benchmark   (size)  (timeout)  Mode  Cnt ScoreError  Units
> CacheBench.put   20480  86400  avgt   2583.653 ?  6.269  us/op
> CacheBench.put   20480  0  avgt   25 0.107 ?  0.001  us/op
> CacheBench.put  204800  86400  avgt   25  2057.781 ? 35.942  us/op
> CacheBench.put  204800  0  avgt   25 0.108 ?  0.001  us/op
> there's a nonlinear performance drop between 20480 and 204800 entries, 
> probably attributable to CPU cache thrashing. Beyond 204800 entries the cache 
> scales more linearly.
> 
> Benchmark results after the 2nd and 3rd patches are pretty similar, so I'll 
> only copy one:
> Benchmark   (size)  (timeout)  Mode  Cnt  Score   Error  Units
> CacheBench.put   20480  86400  avgt   25  0.146 ? 0.002  us/op
> CacheBench.put   20480  0  avgt   25  0.108 ? 0.002  us/op
> CacheBench.put  204800  86400  avgt   25  0.150 ? 0.001  us/op
> CacheBench.put  204800  0  avgt   25  0.106 ? 0.001  us/op
> The third patch improves worst-case times on a mostly idle cache by 
> scattering removal of expired entries over multiple `put` calls. It does not 
> affect performance of an overloaded cache.
> 
> The 4th patch removes all code that clears cached values before handing them 
> over to the GC. [This 
> comment](https://github.com/openjdk/jdk/commit/5859a0320334bfb6b46b62eb16b4c387641f4a2a#diff-c6bd583a97fbc4f471621fee7eab37c63718cdb6932ce357fa403cfda4b32b6fL346)
>  stated that clearing values was supposed to be a GC performance 
> optimization. It wasn't. Benchmark results after that commit:
> Benchmark   (size)  (timeout)  Mode  Cnt  Score   Error  Units
> CacheBench.put   20480  86400  avgt   25  0.113 ? 0.001  us/op
> CacheBench.put   20480  0  avgt   25  0.075 ? 0.002  us/op
> CacheBench.put  204800  86400  avgt   25  0.116 ? 0.001  us/op
> CacheBench.put  204800  0  avgt   25  0.072 ? 0.001  us/op
> I wasn't expecting that much of an improvement, and don't know how to explain 
> it.
> 
> The 40ns difference between cache with and without a timeout can be 
> attributed to 2 `System.currentTimeMillis()` calls; they were pretty slow on 
> my VM.

make/test/BuildMicrobenchmark.gmk line 97:

> 95: SRC := $(MICROBENCHMARK_SRC), \
> 96: BIN := $(MICROBENCHMARK_CLASSES), \
> 97: JAVAC_FLAGS := $(JAVAC_FLAGS) --add-exports 
> java.base/sun.security.util=ALL-UNNAMED, \

Why do you need to add $(JAVAC_FLAGS) here?

-

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


Re: Keytool does not agree with RFC 8410

2021-02-01 Thread Wei-Jun Wang
Thanks. I also noticed ‘openssl x509’ has a -force_pubkey for this case. We’ll 
think about what is the best we can do.

—Max

> On Feb 1, 2021, at 11:23 AM, Anders Rundgren  
> wrote:
> 
> On 2021-02-01 16:01, Wei-Jun Wang wrote:
>>> On Feb 1, 2021, at 2:32 AM, Anders Rundgren  
>>> wrote:
>>> 
>>> On 2021-01-31 20:00, Wei-Jun Wang wrote:
 https://bugs.openjdk.java.net/browse/JDK-8260693 filed.
>>> 
>>> Thanx!
>>> In the bug report you also write:
>>> 
>>>We'll also need a way to generate this kind of certificate (or certreq).
>>>There is no signature algorithm on XDH and we need to use EdDSA instead.
>>>See 
>>> https://urldefense.com/v3/__https://tools.ietf.org/html/rfc8410*section-10.2__;Iw!!GqivPVa7Brio!NmY7j2ZVt-VJoTtZkQFA8tbhvHgLZazChGpwWEbycxxjAHm6aDkm8clW3eJ2H14Ugw$
>>>  .
>>> 
>>> AFAIK there is no standard for CSRs for encryption keys.  You need to use a 
>>> signature key that sort of vouches for the enclosed public key.  This key 
>>> may use any valid signature algorithm.
>> https://urldefense.com/v3/__https://tools.ietf.org/html/rfc2986*section-3__;Iw!!GqivPVa7Brio!Ijzb1F6vf9ECCLwUXYTNnZ0XEm7RWM5C17BPQO2k-YySNn8RpgCInbcsZ1pH23wpwA$
>>   says:
>> 1. A CertificationRequestInfo value containing a subject
>>distinguished name, a subject public key, and optionally a
>>set of attributes is constructed by an entity requesting
>>certification.
>> 2. The CertificationRequestInfo value is signed with the subject
>>entity's private key.  (See Section 4.2.)
>> It hasn’t said the “public key” and “private key” above should be a pair, 
>> though.
> 
> I believe this is sort of "implicit" because otherwise there would be a need 
> to indicate which key that was used in order to verify the signature.
> 
> CMC was probably designed to cope with this restriction.
> https://urldefense.com/v3/__https://tools.ietf.org/html/rfc5272*section-3.2__;Iw!!GqivPVa7Brio!Ijzb1F6vf9ECCLwUXYTNnZ0XEm7RWM5C17BPQO2k-YySNn8RpgCInbcsZ1pvrP3bEQ$
>  
>>> As a side note, my own applications use a key container attestation key for 
>>> *all* CSRs which is a more useful method than self-signed CSRs.
>> Interesting. Is there any document describing this feature?
> 
> WebAuthn appears to use this method although they only register public keys:
> https://urldefense.com/v3/__https://www.w3.org/TR/webauthn-2/*sctn-api__;Iw!!GqivPVa7Brio!Ijzb1F6vf9ECCLwUXYTNnZ0XEm7RWM5C17BPQO2k-YySNn8RpgCInbcsZ1olcwu24Q$
>  
> My particular take on this is a bit more elaborate because the attestation is 
> rather creating a session and shared key which permits secure multi-step key 
> (store) management operations:
> https://urldefense.com/v3/__https://cyberphone.github.io/doc/security/keygen2.html__;!!GqivPVa7Brio!Ijzb1F6vf9ECCLwUXYTNnZ0XEm7RWM5C17BPQO2k-YySNn8RpgCInbcsZ1rBveLZ7A$
>  
> Thanx,
> Anders
> 
>> Thanks,
>> Max
>>> 
>>> Regards,
>>> Anders
>>> 
>>> 
 Thanks,
 Max
> On Jan 31, 2021, at 2:12 AM, Anders Rundgren 
>  wrote:
> 
> Since the JDK bug report tool does not include "keytool" I posted this 
> here.
> 
> Keytool for JDK 15 reports "Subject Public Key Algorithm: XDH key of 
> unknown size" for a certificate  containing the following public key:
> 
> 148: SEQUENCE {
>  150:   SEQUENCE {
>  152: OBJECT IDENTIFIER X25519 (1.3.101.110)
> }
>  157:   BIT STRING, 32 bytes
>   : a3 5e 94 ef bd d0 41 86 90 07 87 9e 80 d0 a5 76 
> '.^Av'
>   0010: 0e a1 ba 82 19 2e c3 90 21 89 05 5a f6 d9 e6 50 
> '!..Z...P'
>   }
> 
> which seems to be aligned with: 
> https://urldefense.com/v3/__https://tools.ietf.org/html/rfc8410*section-10.2__;Iw!!GqivPVa7Brio!NmY7j2ZVt-VJoTtZkQFA8tbhvHgLZazChGpwWEbycxxjAHm6aDkm8clW3eJ2H14Ugw$
> You can verify this issue by importing the certificate in the RFC.
> 
> Anders
> 



Re: RFR: 8211227: Inconsistent TLS protocol version in debug output

2021-02-01 Thread Xue-Lei Andrew Fan
On Mon, 1 Feb 2021 10:37:35 GMT, Evan Whelan  wrote:

> Hi everyone,
> 
> Please review my fix for JDK-8211227
> 
> This supportability fix will result in a more consistent debug format when 
> reading and writing TLS protocol versions.
> 
> Thanks,
> Evan

Marked as reviewed by xuelei (Reviewer).

-

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


Re: RFR: 8257858: [macOS]: Remove JNF dependency from libosxsecurity/KeystoreImpl.m [v7]

2021-02-01 Thread Weijun Wang
> This fix covers both
> 
> - [[macOS]: Remove JNF dependency from 
> libosxsecurity/KeystoreImpl.m](https://bugs.openjdk.java.net/browse/JDK-8257858)
> - [[macOS]: Remove JNF dependency from 
> libosxkrb5/SCDynamicStoreConfig.m](https://bugs.openjdk.java.net/browse/JDK-8257860)

Weijun Wang has updated the pull request incrementally with two additional 
commits since the last revision:

 - file attr error
 - objc use .m

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/1845/files
  - new: https://git.openjdk.java.net/jdk/pull/1845/files/ef337f12..b5f1e15d

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=1845=06
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=1845=05-06

  Stats: 6 lines in 4 files changed: 0 ins; 1 del; 5 mod
  Patch: https://git.openjdk.java.net/jdk/pull/1845.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/1845/head:pull/1845

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


Re: RFR: 8257858: [macOS]: Remove JNF dependency from libosxsecurity/KeystoreImpl.m [v6]

2021-02-01 Thread Weijun Wang
On Mon, 1 Feb 2021 11:41:02 GMT, Magnus Ihse Bursie  wrote:

>> Weijun Wang 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 15 additional 
>> commits since the last revision:
>> 
>>  - move test
>>  - Merge branch 'master' into 8257858
>>  - a test
>>
>>only in patch2:
>>unchanged:
>>  - end values should be vectors
>>  - phil comment
>>  - same behavior as before -- empty realm map
>>  - error check, new JavaStringToNSString
>>  - do not find class and method in loop
>>  - no more header file
>>
>>reverted:
>>  - better macro, no more JNI_COCOA_ENTER
>>  - ... and 5 more: 
>> https://git.openjdk.java.net/jdk/compare/81433b38...ef337f12
>
> make/test/JtregNativeJdk.gmk line 84:
> 
>> 82:   -framework Cocoa -framework JavaNativeFoundation
>> 83:   BUILD_JDK_JTREG_EXECUTABLES_LIBS_exeJniInvocationTest := -ljli
>> 84:   BUILD_JDK_JTREG_LIBRARIES_CFLAGS_libTestDynamicStore := -ObjC
> 
> Instead of "tricking" the build system of compiling an Obj-C file by 
> masquerading it as a C file and passing compiler options, you should expose 
> the test file for what it is, and add support in the build system to handle 
> this. I can help you with that part.

Fixed.

-

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


Re: Keytool does not agree with RFC 8410

2021-02-01 Thread Anders Rundgren

On 2021-02-01 16:01, Wei-Jun Wang wrote:



On Feb 1, 2021, at 2:32 AM, Anders Rundgren  
wrote:

On 2021-01-31 20:00, Wei-Jun Wang wrote:

https://bugs.openjdk.java.net/browse/JDK-8260693 filed.


Thanx!
In the bug report you also write:

We'll also need a way to generate this kind of certificate (or certreq).
There is no signature algorithm on XDH and we need to use EdDSA instead.
See 
https://urldefense.com/v3/__https://tools.ietf.org/html/rfc8410*section-10.2__;Iw!!GqivPVa7Brio!NmY7j2ZVt-VJoTtZkQFA8tbhvHgLZazChGpwWEbycxxjAHm6aDkm8clW3eJ2H14Ugw$
 .

AFAIK there is no standard for CSRs for encryption keys.  You need to use a 
signature key that sort of vouches for the enclosed public key.  This key may 
use any valid signature algorithm.


https://tools.ietf.org/html/rfc2986#section-3 says:

 1. A CertificationRequestInfo value containing a subject
distinguished name, a subject public key, and optionally a
set of attributes is constructed by an entity requesting
certification.

 2. The CertificationRequestInfo value is signed with the subject
entity's private key.  (See Section 4.2.)

It hasn’t said the “public key” and “private key” above should be a pair, 
though.


I believe this is sort of "implicit" because otherwise there would be a need to 
indicate which key that was used in order to verify the signature.

CMC was probably designed to cope with this restriction.
https://tools.ietf.org/html/rfc5272#section-3.2


As a side note, my own applications use a key container attestation key for 
*all* CSRs which is a more useful method than self-signed CSRs.


Interesting. Is there any document describing this feature?


WebAuthn appears to use this method although they only register public keys:
https://www.w3.org/TR/webauthn-2/#sctn-api

My particular take on this is a bit more elaborate because the attestation is 
rather creating a session and shared key which permits secure multi-step key 
(store) management operations:
https://cyberphone.github.io/doc/security/keygen2.html

Thanx,
Anders



Thanks,
Max



Regards,
Anders



Thanks,
Max

On Jan 31, 2021, at 2:12 AM, Anders Rundgren  
wrote:

Since the JDK bug report tool does not include "keytool" I posted this here.

Keytool for JDK 15 reports "Subject Public Key Algorithm: XDH key of unknown 
size" for a certificate  containing the following public key:

148: SEQUENCE {
  150:   SEQUENCE {
  152: OBJECT IDENTIFIER X25519 (1.3.101.110)
 }
  157:   BIT STRING, 32 bytes
   : a3 5e 94 ef bd d0 41 86 90 07 87 9e 80 d0 a5 76 '.^Av'
   0010: 0e a1 ba 82 19 2e c3 90 21 89 05 5a f6 d9 e6 50 '!..Z...P'
   }

which seems to be aligned with: 
https://urldefense.com/v3/__https://tools.ietf.org/html/rfc8410*section-10.2__;Iw!!GqivPVa7Brio!NmY7j2ZVt-VJoTtZkQFA8tbhvHgLZazChGpwWEbycxxjAHm6aDkm8clW3eJ2H14Ugw$
You can verify this issue by importing the certificate in the RFC.

Anders






RFR: 8259886 : Improve SSL session cache performance and scalability

2021-02-01 Thread djelinski
Under certain load, MemoryCache operations take a substantial fraction of the 
time needed to complete SSL handshakes. This series of patches improves 
performance characteristics of MemoryCache, at the cost of a functional change: 
expired entries are no longer guaranteed to be removed before live ones. Unused 
entries are still removed before used ones, and cache performance no longer 
depends on its capacity.

First patch in the series contains a benchmark that can be run with `make test 
TEST="micro:CacheBench"`.
Baseline results before any MemoryCache changes:
Benchmark   (size)  (timeout)  Mode  Cnt ScoreError  Units
CacheBench.put   20480  86400  avgt   2583.653 ?  6.269  us/op
CacheBench.put   20480  0  avgt   25 0.107 ?  0.001  us/op
CacheBench.put  204800  86400  avgt   25  2057.781 ? 35.942  us/op
CacheBench.put  204800  0  avgt   25 0.108 ?  0.001  us/op
there's a nonlinear performance drop between 20480 and 204800 entries, probably 
attributable to CPU cache thrashing. Beyond 204800 entries the cache scales 
more linearly.

Benchmark results after the 2nd and 3rd patches are pretty similar, so I'll 
only copy one:
Benchmark   (size)  (timeout)  Mode  Cnt  Score   Error  Units
CacheBench.put   20480  86400  avgt   25  0.146 ? 0.002  us/op
CacheBench.put   20480  0  avgt   25  0.108 ? 0.002  us/op
CacheBench.put  204800  86400  avgt   25  0.150 ? 0.001  us/op
CacheBench.put  204800  0  avgt   25  0.106 ? 0.001  us/op
The third patch improves worst-case times on a mostly idle cache by scattering 
removal of expired entries over multiple `put` calls. It does not affect 
performance of an overloaded cache.

The 4th patch removes all code that clears cached values before handing them 
over to the GC. [This 
comment](https://github.com/openjdk/jdk/commit/5859a0320334bfb6b46b62eb16b4c387641f4a2a#diff-c6bd583a97fbc4f471621fee7eab37c63718cdb6932ce357fa403cfda4b32b6fL346)
 stated that clearing values was supposed to be a GC performance optimization. 
It wasn't. Benchmark results after that commit:
Benchmark   (size)  (timeout)  Mode  Cnt  Score   Error  Units
CacheBench.put   20480  86400  avgt   25  0.113 ? 0.001  us/op
CacheBench.put   20480  0  avgt   25  0.075 ? 0.002  us/op
CacheBench.put  204800  86400  avgt   25  0.116 ? 0.001  us/op
CacheBench.put  204800  0  avgt   25  0.072 ? 0.001  us/op
I wasn't expecting that much of an improvement, and don't know how to explain 
it.

The 40ns difference between cache with and without a timeout can be attributed 
to 2 `System.currentTimeMillis()` calls; they were pretty slow on my VM.

-

Commit messages:
 - Do not invalidate objects before GC
 - Always expunge on put
 - Stop scanning expired entries after first non-expired one
 - Add cache microbenchmark

Changes: https://git.openjdk.java.net/jdk/pull/2255/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2255=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8259886
  Stats: 138 lines in 3 files changed: 85 ins; 40 del; 13 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2255.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2255/head:pull/2255

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


Re: Keytool does not agree with RFC 8410

2021-02-01 Thread Wei-Jun Wang

> On Feb 1, 2021, at 2:32 AM, Anders Rundgren  
> wrote:
> 
> On 2021-01-31 20:00, Wei-Jun Wang wrote:
>> https://bugs.openjdk.java.net/browse/JDK-8260693 filed.
> 
> Thanx!
> In the bug report you also write:
> 
>We'll also need a way to generate this kind of certificate (or certreq).
>There is no signature algorithm on XDH and we need to use EdDSA instead.
>See 
> https://urldefense.com/v3/__https://tools.ietf.org/html/rfc8410*section-10.2__;Iw!!GqivPVa7Brio!NmY7j2ZVt-VJoTtZkQFA8tbhvHgLZazChGpwWEbycxxjAHm6aDkm8clW3eJ2H14Ugw$
>  .
> 
> AFAIK there is no standard for CSRs for encryption keys.  You need to use a 
> signature key that sort of vouches for the enclosed public key.  This key may 
> use any valid signature algorithm.

https://tools.ietf.org/html/rfc2986#section-3 says:

1. A CertificationRequestInfo value containing a subject
   distinguished name, a subject public key, and optionally a
   set of attributes is constructed by an entity requesting
   certification.

2. The CertificationRequestInfo value is signed with the subject
   entity's private key.  (See Section 4.2.)

It hasn’t said the “public key” and “private key” above should be a pair, 
though.

> 
> As a side note, my own applications use a key container attestation key for 
> *all* CSRs which is a more useful method than self-signed CSRs.

Interesting. Is there any document describing this feature?

Thanks,
Max

> 
> Regards,
> Anders
> 
> 
>> Thanks,
>> Max
>>> On Jan 31, 2021, at 2:12 AM, Anders Rundgren 
>>>  wrote:
>>> 
>>> Since the JDK bug report tool does not include "keytool" I posted this here.
>>> 
>>> Keytool for JDK 15 reports "Subject Public Key Algorithm: XDH key of 
>>> unknown size" for a certificate  containing the following public key:
>>> 
>>> 148: SEQUENCE {
>>>  150:   SEQUENCE {
>>>  152: OBJECT IDENTIFIER X25519 (1.3.101.110)
>>> }
>>>  157:   BIT STRING, 32 bytes
>>>   : a3 5e 94 ef bd d0 41 86 90 07 87 9e 80 d0 a5 76 
>>> '.^Av'
>>>   0010: 0e a1 ba 82 19 2e c3 90 21 89 05 5a f6 d9 e6 50 
>>> '!..Z...P'
>>>   }
>>> 
>>> which seems to be aligned with: 
>>> https://urldefense.com/v3/__https://tools.ietf.org/html/rfc8410*section-10.2__;Iw!!GqivPVa7Brio!NmY7j2ZVt-VJoTtZkQFA8tbhvHgLZazChGpwWEbycxxjAHm6aDkm8clW3eJ2H14Ugw$
>>>  
>>> You can verify this issue by importing the certificate in the RFC.
>>> 
>>> Anders



fix(truststore): log about truststore not found prints every time the same value

2021-02-01 Thread Kevin Davin
Hi everyone,

I'm contacting you because I've opened a PR to fix a bug, but it seems to
be required to have an issue associated with it. The OCA team said to me
I've to ask you to open one for me. Is it ok for you ?

Without it, my PR is blocked https://github.com/openjdk/jdk/pull/1690, and
the bridgekeeper bot said it should be close if there is no further
activity on this PR.

Thank you for your help and support.

Kevin


Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v6]

2021-02-01 Thread Magnus Ihse Bursie
On Mon, 1 Feb 2021 12:35:05 GMT, Alan Hayward 
 wrote:

> > Hello, hsdis is a separate out-of-tree project and is not part of this jep.
> 
> Unless there's something I'm missing it only requires a few lines of change 
> to src/utils/hsdis/makefile (it already has support for macos x86_64)

I agree with Alan that it makes sense to add this trivial change as part of 
this PR, if it allows the current hsdis solution to continue working on 
mac/aarch64.

> 
> > support for looking for proper hsdis dylib library was added as part of 
> > this jep.
> 
> I'm a little confused. Are you planning on adding a new disassembler?

@a74nh I think Vladimir is referring to 
https://github.com/openjdk/jdk/pull/392. The hsdis "component" has been left 
behind for a long time, and there are several requests to add new backends, 
which require a modernized build of hsdis. This is undfortunately not a 
high-priority project, and has been postponed several times already. :(

-

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


Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v6]

2021-02-01 Thread Alan Hayward
On Mon, 1 Feb 2021 11:19:34 GMT, Vladimir Kempik  wrote:

> Hello, hsdis is a separate out-of-tree project and is not part of this jep.

Unless there's something I'm missing it only requires a few lines of change to 
src/utils/hsdis/makefile (it already has support for macos x86_64)

>support for looking for proper hsdis dylib library was added as part of this 
>jep.

I'm a little confused. Are you planning on adding a new disassembler?

-

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


Re: RFR: 8257858: [macOS]: Remove JNF dependency from libosxsecurity/KeystoreImpl.m [v6]

2021-02-01 Thread Magnus Ihse Bursie
On Sun, 31 Jan 2021 18:45:19 GMT, Weijun Wang  wrote:

>> This fix covers both
>> 
>> - [[macOS]: Remove JNF dependency from 
>> libosxsecurity/KeystoreImpl.m](https://bugs.openjdk.java.net/browse/JDK-8257858)
>> - [[macOS]: Remove JNF dependency from 
>> libosxkrb5/SCDynamicStoreConfig.m](https://bugs.openjdk.java.net/browse/JDK-8257860)
>
> Weijun Wang 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 15 additional 
> commits since the last revision:
> 
>  - move test
>  - Merge branch 'master' into 8257858
>  - a test
>
>only in patch2:
>unchanged:
>  - end values should be vectors
>  - phil comment
>  - same behavior as before -- empty realm map
>  - error check, new JavaStringToNSString
>  - do not find class and method in loop
>  - no more header file
>
>reverted:
>  - better macro, no more JNI_COCOA_ENTER
>  - ... and 5 more: 
> https://git.openjdk.java.net/jdk/compare/fdd718db...ef337f12

Basically looks good, but the Obj-C test file needs proper handling.

make/test/JtregNativeJdk.gmk line 84:

> 82:   -framework Cocoa -framework JavaNativeFoundation
> 83:   BUILD_JDK_JTREG_EXECUTABLES_LIBS_exeJniInvocationTest := -ljli
> 84:   BUILD_JDK_JTREG_LIBRARIES_CFLAGS_libTestDynamicStore := -ObjC

Instead of "tricking" the build system of compiling an Obj-C file by 
masquerading it as a C file and passing compiler options, you should expose the 
test file for what it is, and add support in the build system to handle this. I 
can help you with that part.

-

Changes requested by ihse (Reviewer).

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


Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v6]

2021-02-01 Thread Vladimir Kempik
On Mon, 1 Feb 2021 09:31:31 GMT, Alan Hayward 
 wrote:

> You need add macos arm64 to hsdis. Having it working is fairly essential for 
> debugging.
> 
> Inside src/utils/hsdis, After cloning binutils
> 
> ```
> make; make demo; ./build/macosx-arm64/hsdis-demo
> ```
> 
> Results in:
> 
> ```
> Hello, world!
> ...And now for something completely different:
> 
> Decoding from 0x1046e31a4 to 0x1046e3664...with decode_instructions_virtual
> hsdis: bad native mach=architecture not set in Makefile!; please port hsdis 
> to this platform
> hsdis output options:
> ```
> 
> I fixed it by changing the makefile to force the build flags:
> 
> ```
> ARCH=aarch64
> CFLAGS/aarch64+= -m64
> ```
> 
> Resulting in:
> 
> ```
> Hello, world!
> ...And now for something completely different:
> 
> Decoding from 0x10012719c to 0x10012765c...with decode_instructions_virtual
> Decoding for CPU 'aarch64'
> main:
>  0x10012719c  sub sp, sp, #0x60
>  0x1001271a0  stp x29, x30, [sp, #80]
> ...etc
> ```
> 
> Putting the library in the right place then made disassembly in java work for 
> me.

Hello, hsdis is a separate out-of-tree project and is not part of this jep.
support for looking for proper hsdis dylib library was added as part of this 
jep.

-

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


RFR: 8211227: Inconsistent TLS protocol version in debug output

2021-02-01 Thread Evan Whelan
Hi everyone,

Please review my fix for JDK-8211227

This supportability fix will result in a more consistent debug format when 
reading and writing TLS protocol versions.

Thanks,
Evan

-

Commit messages:
 - 8211227: Inconsistent TLS protocol version in debug output

Changes: https://git.openjdk.java.net/jdk/pull/2331/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2331=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8211227
  Stats: 103 lines in 6 files changed: 92 ins; 0 del; 11 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2331.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2331/head:pull/2331

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


Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v6]

2021-02-01 Thread Alan Hayward
On Wed, 27 Jan 2021 14:58:27 GMT, Vladimir Kempik  wrote:

>> Build changes per se now looks okay. However, I agree with Erik that unless 
>> this PR can wait for the JNF removal, at the very least the build docs needs 
>> to be updated to explain how to successfully build for this platform. (I can 
>> live with the configure command line hack, since it's temporary -- otherwise 
>> I'd have requested a new configure argument.) This can be done in this PR or 
>> a follow-up PR.
>
>> Build changes per se now looks okay. However, I agree with Erik that unless 
>> this PR can wait for the JNF removal, at the very least the build docs needs 
>> to be updated to explain how to successfully build for this platform. (I can 
>> live with the configure command line hack, since it's temporary -- otherwise 
>> I'd have requested a new configure argument.) This can be done in this PR or 
>> a follow-up PR.
> 
> I believe it's better be done under separate PR/bugfix, so it can be 
> completely reverted once JNF removed.

You need add macos arm64 to hsdis. Having it working is fairly essential for 
debugging.

Inside src/utils/hsdis, After cloning binutils
make; make demo; ./build/macosx-arm64/hsdis-demo
Results in:
Hello, world!
...And now for something completely different:

Decoding from 0x1046e31a4 to 0x1046e3664...with decode_instructions_virtual
hsdis: bad native mach=architecture not set in Makefile!; please port hsdis to 
this platform
hsdis output options:

I fixed it by changing the makefile to force the build flags:
ARCH=aarch64
CFLAGS/aarch64+= -m64
Resulting in:
Hello, world!
...And now for something completely different:

Decoding from 0x10012719c to 0x10012765c...with decode_instructions_virtual
Decoding for CPU 'aarch64'
main:
 0x10012719csub sp, sp, #0x60
 0x1001271a0stp x29, x30, [sp, #80]
...etc

Putting the library in the right place then made disassembly in java work for 
me.

-

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