JSSE reference guide issue
Hi all, What's the right spot to report documentation issues? I've been reading the JSSE reference guide and noticed that in section "Resuming Session Without Server-Side State" (https://docs.oracle.com/en/java/javase/15/security/java-secure-socket-extension-jsse-reference-guide.html#GUID-64D7EAF6-D2EE-4719-8616-25E2829CF810) it says "This feature is not enabled by default", which appears to be a leftover from Java 13. Also the note about TLS 1.3 in the same section isn't entirely clear to me. What does it mean when the docs say "the contents of stateless tickets, in particular, the contents of a NewSessionTicket message, depend on the value of jdk.tls.server.enableSessionTicketExtension"? How exactly does the contents change and why should I care? Regards, Daniel
Integrated: 8163498: Many long-running security libs tests
On Wed, 3 Feb 2021 13:29:54 GMT, Fernando Guallini wrote: > The following tests have been split based on lower/higher key sizes in order > to reduce individual execution time and run in parallel with jtreg > sun/security/provider/DSA/SupportedDSAParamGen.java > sun/security/provider/NSASuiteB/TestDSAGenParameterSpec.java > com/sun/crypto/provider/KeyAgreement/SupportedDHParamGens.java > > sun/security/rsa/SignatureTest.java is now using a fake generator since its > objective is to test signature not key generation itself. In addition, it was > generating and verifying the same key twice, with same modulus and exponent. > That redundancy is removed. This speeds up the execution from ~54s to ~25s on > average This pull request has now been integrated. Changeset: d2bd4992 Author:Fernando Guallini Committer: Rajan Halade URL: https://git.openjdk.java.net/jdk/commit/d2bd4992 Stats: 285 lines in 8 files changed: 259 ins; 9 del; 17 mod 8163498: Many long-running security libs tests Reviewed-by: rhalade, weijun - PR: https://git.openjdk.java.net/jdk/pull/2381
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v9]
On Tue, 2 Feb 2021 22:09:58 GMT, Daniel D. Daugherty wrote:
>> Anton Kozlov has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> support macos_aarch64 in hsdis
>
> src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp line 363:
>
>> 361: address pc = os::Posix::ucontext_get_pc(uc);
>> 362:
>> 363: if (pc != addr && uc->context_esr == 0x924F) { //TODO: figure
>> out what this value means
>
> Is this TODO going to be resolved by this port?
Where did this come from - some snippet/example/tech note code? Maybe other
people can help figure it out if we provide more info.
> src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp line 374:
>
>> 372:
>> 373: last_addr = (address) -1;
>> 374: } else if (pc == addr && uc->context_esr == 0x820f) { //TODO:
>> figure out what this value means
>
> Is this TODO going to be resolved by this port?
Where did this come from - some snippet/example/tech note code? Maybe other
people can help figure it out if we provide more info.
-
PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v10]
On Wed, 3 Feb 2021 20:01:15 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 incrementally with six additional
> commits since the last revision:
>
> - Merge remote-tracking branch 'origin/jdk/jdk-macos' into jdk-macos
> - Add comments to WX transitions
>
>+ minor change of placements
> - Use macro conditionals instead of empty functions
> - Add W^X to tests
> - Do not require known W^X state
> - Revert w^x in gtests
src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp line 403:
> 401: }
> 402:
> 403: return false; // Mute compiler
Is this comment needed?
src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp line 420:
> 418: size_t os::Posix::_compiler_thread_min_stack_allowed = 72 * K;
> 419: size_t os::Posix::_java_thread_min_stack_allowed = 72 * K;
> 420: size_t os::Posix::_vm_internal_thread_min_stack_allowed = 72 * K;
Those are slightly larger than their x86_64 counter parts. Are they
conservative/aggressive values? How did we arrive at those?
src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp line 652:
> 650:
> 651: void os::setup_fpu() {
> 652: }
Is there really nothing to do here, or does still need to be implemented? A
clarification comment here would help/.
-
PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8258915: Temporary buffer cleanup [v5]
On Thu, 4 Feb 2021 15:20:02 GMT, Valerie Peng wrote: >> Weijun Wang has updated the pull request incrementally with one additional >> commit since the last revision: >> >> cleanups for key generations > > src/java.base/share/classes/com/sun/crypto/provider/AESCrypt.java line 690: > >> 688: limit = ROUNDS*4; >> 689: >> 690: // store the expanded sub keys into 'sessionK' > > Update the comment with "erase the previous values in sessionK or other > similar wording. OK. - PR: https://git.openjdk.java.net/jdk/pull/2070
Re: RFR: 8258915: Temporary buffer cleanup
On Thu, 4 Feb 2021 16:13:11 GMT, Valerie Peng wrote: > > New commit for key generations. > > How about the Tls*Generator classes in SunJCE provider? Looks like they need > to be handled as well. I'll take a look. I thought the secrets going in and out of them are ephemeral. - PR: https://git.openjdk.java.net/jdk/pull/2070
Re: RFR: 8258915: Temporary buffer cleanup [v5]
On Thu, 4 Feb 2021 15:25:14 GMT, Valerie Peng wrote: >> Weijun Wang has updated the pull request incrementally with one additional >> commit since the last revision: >> >> cleanups for key generations > > src/java.base/share/classes/com/sun/crypto/provider/DESedeKeyGenerator.java > line 2: > >> 1: /* >> 2: * Copyright (c) 1997, 2021, Oracle and/or its affiliates. All rights >> reserved. > > No changes? Why update copyright year? Oops. Will revert. - PR: https://git.openjdk.java.net/jdk/pull/2070
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v10]
On Wed, 3 Feb 2021 20:01:15 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 incrementally with six additional > commits since the last revision: > > - Merge remote-tracking branch 'origin/jdk/jdk-macos' into jdk-macos > - Add comments to WX transitions > >+ minor change of placements > - Use macro conditionals instead of empty functions > - Add W^X to tests > - Do not require known W^X state > - Revert w^x in gtests I reviewed bsd_aarch64.cpp digging bit deeper and left some comments. - Changes requested by gziemski (Committer). PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v10]
On Wed, 3 Feb 2021 20:01:15 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 incrementally with six additional
> commits since the last revision:
>
> - Merge remote-tracking branch 'origin/jdk/jdk-macos' into jdk-macos
> - Add comments to WX transitions
>
>+ minor change of placements
> - Use macro conditionals instead of empty functions
> - Add W^X to tests
> - Do not require known W^X state
> - Revert w^x in gtests
src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp line 297:
> 295: stub = SharedRuntime::handle_unsafe_access(thread, next_pc);
> 296: }
> 297: } else if (sig == SIGILL && nativeInstruction_at(pc)->is_stop()) {
Can we add a comment here describing what this case means?
-
PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v10]
On Wed, 3 Feb 2021 20:01:15 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 incrementally with six additional > commits since the last revision: > > - Merge remote-tracking branch 'origin/jdk/jdk-macos' into jdk-macos > - Add comments to WX transitions > >+ minor change of placements > - Use macro conditionals instead of empty functions > - Add W^X to tests > - Do not require known W^X state > - Revert w^x in gtests src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp line 302: > 300: const uint64_t *detail_msg_ptr > 301: = (uint64_t*)(pc + NativeInstruction::instruction_size); > 302: const char *detail_msg = (const char *)*detail_msg_ptr; Where is `detail_msg` used? - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v10]
On Wed, 3 Feb 2021 20:01:15 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 incrementally with six additional > commits since the last revision: > > - Merge remote-tracking branch 'origin/jdk/jdk-macos' into jdk-macos > - Add comments to WX transitions > >+ minor change of placements > - Use macro conditionals instead of empty functions > - Add W^X to tests > - Do not require known W^X state > - Revert w^x in gtests src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp line 237: > 235: os::Posix::ucontext_set_pc(uc, > StubRoutines::continuation_for_safefetch_fault(pc)); > 236: return true; > 237: } Isn't this case already handled by `JVM_HANDLE_XXX_SIGNAL()` ? Why do we need it here again? - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v10]
On Wed, 3 Feb 2021 20:01:15 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 incrementally with six additional
> commits since the last revision:
>
> - Merge remote-tracking branch 'origin/jdk/jdk-macos' into jdk-macos
> - Add comments to WX transitions
>
>+ minor change of placements
> - Use macro conditionals instead of empty functions
> - Add W^X to tests
> - Do not require known W^X state
> - Revert w^x in gtests
src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp line 198:
> 196:
> 197: NOINLINE frame os::current_frame() {
> 198: intptr_t *fp = *(intptr_t **)__builtin_frame_address(0);
In the bsd_x86 counter part we initialize `fp` to `_get_previous_fp()` - do we
need to implement it on aarch64 as well (and using address 0 is just a temp
workaround) or is it doing the right thing here?
-
PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v10]
On Wed, 3 Feb 2021 20:01:15 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 incrementally with six additional
> commits since the last revision:
>
> - Merge remote-tracking branch 'origin/jdk/jdk-macos' into jdk-macos
> - Add comments to WX transitions
>
>+ minor change of placements
> - Use macro conditionals instead of empty functions
> - Add W^X to tests
> - Do not require known W^X state
> - Revert w^x in gtests
src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp line 291:
> 289: bool is_unsafe_arraycopy = (thread->doing_unsafe_access() &&
> UnsafeCopyMemory::contains_pc(pc));
> 290: if ((nm != NULL && nm->has_unsafe_access()) ||
> is_unsafe_arraycopy) {
> 291: address next_pc = pc + NativeCall::instruction_size;
Replace
address next_pc = pc + NativeCall::instruction_size;
with
address next_pc = Assembler::locate_next_instruction(pc);
there is at least one other place that needs it.
src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp line 322:
> 320: #ifdef __APPLE__
> 321: } else if (sig == SIGFPE && info->si_code == FPE_NOOP) {
> 322: Unimplemented();
Is there a follow up issue for this?
-
PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v10]
On Wed, 3 Feb 2021 20:01:15 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 incrementally with six additional
> commits since the last revision:
>
> - Merge remote-tracking branch 'origin/jdk/jdk-macos' into jdk-macos
> - Add comments to WX transitions
>
>+ minor change of placements
> - Use macro conditionals instead of empty functions
> - Add W^X to tests
> - Do not require known W^X state
> - Revert w^x in gtests
src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp line 194:
> 192: // may get turned off by -fomit-frame-pointer.
> 193: frame os::get_sender_for_C_frame(frame* fr) {
> 194: return frame(fr->link(), fr->link(), fr->sender_pc());
Why is it
return frame(fr->link(), fr->link(), fr->sender_pc());
and not
return frame(fr->sender_sp(), fr->link(), fr->sender_pc());
like in the bsd-x86 counter part?
-
PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v9]
On Thu, 4 Feb 2021 15:13:49 GMT, Anton Kozlov wrote: >> Out of curiosity, have you looked at the performance of the W^X state >> transition? In particular I'm wondering if the cost is effectively >> negligible so doing it unconditionally on JVM entry is a no-brainer and just >> easier/cleaner than the alternatives, or if there are reasons to look at >> only doing the transition if/when needed (perhaps do it lazily and revert >> back to X when leaving the JVM?). > >> Out of curiosity, have you looked at the performance of the W^X state >> transition? > > Earlier it was possible to disable W^X protection (unfortunately, I don't > know a way to do this now). We compared Renaissance results with W^X > transitions like the proposed one vs. no transitions with the protection > disabled once. Results were identical for a small and large number of > iterations. > > From the other hand, I've used > https://github.com/AntonKozlov/macos-aarch64-transition-bench to estimate the > cost of the transition. > I re-did measurements with the current implementation and on consumer > hardware: > > testJNIthrpt 25 277997000.151 ± 4095685.956 ops/s > testJniNanoTimethrpt 2517851098.010 ±119489.599 ops/s > testNanoTime thrpt 2578007491.762 ±628455.971 ops/s > testNothingthrpt 25 1724298829.088 ± 100537565.068 ops/s > testTwoStateAndWX thrpt 2521958839.057 ±210490.755 ops/s > testWX thrpt 2523299813.266 ±149837.302 ops/s > > There is an overhead, but it does not look like blocking the first > implementation. I'm not refusing future optimizations like enabling W only > when necessary. But for now, I don't have a robust and maintainable solution > for this, sorry. > _Mailing list message from [erik.joelsson at > oracle.com](mailto:erik.joels...@oracle.com) on > [2d-dev](mailto:2d-...@openjdk.java.net):_ > > On 2021-01-26 04:44, Magnus Ihse Bursie wrote: > > > On 2021-01-26 13:09, Vladimir Kempik wrote: > > > On Tue, 26 Jan 2021 12:02:02 GMT, Alan Hayward > > > wrote: > > > > AIUI, the configure line needs passing a prebuilt > > > > JavaNativeFoundation framework > > > > ie: > > > > `--with-extra-ldflags='-F > > > > /Applications/Xcode.app/Contents/SharedFrameworks/ContentDeliveryServices.framework/Versions/A/itms/java/Frameworks/'` > > > > Otherwise there will be missing _JNFNative* functions. > > > > Is this the long term plan? Or will eventually the required code be > > > > moved into JDK and/or the xcode one automatically get picked up by > > > > the configure scripts? > > > > There is ongoing work by P. Race to eliminate dependence on JNF at all > > > > How far has that work come? Otherwise the logic should be added to > > > > configure to look for this framework automatically, and provide a way > > > > to override it/set it if not found. > > > > > > I don't think it's OK to publish a new port that cannot build > > out-of-the-box without hacks like this. > > My understanding is that Apple chose to not provide JNF for aarch64, so > if you want to build OpenJDK, you first need to build JNF yourself (it's > available in github). Phil is working on removing this dependency > completely, which will solve this issue [1]. > > In the meantime, I don't think we should rely on finding JNF in > unsupported locations inside Xcode. Could we wait with integrating this > port until it can be built without such hacks? If not, then adding > something in the documentation on how to get a working JNF would at > least be needed. > > /Erik > > [1] https://bugs.openjdk.java.net/browse/JDK-8257852 This doesn't seem to be an issue anymore, After P.Race have finished with JDK-8257852, Macarm port can be build without extra ld flags. J2Demo works fine as example. This can be checked if one merges pull/2200 branch into his local copy of master branch. - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8259886 : Improve SSL session cache performance and scalability [v2]
On Thu, 4 Feb 2021 19:36:24 GMT, Xue-Lei Andrew Fan wrote: >> Added benchmarks for get & remove. Added tests for 5M cache size. Switched >> time units to nanoseconds. Results: >> Benchmark (size) (timeout) Mode CntScore Error Units >> CacheBench.get 20480 86400 avgt 25 62.999 ? 2.017 ns/op >> CacheBench.get 20480 0 avgt 25 41.519 ? 1.113 ns/op >> CacheBench.get 204800 86400 avgt 25 67.995 ? 4.530 ns/op >> CacheBench.get 204800 0 avgt 25 46.439 ? 2.222 ns/op >> CacheBench.get 512 86400 avgt 25 72.516 ? 0.759 ns/op >> CacheBench.get 512 0 avgt 25 53.471 ? 0.491 ns/op >> CacheBench.put 20480 86400 avgt 25 117.117 ? 3.424 ns/op >> CacheBench.put 20480 0 avgt 25 73.582 ? 1.484 ns/op >> CacheBench.put 204800 86400 avgt 25 116.983 ? 0.743 ns/op >> CacheBench.put 204800 0 avgt 25 73.945 ? 0.515 ns/op >> CacheBench.put 512 86400 avgt 25 230.878 ? 7.582 ns/op >> CacheBench.put 512 0 avgt 25 192.526 ? 7.048 ns/op >> CacheBench.remove20480 86400 avgt 25 39.048 ? 2.036 ns/op >> CacheBench.remove20480 0 avgt 25 36.293 ? 0.281 ns/op >> CacheBench.remove 204800 86400 avgt 25 43.899 ? 0.895 ns/op >> CacheBench.remove 204800 0 avgt 25 43.046 ? 0.759 ns/op >> CacheBench.remove 512 86400 avgt 25 51.896 ? 0.640 ns/op >> CacheBench.remove 512 0 avgt 25 51.537 ? 0.536 ns/op > > Thank you for the comment. The big picture is more clear to me now. > >> Example 2: >> Old implementation will get: >> |K=3, exp=10|K=5, exp=12|K=7, exp=14|K=9, exp=16| >> >> New implementation will get: >> |K=5, exp=12|K=7, exp=14|K=1, exp=8(expired)|K=9, exp=16| > > K=3 is not expired yet, but get removed, while K=1 is kept. This behavior > change may cause more overall performance hurt than improving the cache > put/get performance. For example, it need to grab the value remotely. A > full handshake or OCSP status grabbing could counteract all the performance > gain with the cache update. > >> All calls to put() remove expired items from the front of the queue, and >> never perform a full scan. get() calls shuffle the queue, moving the >> accessed item to the back. Compare this to original code where put() only >> removed expired items when the cache overflowed, and scanned the entire >> cache. > > I think the idea that put() remove expired items from the front of the queue > is good. I was wondering if it is an option to have the get() method that > removed expired items until the 1st un-expired item, without scan the full > queue and change the order of the queue. But there is still an issue that > the SoftReference may have clear an item, which may be still valid. > > In general, I think the get() performance is more important than put() > method, as get() is called more frequently. So we should try to keep the > cache small if possible. > >>> increase the size to some big scales, like 2M and 20M >> >> Can do. Do you think it makes sense to also benchmark the scenario where GC >> kicks in and collects soft references? > > In the update, the SoftReference.clear() get removed. I'm not sure of the > impact of the enqueued objects any longer. In theory, it could improve the > memory use, which could counteract the performance gain in some situation. > >> Also, what do you think about the changes done in Do not invalidate objects >> before GC 5859a03 commit? > See above, it is a concern to me that the soft reference cannot be cleared > with this update. > >> How do I file a CSR? > Could you edit the bug: https://bugs.openjdk.java.net/browse/JDK-8259886? In > the more drop down menu, there is a "Create CSR" option. You can do it if we > have an agreement about the solution and impact. Thanks for your review! Some comments below. > A full handshake or OCSP status grabbing could counteract all the performance > gain with the cache update. Yes, but that's unlikely. Note that K=3 is before K=1 in the queue only because 3 wasn't used since 1 was last used. This means that either K=3 is used less frequently than K=1, or that all cached items are in active use. In the former case we don't lose much by dropping K=3, and in the latter we are dealing with full cache at all times, which means that most `put()`s remove un-expired items anyway. > get() [..] without [..] change the order of the queue If we do that, frequently used entries will be evicted at the same age as never used ones. This means we will have to recompute (full handshake/fresh OCSP) both the frequently used and the infrequently used entries. It's better to recompute only the infrequently used ones, and reuse the frequently used as long as possible - we will do less work that way. > get() performance
Re: RFR: 8259886 : Improve SSL session cache performance and scalability [v2]
On Thu, 4 Feb 2021 19:17:21 GMT, djelinski wrote: >>> the benchmark performance improvement is a trade-off between CPU and >>> memory, by keeping expired entries while putting a new entry in the cache >> >> Not exactly. The memory use is capped by cache size. The patch is a trade >> off between the cache's hit/miss ratio and CPU; we will get faster cache >> access at the cost of more frequent cache misses. >> >> All calls to `put()` remove expired items from the front of the queue, and >> never perform a full scan. `get()` calls shuffle the queue, moving the >> accessed item to the back. Compare this to original code where `put()` only >> removed expired items when the cache overflowed, and scanned the entire >> cache. >> Let me give some examples. >> **Example 1**: insertions at a fast pace leading to cache overflows and no >> expirations. Here the new implementation improves performance. Consider a >> cache with size=4, timeout=10, and the following sequence of events: >> T=1, put(1); >> T=2, put(2); >> T=3, put(3); >> T=4, put(4); >> Cache contents after these calls (same in old and new scenario). Queue >> order: least recently accessed items on the left, most recently accessed on >> the right. _K_ denotes cache key, _exp_ denotes entry expiration time and is >> equal to insertion time _T_ plus timeout: >> >> |K=1, exp=11|K=2, exp=12|K=3, exp=13|K=4, exp=14| >> >> If we now add another item to the queue, it will overflow. Here's where the >> implementations behave differently, but the outcome is identical: old one >> scans the entire list for expired entries; new one improves performance by >> ending the search for expired entries after encountering the first >> non-expired entry (which is the first entry in the above example). The end >> result is the same in both cases - oldest (least recently accessed) item is >> dropped: >> T=5, put(5) >> >> |K=2, exp=12|K=3, exp=13|K=4, exp=14|K=5, exp=15| >> >> **Example 2**: insertions at a moderate pace, with interleaved reads. Here >> the new implementation improves performance, but at a possible cost of >> wasting cache capacity on expired entries. Consider a cache with size=4, >> timeout=7, and the following sequence of events: >> T=1, put(1); >> T=3, put(3); >> T=5, put(5); >> T=7, put(7); >> T=7, get(1); >> Cache contents after these calls: >> >> |K=3, exp=10|K=5, exp=12|K=7, exp=14|K=1, exp=8| >> >> `get(1)` operation moved item with K=1 to the back of the queue. >> >> If we wait for item with K=1 to expire and then add another item to the >> queue, it will overflow. Here's where the implementations behave >> differently, and the outcome is different: old one scans the entire list for >> expired entries, finds entry with K=1 and drops it; new one gives up after >> first non-expired entry (which is the first entry), and drops the first >> entry. >> >> So, when we perform: >> T=9, put(9); >> >> Old implementation will get: >> |K=3, exp=10|K=5, exp=12|K=7, exp=14|K=9, exp=16| >> >> New implementation will get: >> |K=5, exp=12|K=7, exp=14|K=1, exp=8(expired)|K=9, exp=16| >> >> Note that: >> - an attempt to retrieve expired item (i.e. `get(1)`) will immediately >> remove that item from cache, making room for other items >> - retrieving a non-expired item will move it to the back of the queue, >> behind all expired items >> >> **Example 3**: insertions at a slow pace, where most items expire before >> queue overflows. Here the new implementation improves memory consumption. >> Consider a cache with size=4, timeout=1, and the following sequence of >> events: >> T=1, put(1); >> T=3, put(3); >> T=5, put(5); >> T=7, put(7); >> Every cache item is expired at then point when a new one is added. Old >> implementation only removes expired entries when cache overflows, so all >> entries will still be there: >> >> |K=1, exp=2(expired)|K=3, exp=4(expired)|K=5, exp=6(expired)|K=7, exp=8| >> >> New implementation removes expired entries on every put, so after the last >> put only one entry is in the cache: >> >> |K=7, exp=8| >> >> After another put the old implementation will encounter a cache overflow and >> remove all expired items. >> >> Let me know if that helped. >> >>> add two more types of benchmark: get the entries and remove the entries >> >> Both these operations are constant-time, both before and after my changes. >> Do you expect to see some oddities here, or do we just want a benchmark that >> could be used to compare other implementations? >> >>> increase the size to some big scales, like 2M and 20M >> >> Can do. Do you think it makes sense to also benchmark the scenario where GC >> kicks in and collects soft references? >> >>> it may change the behavior of a few JDK components >> >> Of all uses of Cache, only `SSLSessionContextImpl` (TLS session cache), >> `StatusResponseManager` (OCSP stapling) and `LDAPCertStoreImpl` (I'm not >> familiar with that one) set expiration timeout; when the timeo
OpenJDK-TLS Manual: (Second) call for contributions
I've written an OpenJDK-TLS manual, intended to ease readers into the most recent TLS specification and OpenJDK's implementation. (At the very least, it helped me get to grips with the spec and the code!) I've made the manual available on GitHub (https://github.com/BenSmyth/tls-tutorial/) and a pdf is also available: https://bensmyth.com/files/Smyth19-TLS-tutorial.pdf I hope the manual will help you too. I'm far from perfect and I'm sure the manuscript has numerous deficiencies. Interesting aspects are omitted, because I didn't have the time, knowledge, or expertise to add them. In particular, my coverage of OpenJDK could be substantially improved and extended. I encourage you to improve the manuscript. Fix a typo. Patch grammar. Revise awkward, overcomplicated, or otherwise poorly-written passages, and correct my outright mistakes. Contribute an entire section, there's certainly a lot of scope regarding OpenJDK. Help evolve the manual. (Perhaps get in touch prior to writing an entire section! We should probably reach consensus on what to add.) Contributions will be recognised through acknowledgements or co-authorship. Best regards, Ben -- https://bensmyth.com/
Re: RFR: 8259886 : Improve SSL session cache performance and scalability [v2]
On Tue, 2 Feb 2021 12:19:22 GMT, djelinski wrote: >> 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. > >> the benchmark performance improvement is a trade-off between CPU and memory, >> by keeping expired entries while putting a new entry in the cache > > Not exactly. The memory use is capped by cache size. The patch is a trade off > between the cache's hit/miss ratio and CPU; we will get faster cache access > at the cost of more frequent cache misses. > > All calls to `put()` remove expired items from the front of the queue, and > never perform a full scan. `get()` calls shuffle the queue, moving the > accessed item to the back. Compare this to original code where `put()` only > removed expired items when the cache overflowed, and scanned the entire cache. > Let me give some examples. > **Example 1**: insertions at a fast pace leading to cache overflows and no > expirations. Here the new implementation improves performance. Consider a > cache with size=4, timeout=10, and the following sequence of events: > T=1, put(1); > T=2, put(2); > T=3, put(3); > T=4, put(4); > Cache contents after these calls (same in old and new scenario). Queue order: > least recently accessed items on the left, most recently accessed on the > right. _K_ denotes cache key, _exp_ denotes entry expiration time and is > equal to insertion time _T_ plus timeout: > > |K=1, exp=11|K=2, exp=12|K=3, exp=13|K=4, exp=14| > > If we now add another item to the queue, it will overflow. Here's where the > implementations behave differently, but the outcome is identical: old one > scans the entire list for expired entries; new one improves performance by > ending the search for expired entries after encountering the first > non-expired entry (which is the first entry in the above example). The end > result is the same in both cases - oldest (least recently accessed) item is > dropped: > T=5, put(5) > > |K=2, exp=12|K=3, exp=13|K=4, exp=14|K=5, exp=15| > > **Example 2**: insertions at a moderate pace, with interleaved reads. Here > the new implementation improves performance, but at a possible cost of > wasting cache capacity on expired entries. Consider a cache with size=4, > timeout=7, and the following sequence of events: > T=1, put(1); > T=3, put(3); > T=5, put(5); > T=7, put(7); > T=7, get(1); > Cache contents after these calls: > > |K=3, exp=10|K=5, exp=12|K=7, exp=14|K=1, exp=8| > > `get(1)` operation moved item with K=1 to the back of the queue. > > If we wait for item with K=1 to expire and then add another item to the > queue, it will overflow. Here's where the implementations behave differently, > and the outcome is different: old one scans the entire list for expired > entries, finds entry with K=1 and drops it; new one gives up after first > non-expired entry (which is the first entry), and drops the first entry. > > So, when we perform: > T=9, put(9); > > Old implementation will get: > |K=3, exp=10|K=5, exp=12|K=7, exp=14|K=9, exp=16| > > New implementation will get: > |K=5, exp=12|K=7, exp=14|K=1, exp=8(expired)|K=9, exp=16| > > Note that: > - an attempt to retrieve expired item (i.e. `get(1)`) will immediately remove > that item from cache, making room for other items > - retrieving a non-expired item will move it to the back of the queue, behind > all expired items > > **Example 3**: insertions at a slow pace, where most items expire before > queue overflows. Here the new implementation improves memory consumption. > Consider a cache with size=4, timeout=1, and the following sequence of events: > T=1, put(1); > T=3, put(3); > T=5, put(5); > T=7, put(7); > Every cache item is expired at then point when a new one is added. Old > implementation only removes expired entries when cache overflows, so all > entries will still be there: > > |K=1, exp=2(expired)|K=3, exp=4(expired)|K=5, exp=6(expired)|K=7, exp=8| > > New implementation removes expired entries on every put, so after the last > put only one entry is in the cache: > > |K=7, exp=8| > > After another put the old implementation will encounter a cache overflow and > r
Re: RFR: 8259886 : Improve SSL session cache performance and scalability [v3]
> 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: Add benchmarks for get and remove - Changes: - all: https://git.openjdk.java.net/jdk/pull/2255/files - new: https://git.openjdk.java.net/jdk/pull/2255/files/34949970..abe0e238 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2255&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2255&range=01-02 Stats: 76 lines in 1 file changed: 54 ins; 4 del; 18 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: 8163498: Many long-running security libs tests [v4]
On Thu, 4 Feb 2021 16:08:09 GMT, Fernando Guallini wrote: >> The following tests have been split based on lower/higher key sizes in order >> to reduce individual execution time and run in parallel with jtreg >> sun/security/provider/DSA/SupportedDSAParamGen.java >> sun/security/provider/NSASuiteB/TestDSAGenParameterSpec.java >> com/sun/crypto/provider/KeyAgreement/SupportedDHParamGens.java >> >> sun/security/rsa/SignatureTest.java is now using a fake generator since its >> objective is to test signature not key generation itself. In addition, it >> was generating and verifying the same key twice, with same modulus and >> exponent. That redundancy is removed. This speeds up the execution from ~54s >> to ~25s on average > > Fernando Guallini has updated the pull request incrementally with one > additional commit since the last revision: > > add short description regarding test split. Perfect. - Marked as reviewed by weijun (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/2381
Re: RFR: 8258915: Temporary buffer cleanup
On Thu, 28 Jan 2021 16:13:01 GMT, Weijun Wang wrote: > > > New commit for key generations. How about the Tls*Generator classes in SunJCE provider? Looks like they need to be handled as well. - PR: https://git.openjdk.java.net/jdk/pull/2070
Re: RFR: 8163498: Many long-running security libs tests [v4]
> The following tests have been split based on lower/higher key sizes in order > to reduce individual execution time and run in parallel with jtreg > sun/security/provider/DSA/SupportedDSAParamGen.java > sun/security/provider/NSASuiteB/TestDSAGenParameterSpec.java > com/sun/crypto/provider/KeyAgreement/SupportedDHParamGens.java > > sun/security/rsa/SignatureTest.java is now using a fake generator since its > objective is to test signature not key generation itself. In addition, it was > generating and verifying the same key twice, with same modulus and exponent. > That redundancy is removed. This speeds up the execution from ~54s to ~25s on > average Fernando Guallini has updated the pull request incrementally with one additional commit since the last revision: add short description regarding test split. - Changes: - all: https://git.openjdk.java.net/jdk/pull/2381/files - new: https://git.openjdk.java.net/jdk/pull/2381/files/56ff061e..e408a627 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2381&range=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2381&range=02-03 Stats: 41 lines in 7 files changed: 37 ins; 1 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/2381.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2381/head:pull/2381 PR: https://git.openjdk.java.net/jdk/pull/2381
Re: RFR: 8163498: Many long-running security libs tests [v3]
On Wed, 3 Feb 2021 22:30:02 GMT, Weijun Wang wrote: >> Fernando Guallini has updated the pull request incrementally with one >> additional commit since the last revision: >> >> add key itself to signature verification > > test/jdk/sun/security/provider/NSASuiteB/TestDSAGenParameterSpecLongKey.java > line 30: > >> 28: * DSA within some certain range of key sizes as described in >> the class >> 29: * specification (L, N) as (1024, 160), (2048, 224), (2048, >> 256) and >> 30: * (3072, 256) should be OK for DSAGenParameterSpec. > > Please modify the `@summary` to match what this test does. sure, short description about this change was added to summary > test/jdk/sun/security/rsa/SignatureTest.java line 145: > >> 143: Asserts.assertTrue(keySpecEquals(pubKeySpec1, >> pubKeySpec2), >> 144: "Both RSAPublicKeySpec should be equal"); >> 145: > > Do you also want to add similar check on X509EncodedKeySpec? Same for > PKCS8EncodedKeySpec below. Both verifications included now - PR: https://git.openjdk.java.net/jdk/pull/2381
Re: RFR: 8258915: Temporary buffer cleanup [v5]
On Thu, 28 Jan 2021 16:12:58 GMT, Weijun Wang wrote: >> Clean up temporary byte array, char array, and keyspec around keys and >> passwords. >> >> No new regression test. > > Weijun Wang has updated the pull request incrementally with one additional > commit since the last revision: > > cleanups for key generations src/java.base/share/classes/com/sun/crypto/provider/DESedeKeyGenerator.java line 2: > 1: /* > 2: * Copyright (c) 1997, 2021, Oracle and/or its affiliates. All rights > reserved. No changes? Why update copyright year? - PR: https://git.openjdk.java.net/jdk/pull/2070
Re: RFR: 8258915: Temporary buffer cleanup [v5]
On Thu, 28 Jan 2021 16:12:58 GMT, Weijun Wang wrote: >> Clean up temporary byte array, char array, and keyspec around keys and >> passwords. >> >> No new regression test. > > Weijun Wang has updated the pull request incrementally with one additional > commit since the last revision: > > cleanups for key generations src/java.base/share/classes/com/sun/crypto/provider/AESCrypt.java line 690: > 688: limit = ROUNDS*4; > 689: > 690: // store the expanded sub keys into 'sessionK' Update the comment with "erase the previous values in sessionK or other similar wording. - PR: https://git.openjdk.java.net/jdk/pull/2070
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v9]
On Wed, 3 Feb 2021 20:08:28 GMT, Mikael Vidstedt wrote: > Out of curiosity, have you looked at the performance of the W^X state > transition? Earlier it was possible to disable W^X protection (unfortunately, I don't know a way to do this now). We compared Renaissance results with W^X transitions like the proposed one vs. no transitions with the protection disabled once. Results were identical for a small and large number of iterations. >From the other hand, I've used >https://github.com/AntonKozlov/macos-aarch64-transition-bench to estimate the >cost of the transition. I re-did measurements with the current implementation and on consumer hardware: testJNIthrpt 25 277997000.151 ± 4095685.956 ops/s testJniNanoTimethrpt 2517851098.010 ±119489.599 ops/s testNanoTime thrpt 2578007491.762 ±628455.971 ops/s testNothingthrpt 25 1724298829.088 ± 100537565.068 ops/s testTwoStateAndWX thrpt 2521958839.057 ±210490.755 ops/s testWX thrpt 2523299813.266 ±149837.302 ops/s There is an overhead, but it does not look like blocking the first implementation. I'm not refusing future optimizations like enabling W only when necessary. But for now, I don't have a robust and maintainable solution for this, sorry. - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8241372: Several test failures due to javax.net.ssl.SSLException: Connection reset
On Thu, 4 Feb 2021 13:00:47 GMT, Fernando Guallini
wrote:
> The server side is binding to the wildcard address which has been a source of
> instability in many networking tests due to javax.net.ssl.SSLException:
> Connection reset. Changing the following tests to bind to loopback address
> fixes intermittent failures:
> sun/security/ssl/SSLSocketImpl/ReverseNameLookup.java
> javax/net/ssl/TLSCommon/TLSTest.java
> sun/security/ssl/CipherSuite/SupportedGroups.java
>
>
> javax/net/ssl/SSLSession/TestEnabledProtocols.java still throws a connection
> reset occasionally because the server closes the connection before the client
> is done during handshake. That race condition cannot be completely removed in
> this test, thus is now handled and logged.
test/jdk/javax/net/ssl/SSLSession/TestEnabledProtocols.java line 29:
> 27: /*
> 28: * @test
> 29: * @bug 4416068 4478803 4479736 8241372
Here and in the other tests:
If a fix is a test bug and only contains test changes, we usually don't add the
bug id to the modified tests. Instead we add the label `noreg-self` to the JBS
issue. The bug ids in the tests files are supposed to identify *source changes*
that can be verified by running the test.
test/jdk/javax/net/ssl/SSLSession/TestEnabledProtocols.java line 145:
> 143: // The server side may have closed the socket.
> 144: System.out.println("Client SSLException:");
> 145: ssle.printStackTrace(System.out);
If security / TLS experts agree that this is OK, then it is OK for me. Not
being an expert, I'd be concerned that catching plain SSLException might be too
wide and that this might hide errors. In that case, and if the error is
intermittent, one possibility could be to respin the test in case of
SSLException (do client side and server side again) and fail if the second
attempts fails too.
test/jdk/sun/security/ssl/SSLSocketImpl/ReverseNameLookup.java line 90:
> 88: (SSLServerSocketFactory) SSLServerSocketFactory.getDefault();
> 89: InetSocketAddress socketAddress =
> 90: new InetSocketAddress(InetAddress.getLoopbackAddress(),
> serverPort);
The client is using 127.0.0.1. InetAddress.getLoopbackAddress() might return
::1 (IPv6 loopback) on certain configurations.
Two possibility:
1. run this test with -Djava.net.preferIPv4Stack to force usage of IPv4, but
trivially pass if IPv4 is not available on the machine (see IPSupport in the
test library)
2. use InetAddress.getLoopbackAddress().getHostAddress() on the client side
instead of "127.0.0.1"
If you choose 2. you may want to add an additional @run the test with
-Djava.net.preferIPv6Address to verify that it works with IPv6 - as there are
several ways to represent the IPv6 loopback as a string
-
PR: https://git.openjdk.java.net/jdk/pull/2405
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v9]
On Thu, 4 Feb 2021 14:27:53 GMT, Andrew Haley wrote: > > > You read my mind, Andrew. Unless, of course, it's optimized to leverage > > > the fact that it's thread-specific.. > > > > > > it's thread-specific > > https://developer.apple.com/documentation/apple_silicon/porting_just-in-time_compilers_to_apple_silicon > > > Because pthread_jit_write_protect_np changes only the current thread’s > > > permissions, avoid accessing the same memory region from multiple > > > threads. Giving multiple threads access to the same memory region opens > > > up a potential attack vector, in which one thread has write access and > > > another has executable access to the same region. > > Umm, so how does patching work? We don't even know if other threads are > executing the code we need to patch. I thought java can handle that scenario in usual (non W^X systems) just fine, so we just believe jvm did everything right and it's safe to rewrite some code at specific moment. - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v9]
On Thu, 4 Feb 2021 09:49:17 GMT, Vladimir Kempik wrote: > > You read my mind, Andrew. Unless, of course, it's optimized to leverage the > > fact that it's thread-specific.. > > it's thread-specific > > https://developer.apple.com/documentation/apple_silicon/porting_just-in-time_compilers_to_apple_silicon > > > Because pthread_jit_write_protect_np changes only the current thread’s > > permissions, avoid accessing the same memory region from multiple threads. > > Giving multiple threads access to the same memory region opens up a > > potential attack vector, in which one thread has write access and another > > has executable access to the same region. Umm, so how does patching work? We don't even know if other threads are executing the code we need to patch. - PR: https://git.openjdk.java.net/jdk/pull/2200
RFR: 8241372: Several test failures due to javax.net.ssl.SSLException: Connection reset
The server side is binding to the wildcard address which has been a source of instability in many networking tests due to javax.net.ssl.SSLException: Connection reset. Changing the following tests to bind to loopback address fixes intermittent failures: sun/security/ssl/SSLSocketImpl/ReverseNameLookup.java javax/net/ssl/TLSCommon/TLSTest.java sun/security/ssl/CipherSuite/SupportedGroups.java javax/net/ssl/SSLSession/TestEnabledProtocols.java still throws a connection reset occasionally because the server closes the connection before the client is done during handshake. That race condition cannot be completely removed in this test, thus is now handled and logged. - Commit messages: - update log message - add catch clause - Merge remote-tracking branch 'upstream/master' into 8241372 - bind to loopback address instead of wildcard Changes: https://git.openjdk.java.net/jdk/pull/2405/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2405&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8241372 Stats: 29 lines in 4 files changed: 19 ins; 0 del; 10 mod Patch: https://git.openjdk.java.net/jdk/pull/2405.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2405/head:pull/2405 PR: https://git.openjdk.java.net/jdk/pull/2405
Re: RFR: 8241372: Several test failures due to javax.net.ssl.SSLException: Connection reset
On Thu, 4 Feb 2021 13:00:47 GMT, Fernando Guallini wrote: > The server side is binding to the wildcard address which has been a source of > instability in many networking tests due to javax.net.ssl.SSLException: > Connection reset. Changing the following tests to bind to loopback address > fixes intermittent failures: > sun/security/ssl/SSLSocketImpl/ReverseNameLookup.java > javax/net/ssl/TLSCommon/TLSTest.java > sun/security/ssl/CipherSuite/SupportedGroups.java > > > javax/net/ssl/SSLSession/TestEnabledProtocols.java still throws a connection > reset occasionally because the server closes the connection before the client > is done during handshake. That race condition cannot be completely removed in > this test, thus is now handled and logged. @dfuch a similar fix was applied here: https://bugs.openjdk.java.net/browse/JDK-8230858 - PR: https://git.openjdk.java.net/jdk/pull/2405
Re: RFR: 8248268: Support KWP in addition to KW [v2]
> This change updates SunJCE provider as below: > - updated existing AESWrap support with AES/KW/NoPadding cipher > transformation. > - added support for AES/KWP/NoPadding and AES/KW/PKCS5Padding. > > Existing AESWrap impl, i.e. AESWrapCipher class, is re-factored and renamed > to KeyWrapCipher class. The W and W_inverse functions are moved to KWUtil > class. The KW and KWP support are in the new AESKeyWrap and AESKeyWrapPadded > classes which extend FeedbackCipher and used in KeyWrapCipher class. To > minimize data copying, AESKeyWrap and AESKeyWrapPadded will do the crypto > operation over the same input buffer which is allocated and managed by > KeyWrapCipher class. > > Also note that existing AESWrap impl does not take IV. However, the > corresponding PKCS#11 mechanisms do, so I added support for accepting IVs to > both KW and KWP. > > Thanks, > Valerie Valerie Peng has updated the pull request incrementally with one additional commit since the last revision: Restored Iv algorithm parameters impl. - Changes: - all: https://git.openjdk.java.net/jdk/pull/2404/files - new: https://git.openjdk.java.net/jdk/pull/2404/files/fa468921..bc912f63 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2404&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2404&range=00-01 Stats: 330 lines in 4 files changed: 177 ins; 147 del; 6 mod Patch: https://git.openjdk.java.net/jdk/pull/2404.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2404/head:pull/2404 PR: https://git.openjdk.java.net/jdk/pull/2404
Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators [v16]
> This PR is to introduce a new random number API for the JDK. The primary API > is found in RandomGenerator and RandomGeneratorFactory. Further description > can be found in the JEP https://openjdk.java.net/jeps/356 . > > javadoc can be found at > http://cr.openjdk.java.net/~jlaskey/prng/doc/api/java.base/java/util/random/package-summary.html > > old PR: https://github.com/openjdk/jdk/pull/1273 Jim Laskey has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 44 commits: - Merge branch 'master' into 8248862 - Update SplittableRandom to remove unnecessary overrides - Updated API based on CSR review. - Update package info to credit LMX algorithm - Merge branch 'master' into 8248862 - Correct range used by nextBytes - Merge branch 'master' into 8248862 - Use annotation for properties. Add getDefault(). - Merge branch 'master' into 8248862 - Introduce RandomGeneratorProperties annotation - ... and 34 more: https://git.openjdk.java.net/jdk/compare/83357b11...1c17ad35 - Changes: https://git.openjdk.java.net/jdk/pull/1292/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=1292&range=15 Stats: 13259 lines in 26 files changed: 9 ins; 2050 del; 90 mod Patch: https://git.openjdk.java.net/jdk/pull/1292.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1292/head:pull/1292 PR: https://git.openjdk.java.net/jdk/pull/1292
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v9]
On Tue, 2 Feb 2021 22:56:55 GMT, Daniel D. Daugherty wrote: >> Anton Kozlov has updated the pull request incrementally with one additional >> commit since the last revision: >> >> support macos_aarch64 in hsdis > > src/hotspot/share/runtime/stubRoutines.inline.hpp line 1: > >> 1: /* > > NOW I understand the reason for switching from include to inline-include. > Is there a reason that this change is part of this project and not extracted > into a separate RFE. That would reduce the number of files touched by > this project. Makes sense, thanks. I'll do this as JDK-8261075. - PR: https://git.openjdk.java.net/jdk/pull/2200
RFR: 8248268: Support KWP in addition to KW
This change updates SunJCE provider as below: - updated existing AESWrap support with AES/KW/NoPadding cipher transformation. - added support for AES/KWP/NoPadding and AES/KW/PKCS5Padding. Existing AESWrap impl, i.e. AESWrapCipher class, is re-factored and renamed to KeyWrapCipher class. The W and W_inverse functions are moved to KWUtil class. The KW and KWP support are in the new AESKeyWrap and AESKeyWrapPadded classes which extend FeedbackCipher and used in KeyWrapCipher class. To minimize data copying, AESKeyWrap and AESKeyWrapPadded will do the crypto operation over the same input buffer which is allocated and managed by KeyWrapCipher class. Also note that existing AESWrap impl does not take IV. However, the corresponding PKCS#11 mechanisms do, so I added support for accepting IVs to both KW and KWP. Thanks, Valerie - Commit messages: - 8248268: Support KWP in addition to KW Changes: https://git.openjdk.java.net/jdk/pull/2404/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2404&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8248268 Stats: 2534 lines in 14 files changed: 1923 ins; 536 del; 75 mod Patch: https://git.openjdk.java.net/jdk/pull/2404.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2404/head:pull/2404 PR: https://git.openjdk.java.net/jdk/pull/2404
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v9]
On Thu, 4 Feb 2021 08:26:35 GMT, Mikael Vidstedt wrote: > You read my mind, Andrew. Unless, of course, it's optimized to leverage the > fact that it's thread-specific.. it's thread-specific https://developer.apple.com/documentation/apple_silicon/porting_just-in-time_compilers_to_apple_silicon >Because pthread_jit_write_protect_np changes only the current thread’s >permissions, avoid accessing the same memory region from multiple threads. >Giving multiple threads access to the same memory region opens up a potential >attack vector, in which one thread has write access and another has executable >access to the same region. - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v9]
On Wed, 3 Feb 2021 20:08:28 GMT, Mikael Vidstedt wrote:
>>> I wonder if this is the right choice
>>> ...
>>> ```
>>> OopStorageParIterPerf::~OopStorageParIterPerf() {
>>> ...
>>> ```
>>>
>>
>> The transition in OopStorageParIterPerf was made for gtest setup to execute
>> in WXWrite context. For tests themselves, defining macro set WXWrite.
>>
>> I've simplified the scheme and now we switch to WXWrite once at the gtest
>> launcher. So this transition was dropped.
>>
>> I've also refreshed my memory and tried to switch to WXWrite as close as
>> possible to each place where we'll be writing executable memory. There are a
>> lot of such places! As you correctly noted, code cache contains objects, not
>> plain data. For example, CodeCache memory management structures,
>> CompiledMethod, ... are there, so we need more WXWrite switches than we
>> have in the current approach. I had a comparable amount of them just to run
>> -version, but certainly not enough to run tier1 tests.
>>
>> Following your advice, I don't require a known "from" state anymore. So a
>> few W^X transitions were dropped, e.g. when the JVM code calls a JNI entry
>> function, which expects to be called from the native code. I had to switch
>> to WXExec just only to satisfy the expectations. After the update, we don't
>> need this anymore.
>>
>> W^X switches are mostly hidden by VM_ENTRY and similar macros. Some JVM
>> functions are not marked as entries for some reason, although they are
>> called directly from e.g. interpreter. I added W^X management to such
>> functions.
>>
>> Thank you!
>
> Out of curiosity, have you looked at the performance of the W^X state
> transition? In particular I'm wondering if the cost is effectively negligible
> so doing it unconditionally on JVM entry is a no-brainer and just
> easier/cleaner than the alternatives, or if there are reasons to look at only
> doing the transition if/when needed (perhaps do it lazily and revert back to
> X when leaving the JVM?).
You read my mind, Andrew. Unless, of course, it's optimized to leverage the
fact that it's thread-specific..
-
PR: https://git.openjdk.java.net/jdk/pull/2200
