Re: RFR: 8281236: (D)TLS key exchange named groups [v4]
> This update is to support key exchange named groups customization for > individual (D)TLS connection. Please review the CSR as well: > CSR: https://bugs.openjdk.org/browse/JDK-8291950 > RFE: https://bugs.openjdk.org/browse/JDK-8281236 > Release-note: https://bugs.openjdk.org/browse/JDK-8291975 > > This is an effort similar to [JDK-8280494: "(D)TLS signature > schemes"](https://bugs.openjdk.org/browse/JDK-8280494) Xue-Lei Andrew Fan has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains six commits: - check duplicate - Merge - Merge - Merge - add test cases - 8281236: (D)TLS key exchange algorithms - Changes: https://git.openjdk.org/jdk/pull/9776/files Webrev: https://webrevs.openjdk.org/?repo=jdk=9776=03 Stats: 982 lines in 17 files changed: 746 ins; 195 del; 41 mod Patch: https://git.openjdk.org/jdk/pull/9776.diff Fetch: git fetch https://git.openjdk.org/jdk pull/9776/head:pull/9776 PR: https://git.openjdk.org/jdk/pull/9776
Re: RFR: 8295803: Console should be usable in jshell and other environments [v7]
On Tue, 6 Dec 2022 07:08:55 GMT, Jaikiran Pai wrote: > Furthermore, I think in its current form it means that this will > load/instantiate any `JdkConsoleProvider` implementations that are accessible > to the thread context classloader but may not have been from the module > configured through `jdk.console` system property. That could potentially > mean, in the best case, unnecessary classloading of additional classes and in > the worst case, could result in `ServiceLoader.Provider::get` throwing a > `ServiceConfigurationError` error for any of such unused provider > implementations, thus forcing us to use `java.io.Console` instance. You are right that the ServiceLoader.load should specify the system class loader or the boot layer. However, there isn't an accessibility issue as a class loader just load classes so it's more about visibility and whether the TCCL will ultimately delegate to the application class loader. - PR: https://git.openjdk.org/jdk/pull/11421
Re: RFR: 8295803: Console should be usable in jshell and other environments [v7]
On Mon, 5 Dec 2022 19:52:59 GMT, Naoto Sato wrote:
>> This is to allow Console to be used even when it is not attached to the
>> platform provided terminal, such as the case when the standard input is
>> redirected. `System.console()` now returns a Console implementation based on
>> `jdk.internal.le` terminal by default, or jshell implementation if
>> available. A corresponding CSR has been drafted.
>
> Naoto Sato has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Fixed the copyright year
Hello Naoto, the class level javadoc of `java.io.Console` currently specifies
synchronization expectations in the context of multithreaded use:
> Read and write operations are synchronized to guarantee the atomic
> completion of critical operations; therefore invoking methods
> {@link #readLine()}, {@link #readPassword()}, {@link #format format()},
> {@link #printf printf()} as well as the read, format and write operations
> on the objects returned by {@link #reader()} and {@link #writer()} may
> block in multithreaded scenarios.
So the `Console` instance returned from `System.console()`, thus far, follows
these semantics. However, with the change proposed in this PR, the default
implementation will now be the jline backed `JdkConsoleImpl` implementation.
From what I can see there, we don't seem to have any similar guarantees around
multithreaded access. Do we need similar locking constructs in that
implementation to guarantee/verify it works as per the expectations of
`java.io.Console` API?
While we are at it, the Console class level javadoc also states:
> If the virtual machine is started from an
> interactive command line without redirecting the standard input and
> output streams then its console will exist and will typically be
> connected to the keyboard and display from which the virtual machine
> was launched.
With this proposed change, to by default use the jline backed implementation,
would we need to reword/update that javadoc?
-
PR: https://git.openjdk.org/jdk/pull/11421
Re: RFR: 8295803: Console should be usable in jshell and other environments [v7]
On Mon, 5 Dec 2022 19:52:59 GMT, Naoto Sato wrote:
>> This is to allow Console to be used even when it is not attached to the
>> platform provided terminal, such as the case when the standard input is
>> redirected. `System.console()` now returns a Console implementation based on
>> `jdk.internal.le` terminal by default, or jshell implementation if
>> available. A corresponding CSR has been drafted.
>
> Naoto Sato has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Fixed the copyright year
src/java.base/share/classes/java/io/Console.java line 625:
> 623: };
> 624: return AccessController.doPrivileged(pa);
> 625: } catch (ServiceConfigurationError ignore) {
Should we perhaps just catch `Throwable` here since it's possible that the
`PrivelegedAction` code could throw unchecked exception (for example the call
to `JdkConsoleProvider.console()` could, in theory, lead to any kind of
unchecked exceptions or errors like `NoClassDefFoundError`).
-
PR: https://git.openjdk.org/jdk/pull/11421
Re: RFR: 8295803: Console should be usable in jshell and other environments [v7]
On Mon, 5 Dec 2022 19:52:59 GMT, Naoto Sato wrote: >> This is to allow Console to be used even when it is not attached to the >> platform provided terminal, such as the case when the standard input is >> redirected. `System.console()` now returns a Console implementation based on >> `jdk.internal.le` terminal by default, or jshell implementation if >> available. A corresponding CSR has been drafted. > > Naoto Sato has updated the pull request incrementally with one additional > commit since the last revision: > > Fixed the copyright year src/java.base/share/classes/java/io/Console.java line 616: > 614: JdkConsoleProvider.DEFAULT_PROVIDER_MODULE_NAME); > 615: return > ServiceLoader.load(JdkConsoleProvider.class).stream() > 616: .map(ServiceLoader.Provider::get) Furthermore, I think in its current form it means that this will load/instantiate any `JdkConsoleProvider` implementations that are accessible to the thread context classloader but may not have been from the module configured through `jdk.console` system property. That could potentially mean, in the best case, unnecessary classloading of additional classes and in the worst case, could result in `ServiceLoader.Provider::get` throwing a `ServiceConfigurationError` error for any of such unused provider implementations, thus forcing us to use `java.io.Console` instance. - PR: https://git.openjdk.org/jdk/pull/11421
Re: RFR: 8295803: Console should be usable in jshell and other environments [v7]
On Mon, 5 Dec 2022 19:52:59 GMT, Naoto Sato wrote:
>> This is to allow Console to be used even when it is not attached to the
>> platform provided terminal, such as the case when the standard input is
>> redirected. `System.console()` now returns a Console implementation based on
>> `jdk.internal.le` terminal by default, or jshell implementation if
>> available. A corresponding CSR has been drafted.
>
> Naoto Sato has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Fixed the copyright year
src/java.base/share/classes/java/io/Console.java line 615:
> 613: var consModName = System.getProperty("jdk.console",
> 614: JdkConsoleProvider.DEFAULT_PROVIDER_MODULE_NAME);
> 615: return
> ServiceLoader.load(JdkConsoleProvider.class).stream()
Are we intentionally using thread context classloader (which can be different
depending on which caller ends up first accessing/initializing the
`java.io.Console` class) to load these services?
I initially thought that `java.io.Console` might be used/initialized early in
the bootstrap process of Java so the classloader could perhaps be
deterministic, but running a trivial Java application with `-verbose:class`
shows that `java.io.Console` doesn't get instantiated during the launch, so
that leaves this code to "first access wins" situation and maybe an "incorrect"
context classloader which doesn't have access the configured `jdk.console`
module may end up causing this code to default to `java.io.Console`?
public class Hello {
public static void main(final String[] args) {
}
}
java -verbose:class Hello.java
Instead, should we perhaps use the ModuleLayer to find this configured module
and then use its classloader to load the `JdkConsoleProvider` service provider?
Something like:
final Optional mod = ModuleLayer.boot().findModule(consModName);
// ... if not present default to java.io.Console else use the module's
classloader to try and load the JdkConsoleProvider
return ServiceLoader.load(JdkConsoleProvider.class,
mod.get().getClassLoader()).stream()..
-
PR: https://git.openjdk.org/jdk/pull/11421
Re: RFR: 8295803: Console should be usable in jshell and other environments [v7]
On Mon, 5 Dec 2022 19:52:59 GMT, Naoto Sato wrote: >> This is to allow Console to be used even when it is not attached to the >> platform provided terminal, such as the case when the standard input is >> redirected. `System.console()` now returns a Console implementation based on >> `jdk.internal.le` terminal by default, or jshell implementation if >> available. A corresponding CSR has been drafted. > > Naoto Sato has updated the pull request incrementally with one additional > commit since the last revision: > > Fixed the copyright year src/java.base/share/classes/java/io/Console.java line 99: > 97: */ > 98: > 99: public class Console implements Flushable Should we perhaps `seal` this class and only `permit` `ProxyingConsole` to `extend` it? - PR: https://git.openjdk.org/jdk/pull/11421
Re: RFR: 8295803: Console should be usable in jshell and other environments [v7]
On Mon, 5 Dec 2022 19:52:59 GMT, Naoto Sato wrote:
>> This is to allow Console to be used even when it is not attached to the
>> platform provided terminal, such as the case when the standard input is
>> redirected. `System.console()` now returns a Console implementation based on
>> `jdk.internal.le` terminal by default, or jshell implementation if
>> available. A corresponding CSR has been drafted.
>
> Naoto Sato has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Fixed the copyright year
src/jdk.internal.le/share/classes/jdk/internal/org/jline/JdkConsoleProviderImpl.java
line 113:
> 111: public JdkConsoleImpl() {
> 112: try {
> 113: terminal = TerminalBuilder.builder().build();
The `java.io.Console` in its static initialization code has some logic to
determine the `Charset` to use. Should that same `Charset` (or logic) be used
here to build the terminal? Something like
`TerminalBuilder.builder().encoding(fooBarCharset).build();`.
-
PR: https://git.openjdk.org/jdk/pull/11421
Re: RFR: 8295803: Console should be usable in jshell and other environments [v7]
On Mon, 5 Dec 2022 19:52:59 GMT, Naoto Sato wrote:
>> This is to allow Console to be used even when it is not attached to the
>> platform provided terminal, such as the case when the standard input is
>> redirected. `System.console()` now returns a Console implementation based on
>> `jdk.internal.le` terminal by default, or jshell implementation if
>> available. A corresponding CSR has been drafted.
>
> Naoto Sato has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Fixed the copyright year
src/jdk.internal.le/share/classes/jdk/internal/org/jline/JdkConsoleProviderImpl.java
line 50:
> 48: * {@inheritDoc}
> 49: */
> 50: public JdkConsole console(boolean isTTY) {
Hello Naoto, this is missing a `@Override`.
src/jdk.internal.le/share/classes/jdk/internal/org/jline/JdkConsoleProviderImpl.java
line 58:
> 56: * public Console class.
> 57: */
> 58: public static class JdkConsoleImpl implements JdkConsole {
Can this be `private` class? Also, the methods in this class are missing the
`@Override`.
-
PR: https://git.openjdk.org/jdk/pull/11421
Re: RFR: 8297379: Enable the ByteBuffer path of Poly1305 optimizations [v4]
On Mon, 5 Dec 2022 22:05:51 GMT, Volodymyr Paprotski wrote: >> There is now an intrinsic for Poly1305, which is only enabled on the >> `engineUpdate([]byte)` path. This PR adds intrinsic support >> `engineUpdate(ByteBuffer)` (when the bytebuffer `hasArray`). >> >> Fuzzing test expanded to also include ByteBuffer payloads. >> >> Performance is now matched: >> >> Benchmark (dataSize) (provider) Mode Cnt >> ScoreError Units >> Poly1305DigestBench.digestBuffer 64 thrpt3 >> 3320909.857 ± 787300.545 ops/s >> Poly1305DigestBench.digestBuffer 256 thrpt3 >> 3006447.324 ± 704790.796 ops/s >> Poly1305DigestBench.digestBuffer1024 thrpt3 >> 2645041.509 ± 664904.622 ops/s >> Poly1305DigestBench.digestBuffer 16384 thrpt3 >> 893389.209 ± 6288.743 ops/s >> Poly1305DigestBench.digestBuffer 1048576 thrpt3 >> 14932.680 ±170.238 ops/s >> Poly1305DigestBench.digestBytes 64 thrpt3 >> 3548298.515 ± 859964.530 ops/s >> Poly1305DigestBench.digestBytes 256 thrpt3 >> 3083261.994 ± 141802.417 ops/s >> Poly1305DigestBench.digestBytes 1024 thrpt3 >> 2704357.584 ± 554683.019 ops/s >> Poly1305DigestBench.digestBytes16384 thrpt3 >> 898913.339 ± 99733.295 ops/s >> Poly1305DigestBench.digestBytes 1048576 thrpt3 >> 14961.872 ± 38.003 ops/s >> Finished running test >> 'micro:org.openjdk.bench.javax.crypto.full.Poly1305DigestBench' >> >> Relates to: >> - https://github.com/openjdk/jdk/pull/11463: Found inconsistency between >> processing `[]byte` and `ByteBuffer`. When that one is fixed, >> `Poly1305IntrinsicFuzzTest.java` should not be setting the endianness on the >> `ByteBuffer` >> - Intrinsic introduced by https://github.com/openjdk/jdk/pull/10582. > > Volodymyr Paprotski 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 six additional > commits since the last revision: > > - Merge remote-tracking branch 'origin/master' into avx512-poly-buf > - remove comment > - bench and handle buf.position() > - Merge remote-tracking branch 'origin/master' into avx512-poly-buf > - whitespace > - ByteBuffer support and tests Changes look fine. tier1-5 + hs-precheckin-comp test jobs came back clean with the exception of two jaxp failures only on linux-x64 but they don't appear to be related to your changes. - Marked as reviewed by jnimeh (Reviewer). PR: https://git.openjdk.org/jdk/pull/11338
Re: RFR: 8296507: GCM using more memory than necessary with in-place operations [v4]
> I would like a review of an update to the GCM code. A recent report showed > that GCM memory usage for TLS was very large. This was a result of in-place > buffers, which TLS uses, and how the code handled the combined intrinsic > method during decryption. A temporary buffer was used because the combined > intrinsic does gctr before ghash which results in a bad tag. The fix is to > not use the combined intrinsic during in-place decryption and depend on the > individual GHASH and CounterMode intrinsics. Direct ByteBuffers are not > affected as they are not used by the intrinsics directly. > > The reduction in the memory usage boosted performance back to where it was > before despite using slower intrinsics (gctr & ghash individually). The > extra memory allocation for the temporary buffer out-weighted the faster > intrinsic. > > > JDK 17: 122913.554 ops/sec > JDK 19:94885.008 ops/sec > Post fix: 122735.804 ops/sec > > There is no regression test because this is a memory change and test coverage > already existing. Anthony Scarpino has updated the pull request incrementally with one additional commit since the last revision: another comment update - Changes: - all: https://git.openjdk.org/jdk/pull/11121/files - new: https://git.openjdk.org/jdk/pull/11121/files/340ab22f..99e350b2 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=11121=03 - incr: https://webrevs.openjdk.org/?repo=jdk=11121=02-03 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/11121.diff Fetch: git fetch https://git.openjdk.org/jdk pull/11121/head:pull/11121 PR: https://git.openjdk.org/jdk/pull/11121
Re: RFR: 8297379: Enable the ByteBuffer path of Poly1305 optimizations [v4]
On Mon, 5 Dec 2022 22:05:51 GMT, Volodymyr Paprotski wrote: >> There is now an intrinsic for Poly1305, which is only enabled on the >> `engineUpdate([]byte)` path. This PR adds intrinsic support >> `engineUpdate(ByteBuffer)` (when the bytebuffer `hasArray`). >> >> Fuzzing test expanded to also include ByteBuffer payloads. >> >> Performance is now matched: >> >> Benchmark (dataSize) (provider) Mode Cnt >> ScoreError Units >> Poly1305DigestBench.digestBuffer 64 thrpt3 >> 3320909.857 ± 787300.545 ops/s >> Poly1305DigestBench.digestBuffer 256 thrpt3 >> 3006447.324 ± 704790.796 ops/s >> Poly1305DigestBench.digestBuffer1024 thrpt3 >> 2645041.509 ± 664904.622 ops/s >> Poly1305DigestBench.digestBuffer 16384 thrpt3 >> 893389.209 ± 6288.743 ops/s >> Poly1305DigestBench.digestBuffer 1048576 thrpt3 >> 14932.680 ±170.238 ops/s >> Poly1305DigestBench.digestBytes 64 thrpt3 >> 3548298.515 ± 859964.530 ops/s >> Poly1305DigestBench.digestBytes 256 thrpt3 >> 3083261.994 ± 141802.417 ops/s >> Poly1305DigestBench.digestBytes 1024 thrpt3 >> 2704357.584 ± 554683.019 ops/s >> Poly1305DigestBench.digestBytes16384 thrpt3 >> 898913.339 ± 99733.295 ops/s >> Poly1305DigestBench.digestBytes 1048576 thrpt3 >> 14961.872 ± 38.003 ops/s >> Finished running test >> 'micro:org.openjdk.bench.javax.crypto.full.Poly1305DigestBench' >> >> Relates to: >> - https://github.com/openjdk/jdk/pull/11463: Found inconsistency between >> processing `[]byte` and `ByteBuffer`. When that one is fixed, >> `Poly1305IntrinsicFuzzTest.java` should not be setting the endianness on the >> `ByteBuffer` >> - Intrinsic introduced by https://github.com/openjdk/jdk/pull/10582. > > Volodymyr Paprotski 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 six additional > commits since the last revision: > > - Merge remote-tracking branch 'origin/master' into avx512-poly-buf > - remove comment > - bench and handle buf.position() > - Merge remote-tracking branch 'origin/master' into avx512-poly-buf > - whitespace > - ByteBuffer support and tests Marked as reviewed by ascarpino (Reviewer). - PR: https://git.openjdk.org/jdk/pull/11338
Re: RFR: 8297379: Enable the ByteBuffer path of Poly1305 optimizations [v3]
On Thu, 1 Dec 2022 18:23:45 GMT, Volodymyr Paprotski wrote: >> There is now an intrinsic for Poly1305, which is only enabled on the >> `engineUpdate([]byte)` path. This PR adds intrinsic support >> `engineUpdate(ByteBuffer)` (when the bytebuffer `hasArray`). >> >> Fuzzing test expanded to also include ByteBuffer payloads. >> >> Performance is now matched: >> >> Benchmark (dataSize) (provider) Mode Cnt >> ScoreError Units >> Poly1305DigestBench.digestBuffer 64 thrpt3 >> 3320909.857 ± 787300.545 ops/s >> Poly1305DigestBench.digestBuffer 256 thrpt3 >> 3006447.324 ± 704790.796 ops/s >> Poly1305DigestBench.digestBuffer1024 thrpt3 >> 2645041.509 ± 664904.622 ops/s >> Poly1305DigestBench.digestBuffer 16384 thrpt3 >> 893389.209 ± 6288.743 ops/s >> Poly1305DigestBench.digestBuffer 1048576 thrpt3 >> 14932.680 ±170.238 ops/s >> Poly1305DigestBench.digestBytes 64 thrpt3 >> 3548298.515 ± 859964.530 ops/s >> Poly1305DigestBench.digestBytes 256 thrpt3 >> 3083261.994 ± 141802.417 ops/s >> Poly1305DigestBench.digestBytes 1024 thrpt3 >> 2704357.584 ± 554683.019 ops/s >> Poly1305DigestBench.digestBytes16384 thrpt3 >> 898913.339 ± 99733.295 ops/s >> Poly1305DigestBench.digestBytes 1048576 thrpt3 >> 14961.872 ± 38.003 ops/s >> Finished running test >> 'micro:org.openjdk.bench.javax.crypto.full.Poly1305DigestBench' >> >> Relates to: >> - https://github.com/openjdk/jdk/pull/11463: Found inconsistency between >> processing `[]byte` and `ByteBuffer`. When that one is fixed, >> `Poly1305IntrinsicFuzzTest.java` should not be setting the endianness on the >> `ByteBuffer` >> - Intrinsic introduced by https://github.com/openjdk/jdk/pull/10582. > > Volodymyr Paprotski has updated the pull request incrementally with one > additional commit since the last revision: > > remove comment (The build failures do not look related to this PR, trying to see if fixed in main branch) - PR: https://git.openjdk.org/jdk/pull/11338
Re: RFR: 8297379: Enable the ByteBuffer path of Poly1305 optimizations [v4]
> There is now an intrinsic for Poly1305, which is only enabled on the > `engineUpdate([]byte)` path. This PR adds intrinsic support > `engineUpdate(ByteBuffer)` (when the bytebuffer `hasArray`). > > Fuzzing test expanded to also include ByteBuffer payloads. > > Performance is now matched: > > Benchmark (dataSize) (provider) Mode Cnt > ScoreError Units > Poly1305DigestBench.digestBuffer 64 thrpt3 > 3320909.857 ± 787300.545 ops/s > Poly1305DigestBench.digestBuffer 256 thrpt3 > 3006447.324 ± 704790.796 ops/s > Poly1305DigestBench.digestBuffer1024 thrpt3 > 2645041.509 ± 664904.622 ops/s > Poly1305DigestBench.digestBuffer 16384 thrpt3 > 893389.209 ± 6288.743 ops/s > Poly1305DigestBench.digestBuffer 1048576 thrpt3 > 14932.680 ±170.238 ops/s > Poly1305DigestBench.digestBytes 64 thrpt3 > 3548298.515 ± 859964.530 ops/s > Poly1305DigestBench.digestBytes 256 thrpt3 > 3083261.994 ± 141802.417 ops/s > Poly1305DigestBench.digestBytes 1024 thrpt3 > 2704357.584 ± 554683.019 ops/s > Poly1305DigestBench.digestBytes16384 thrpt3 > 898913.339 ± 99733.295 ops/s > Poly1305DigestBench.digestBytes 1048576 thrpt3 > 14961.872 ± 38.003 ops/s > Finished running test > 'micro:org.openjdk.bench.javax.crypto.full.Poly1305DigestBench' > > Relates to: > - https://github.com/openjdk/jdk/pull/11463: Found inconsistency between > processing `[]byte` and `ByteBuffer`. When that one is fixed, > `Poly1305IntrinsicFuzzTest.java` should not be setting the endianness on the > `ByteBuffer` > - Intrinsic introduced by https://github.com/openjdk/jdk/pull/10582. Volodymyr Paprotski 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 six additional commits since the last revision: - Merge remote-tracking branch 'origin/master' into avx512-poly-buf - remove comment - bench and handle buf.position() - Merge remote-tracking branch 'origin/master' into avx512-poly-buf - whitespace - ByteBuffer support and tests - Changes: - all: https://git.openjdk.org/jdk/pull/11338/files - new: https://git.openjdk.org/jdk/pull/11338/files/14726d7b..a87297f3 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=11338=03 - incr: https://webrevs.openjdk.org/?repo=jdk=11338=02-03 Stats: 45355 lines in 999 files changed: 17638 ins; 21210 del; 6507 mod Patch: https://git.openjdk.org/jdk/pull/11338.diff Fetch: git fetch https://git.openjdk.org/jdk pull/11338/head:pull/11338 PR: https://git.openjdk.org/jdk/pull/11338
Re: RFR: 8297379: Enable the ByteBuffer path of Poly1305 optimizations [v3]
On Thu, 1 Dec 2022 18:23:45 GMT, Volodymyr Paprotski wrote: >> There is now an intrinsic for Poly1305, which is only enabled on the >> `engineUpdate([]byte)` path. This PR adds intrinsic support >> `engineUpdate(ByteBuffer)` (when the bytebuffer `hasArray`). >> >> Fuzzing test expanded to also include ByteBuffer payloads. >> >> Performance is now matched: >> >> Benchmark (dataSize) (provider) Mode Cnt >> ScoreError Units >> Poly1305DigestBench.digestBuffer 64 thrpt3 >> 3320909.857 ± 787300.545 ops/s >> Poly1305DigestBench.digestBuffer 256 thrpt3 >> 3006447.324 ± 704790.796 ops/s >> Poly1305DigestBench.digestBuffer1024 thrpt3 >> 2645041.509 ± 664904.622 ops/s >> Poly1305DigestBench.digestBuffer 16384 thrpt3 >> 893389.209 ± 6288.743 ops/s >> Poly1305DigestBench.digestBuffer 1048576 thrpt3 >> 14932.680 ±170.238 ops/s >> Poly1305DigestBench.digestBytes 64 thrpt3 >> 3548298.515 ± 859964.530 ops/s >> Poly1305DigestBench.digestBytes 256 thrpt3 >> 3083261.994 ± 141802.417 ops/s >> Poly1305DigestBench.digestBytes 1024 thrpt3 >> 2704357.584 ± 554683.019 ops/s >> Poly1305DigestBench.digestBytes16384 thrpt3 >> 898913.339 ± 99733.295 ops/s >> Poly1305DigestBench.digestBytes 1048576 thrpt3 >> 14961.872 ± 38.003 ops/s >> Finished running test >> 'micro:org.openjdk.bench.javax.crypto.full.Poly1305DigestBench' >> >> Relates to: >> - https://github.com/openjdk/jdk/pull/11463: Found inconsistency between >> processing `[]byte` and `ByteBuffer`. When that one is fixed, >> `Poly1305IntrinsicFuzzTest.java` should not be setting the endianness on the >> `ByteBuffer` >> - Intrinsic introduced by https://github.com/openjdk/jdk/pull/10582. > > Volodymyr Paprotski has updated the pull request incrementally with one > additional commit since the last revision: > > remove comment The changes look ok to me - PR: https://git.openjdk.org/jdk/pull/11338
Re: RFR: 8298108: Add a regression test for JDK-8297684 [v3]
On Mon, 5 Dec 2022 17:56:49 GMT, Severin Gehwolf wrote: >> Please review this test addition as it would have helped discover a >> regression when https://bugs.openjdk.org/browse/JDK-8269039 got introduced >> and subsequently backported. What's more, it might help discover similar >> issues going forward. Thoughts? >> >> Testing: Manually produced a build without >> [JDK-8269039](https://bugs.openjdk.org/browse/JDK-8269039). Test fails >> without the fix, passes after. > > Severin Gehwolf has updated the pull request incrementally with one > additional commit since the last revision: > > Remove unused Path arg for test invocation A regression test was added as part of the fix for https://bugs.openjdk.org/browse/JDK-8280890. See the commit log for more details: https://github.com/openjdk/jdk/commit/a0f6f2409ea61ff9ed9dc2e2b46e309c751d456d#diff-6de8303e80bab41e34a29b272cff64a68d3b255f0f147ee16ef21680abf7f41a Since this is a different way of testing the same underlying issue and the problem shows up in more than one way, I agree it is a good idea to add another regression test. However, I would put in the same test directory as 8280890: test/jdk/java/security/SignedJar - PR: https://git.openjdk.org/jdk/pull/11515
Re: RFR: 8295803: Console should be usable in jshell and other environments [v7]
On Mon, 5 Dec 2022 19:52:59 GMT, Naoto Sato wrote: >> This is to allow Console to be used even when it is not attached to the >> platform provided terminal, such as the case when the standard input is >> redirected. `System.console()` now returns a Console implementation based on >> `jdk.internal.le` terminal by default, or jshell implementation if >> available. A corresponding CSR has been drafted. > > Naoto Sato has updated the pull request incrementally with one additional > commit since the last revision: > > Fixed the copyright year Marked as reviewed by jlaskey (Reviewer). Marked as reviewed by jlaskey (Reviewer). LGTM Marked as reviewed by jlaskey (Reviewer). - PR: https://git.openjdk.org/jdk/pull/11421Marked as reviewed by jlaskey (Reviewer).
Re: RFR: 8295803: Console should be usable in jshell and other environments [v4]
On Sun, 4 Dec 2022 17:09:15 GMT, Alan Bateman wrote: >> Naoto Sato has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Changed the expected behavior when the SecurityManager is enabled > > src/java.base/share/classes/java/io/Console.java line 627: > >> 625: }); >> 626: } >> 627: private static Console cons; > > The initialization of cons is replaced in this PR so maybe we can make it > final at the same time. Changed it to final - PR: https://git.openjdk.org/jdk/pull/11421
Re: RFR: 8295803: Console should be usable in jshell and other environments [v7]
> This is to allow Console to be used even when it is not attached to the > platform provided terminal, such as the case when the standard input is > redirected. `System.console()` now returns a Console implementation based on > `jdk.internal.le` terminal by default, or jshell implementation if available. > A corresponding CSR has been drafted. Naoto Sato has updated the pull request incrementally with one additional commit since the last revision: Fixed the copyright year - Changes: - all: https://git.openjdk.org/jdk/pull/11421/files - new: https://git.openjdk.org/jdk/pull/11421/files/31656331..8b6859ed Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=11421=06 - incr: https://webrevs.openjdk.org/?repo=jdk=11421=05-06 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/11421.diff Fetch: git fetch https://git.openjdk.org/jdk pull/11421/head:pull/11421 PR: https://git.openjdk.org/jdk/pull/11421
Re: RFR: JDK-8295087: Manual Test to Automated Test Conversion [v5]
> This task converts 5 manual tests to automated tests. > > sun/security/provider/PolicyParser/ExtDirsDefaultPolicy.java > sun/security/provider/PolicyParser/ExtDirsChange.java > sun/security/provider/PolicyParser/ExtDirs.java > java/security/Policy/Root/Root.javajava/security/Policy/Root/Root.java > javax/crypto/CryptoPermissions/InconsistentEntries.java Bill Huang 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 five additional commits since the last revision: - Merge branch 'master' into JDK-8295087 - Added an extra line to the end of the policy file. - AssertThrows an exception in InconsistentEntries test. - Refactored to use testng framework for test enviroment setup. - Converted security manual tests to automated tests. - Changes: - all: https://git.openjdk.org/jdk/pull/10637/files - new: https://git.openjdk.org/jdk/pull/10637/files/8b45f39f..32b656ca Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=10637=04 - incr: https://webrevs.openjdk.org/?repo=jdk=10637=03-04 Stats: 297635 lines in 3764 files changed: 148190 ins; 96710 del; 52735 mod Patch: https://git.openjdk.org/jdk/pull/10637.diff Fetch: git fetch https://git.openjdk.org/jdk pull/10637/head:pull/10637 PR: https://git.openjdk.org/jdk/pull/10637
Re: RFR: 8298045: Fix hidden but significant trailing whitespace in properties files for core-libs code
On Fri, 2 Dec 2022 16:40:51 GMT, Magnus Ihse Bursie wrote:
> According to [the
> specification](https://docs.oracle.com/en/java/javase/19/docs/api/java.base/java/util/Properties.html#load(java.io.Reader))
> trailing whitespaces in the values of properties files are (somewhat
> surprisingly) actually significant.
>
> We have multiple files in the JDK with trailing whitespaces in the values.
> For most of this files, this is likely incorrect and due to oversight, but in
> a few cases it might actually be intended (like "The value is: ").
>
> After a discussion in the PR for
> [JDK-8295729](https://bugs.openjdk.org/browse/JDK-8295729), the consensus was
> to replace valid trailing spaces with the corresponding unicode sequence,
> `\u0020`. (And of course remove non-wanted trailing spaces.)
>
> Doing so has a dual benefit:
>
> 1) It makes it clear to everyone reading the code that there is a trailing
> space and it is intended
>
> 2) It will allow us to remove all actual trailing space characters, and turn
> on the corresponding check in jcheck to keep the properties files, just like
> all other source code files, free of trailing spaces.
>
> Ultimately, the call of whether a trailing space is supposed to be there, or
> is a bug, lies with the respective component teams owning these files. Thus I
> have split up the set of properties files with trailing spaces in several
> groups, to match the JDK teams, and open a JBS issue for each of them. This
> issue is for code I believe belong with the core-libs team.
src/java.sql.rowset/share/classes/com/sun/rowset/RowSetResourceBundle_zh_TW.properties
line 160:
> 158: xmlrch.errupdate =
> \u5EFA\u69CB\u66F4\u65B0\u5217\u6642\u767C\u751F\u932F\u8AA4: {0}
> 159: xmlrch.errupdrow = \u66F4\u65B0\u5217\u6642\u767C\u751F\u932F\u8AA4: {0}
> 160: xmlrch.chars = \u5B57\u5143:\u0020
Likely not needed, since the original and all other l10n versions of
RowSetResourceBundle.properties do not have a trailing space for `xmlrch.chars`
src/java.xml/share/classes/com/sun/org/apache/xerces/internal/impl/msg/XMLMessages.properties
line 24:
> 22:
> 23: BadMessageKey = The error message corresponding to the message
> key can not be found.
> 24: FormatFailed = An internal error occurred while formatting the
> following message:\n\u0020\u0020
Likely a mistake, since as you stated, it is not in the format “foo:\u0020” as
there is a newline before the trailing spaces.
However if intentional it should probably be `FormatFailed = An internal error
occurred while formatting the following message:\n\u0020`
src/java.xml/share/classes/com/sun/org/apache/xerces/internal/impl/msg/XMLMessages.properties
line 139:
> 137: EncodingByteOrderUnsupported = Given byte order for encoding
> \"{0}\" is not supported.
> 138: InvalidByte = Invalid byte {0} of {1}-byte UTF-8 sequence.
> 139: ExpectedByte = Expected byte {0} of {1}-byte UTF-8
> sequence.\u0020\u0020
Same here as well, either a mistake or should be ` ExpectedByte = Expected byte
{0} of {1}-byte UTF-8 sequence.\u0020`
src/java.xml/share/classes/com/sun/org/apache/xerces/internal/impl/msg/XMLMessages.properties
line 219:
> 217: MSG_NOTATION_NAME_REQUIRED_FOR_UNPARSED_ENTITYDECL = The
> notation name is required after \"NDATA\" in the declaration for the entity
> \"{0}\".
> 218: EntityDeclUnterminated = The declaration for the entity \"{0}\"
> must end with ''>''.
> 219: MSG_DUPLICATE_ENTITY_DEFINITION = Entity \"{0}\" is declared more than
> once.\u0020\u0020\u0020\u0020\u0020\u0020\u0020\u0020
Likely not intentional, but if it is then perhaps
`MSG_DUPLICATE_ENTITY_DEFINITION = Entity "{0}" is declared more than
once.\u0009` instead.
-
PR: https://git.openjdk.org/jdk/pull/11489
Re: RFR: 8295803: Console should be usable in jshell and other environments [v6]
On Mon, 5 Dec 2022 18:33:10 GMT, Naoto Sato wrote: >> This is to allow Console to be used even when it is not attached to the >> platform provided terminal, such as the case when the standard input is >> redirected. `System.console()` now returns a Console implementation based on >> `jdk.internal.le` terminal by default, or jshell implementation if >> available. A corresponding CSR has been drafted. > > Naoto Sato has updated the pull request incrementally with one additional > commit since the last revision: > > oleole -> ole LGTM src/java.base/share/classes/jdk/internal/access/JavaIOAccess.java line 26: > 24: */ > 25: > 26: package jdk.internal.access; Update copyright - Marked as reviewed by jlaskey (Reviewer). PR: https://git.openjdk.org/jdk/pull/11421
Re: RFR: JDK-8293412 Remove unnecessary java.security.egd overrides [v4]
> https://bugs.openjdk.org/browse/JDK-8293412 Mark Powers 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 eight additional commits since the last revision: - Merge - javadoc update - yes we need /dev/urandom - Merge - Merge - second iteration - Merge - first iteration - Changes: - all: https://git.openjdk.org/jdk/pull/10716/files - new: https://git.openjdk.org/jdk/pull/10716/files/80be7e6b..8076b616 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=10716=03 - incr: https://webrevs.openjdk.org/?repo=jdk=10716=02-03 Stats: 89795 lines in 1773 files changed: 38018 ins; 35347 del; 16430 mod Patch: https://git.openjdk.org/jdk/pull/10716.diff Fetch: git fetch https://git.openjdk.org/jdk pull/10716/head:pull/10716 PR: https://git.openjdk.org/jdk/pull/10716
Re: RFR: 8297379: Enable the ByteBuffer path of Poly1305 optimizations [v3]
On Mon, 5 Dec 2022 18:22:00 GMT, Sandhya Viswanathan wrote: >> Volodymyr Paprotski has updated the pull request incrementally with one >> additional commit since the last revision: >> >> remove comment > > @valeriepeng Could you please take a look at this PR? @sviswa7 I will be looking at this today - PR: https://git.openjdk.org/jdk/pull/11338
Re: RFR: 8295803: Console should be usable in jshell and other environments [v6]
> This is to allow Console to be used even when it is not attached to the > platform provided terminal, such as the case when the standard input is > redirected. `System.console()` now returns a Console implementation based on > `jdk.internal.le` terminal by default, or jshell implementation if available. > A corresponding CSR has been drafted. Naoto Sato has updated the pull request incrementally with one additional commit since the last revision: oleole -> ole - Changes: - all: https://git.openjdk.org/jdk/pull/11421/files - new: https://git.openjdk.org/jdk/pull/11421/files/00a0e3fc..31656331 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=11421=05 - incr: https://webrevs.openjdk.org/?repo=jdk=11421=04-05 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/11421.diff Fetch: git fetch https://git.openjdk.org/jdk pull/11421/head:pull/11421 PR: https://git.openjdk.org/jdk/pull/11421
Re: RFR: 8295803: Console should be usable in jshell and other environments [v5]
> This is to allow Console to be used even when it is not attached to the > platform provided terminal, such as the case when the standard input is > redirected. `System.console()` now returns a Console implementation based on > `jdk.internal.le` terminal by default, or jshell implementation if available. > A corresponding CSR has been drafted. Naoto Sato has updated the pull request incrementally with one additional commit since the last revision: Addressing review comments. - Changes: - all: https://git.openjdk.org/jdk/pull/11421/files - new: https://git.openjdk.org/jdk/pull/11421/files/7c0740aa..00a0e3fc Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=11421=04 - incr: https://webrevs.openjdk.org/?repo=jdk=11421=03-04 Stats: 57 lines in 5 files changed: 26 ins; 22 del; 9 mod Patch: https://git.openjdk.org/jdk/pull/11421.diff Fetch: git fetch https://git.openjdk.org/jdk pull/11421/head:pull/11421 PR: https://git.openjdk.org/jdk/pull/11421
Re: RFR: 8297379: Enable the ByteBuffer path of Poly1305 optimizations [v3]
On Thu, 1 Dec 2022 18:23:45 GMT, Volodymyr Paprotski wrote: >> There is now an intrinsic for Poly1305, which is only enabled on the >> `engineUpdate([]byte)` path. This PR adds intrinsic support >> `engineUpdate(ByteBuffer)` (when the bytebuffer `hasArray`). >> >> Fuzzing test expanded to also include ByteBuffer payloads. >> >> Performance is now matched: >> >> Benchmark (dataSize) (provider) Mode Cnt >> ScoreError Units >> Poly1305DigestBench.digestBuffer 64 thrpt3 >> 3320909.857 ± 787300.545 ops/s >> Poly1305DigestBench.digestBuffer 256 thrpt3 >> 3006447.324 ± 704790.796 ops/s >> Poly1305DigestBench.digestBuffer1024 thrpt3 >> 2645041.509 ± 664904.622 ops/s >> Poly1305DigestBench.digestBuffer 16384 thrpt3 >> 893389.209 ± 6288.743 ops/s >> Poly1305DigestBench.digestBuffer 1048576 thrpt3 >> 14932.680 ±170.238 ops/s >> Poly1305DigestBench.digestBytes 64 thrpt3 >> 3548298.515 ± 859964.530 ops/s >> Poly1305DigestBench.digestBytes 256 thrpt3 >> 3083261.994 ± 141802.417 ops/s >> Poly1305DigestBench.digestBytes 1024 thrpt3 >> 2704357.584 ± 554683.019 ops/s >> Poly1305DigestBench.digestBytes16384 thrpt3 >> 898913.339 ± 99733.295 ops/s >> Poly1305DigestBench.digestBytes 1048576 thrpt3 >> 14961.872 ± 38.003 ops/s >> Finished running test >> 'micro:org.openjdk.bench.javax.crypto.full.Poly1305DigestBench' >> >> Relates to: >> - https://github.com/openjdk/jdk/pull/11463: Found inconsistency between >> processing `[]byte` and `ByteBuffer`. When that one is fixed, >> `Poly1305IntrinsicFuzzTest.java` should not be setting the endianness on the >> `ByteBuffer` >> - Intrinsic introduced by https://github.com/openjdk/jdk/pull/10582. > > Volodymyr Paprotski has updated the pull request incrementally with one > additional commit since the last revision: > > remove comment @valeriepeng Could you please take a look at this PR? - PR: https://git.openjdk.org/jdk/pull/11338
Re: RFR: 8298108: Add a regression test for JDK-8297684 [v3]
> Please review this test addition as it would have helped discover a > regression when https://bugs.openjdk.org/browse/JDK-8269039 got introduced > and subsequently backported. What's more, it might help discover similar > issues going forward. Thoughts? > > Testing: Manually produced a build without > [JDK-8269039](https://bugs.openjdk.org/browse/JDK-8269039). Test fails > without the fix, passes after. Severin Gehwolf has updated the pull request incrementally with one additional commit since the last revision: Remove unused Path arg for test invocation - Changes: - all: https://git.openjdk.org/jdk/pull/11515/files - new: https://git.openjdk.org/jdk/pull/11515/files/c380b8ae..ebc86659 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=11515=02 - incr: https://webrevs.openjdk.org/?repo=jdk=11515=01-02 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/11515.diff Fetch: git fetch https://git.openjdk.org/jdk pull/11515/head:pull/11515 PR: https://git.openjdk.org/jdk/pull/11515
Re: RFR: 8298108: Add a regression test for JDK-8297684 [v2]
> Please review this test addition as it would have helped discover a > regression when https://bugs.openjdk.org/browse/JDK-8269039 got introduced > and subsequently backported. What's more, it might help discover similar > issues going forward. Thoughts? > > Testing: Manually produced a build without > [JDK-8269039](https://bugs.openjdk.org/browse/JDK-8269039). Test fails > without the fix, passes after. Severin Gehwolf has updated the pull request incrementally with one additional commit since the last revision: Remove extraneous process builder creation - Changes: - all: https://git.openjdk.org/jdk/pull/11515/files - new: https://git.openjdk.org/jdk/pull/11515/files/c2418a8d..c380b8ae Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=11515=01 - incr: https://webrevs.openjdk.org/?repo=jdk=11515=00-01 Stats: 1 line in 1 file changed: 0 ins; 1 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/11515.diff Fetch: git fetch https://git.openjdk.org/jdk pull/11515/head:pull/11515 PR: https://git.openjdk.org/jdk/pull/11515
RFR: 8298108: Add a regression test for JDK-8297684
Please review this test addition as it would have helped discover a regression when https://bugs.openjdk.org/browse/JDK-8269039 got introduced and subsequently backported. What's more, it might help discover similar issues going forward. Thoughts? Testing: Manually produced a build without [JDK-8269039](https://bugs.openjdk.org/browse/JDK-8269039). Test fails without the fix, passes after. - Commit messages: - 8298108: Add a regression test for JDK-8297684 Changes: https://git.openjdk.org/jdk/pull/11515/files Webrev: https://webrevs.openjdk.org/?repo=jdk=11515=00 Issue: https://bugs.openjdk.org/browse/JDK-8298108 Stats: 177 lines in 3 files changed: 177 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/11515.diff Fetch: git fetch https://git.openjdk.org/jdk pull/11515/head:pull/11515 PR: https://git.openjdk.org/jdk/pull/11515
Re: RFR: 8267617: Certificate's IP x509 NameConstraints raises ArrayIndexOutOfBoundsException
On Thu, 1 Dec 2022 16:37:48 GMT, Daniel Jeliński wrote: > This patch fixes the exceptions that may occur when merging IP address > NameConstraints from different certificates in a chain. > > The included test reports 3 exceptions without the fix, passes with the fix > applied. > > Tiers 1-3 continue to pass. Looks good. - Marked as reviewed by mullan (Reviewer). PR: https://git.openjdk.org/jdk/pull/11459
