Integrated: Merge jdk17
On Fri, 18 Jun 2021 22:17:41 GMT, Jesper Wilhelmsson wrote: > Forwardport JDK 17 -> JDK 18 This pull request has now been integrated. Changeset: b7d78a5b Author:Jesper Wilhelmsson URL: https://git.openjdk.java.net/jdk/commit/b7d78a5b661e2b00f271298db3b6cc873cf754e7 Stats: 12229 lines in 119 files changed: 6768 ins; 5337 del; 124 mod Merge - PR: https://git.openjdk.java.net/jdk/pull/4533
Re: RFR: Merge jdk17 [v2]
> Forwardport JDK 17 -> JDK 18 Jesper Wilhelmsson has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 46 commits: - Merge - 8267042: bug in monitor locking/unlocking on ARM32 C1 due to uninitialized BasicObjectLock::_displaced_header Co-authored-by: Chris Cole Reviewed-by: dsamersoff - 8268964: Remove unused ReferenceProcessorAtomicMutator Reviewed-by: tschatzl, pliden - 8268900: com/sun/net/httpserver/Headers.java: Fix indentation and whitespace Reviewed-by: dfuchs, chegar, michaelm - Merge - 8268678: LetsEncryptCA.java test fails as Let’s Encrypt Authority X3 is retired Reviewed-by: xuelei - 8267189: Remove duplicated unregistered classes from dynamic archive Reviewed-by: ccheung, minqi - 8268638: semaphores of AsyncLogWriter may be broken when JVM is exiting. Reviewed-by: dholmes, phh - 8268556: Use bitmap for storing regions that failed evacuation Reviewed-by: kbarrett, iwalulya, sjohanss - 8268294: Reusing HttpClient in a WebSocket.Listener hangs. Reviewed-by: dfuchs - ... and 36 more: https://git.openjdk.java.net/jdk/compare/b8f073be...ed622f4b - Changes: https://git.openjdk.java.net/jdk/pull/4533/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4533=01 Stats: 8681 lines in 159 files changed: 7992 ins; 386 del; 303 mod Patch: https://git.openjdk.java.net/jdk/pull/4533.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4533/head:pull/4533 PR: https://git.openjdk.java.net/jdk/pull/4533
RFR: Merge jdk17
Forwardport JDK 17 -> JDK 18 - Commit messages: - Merge - 8268316: Typo in JFR jdk.Deserialization event - 8268638: semaphores of AsyncLogWriter may be broken when JVM is exiting. - 8264775: ClhsdbFindPC still fails with java.lang.RuntimeException: 'In java stack' missing from stdout/stderr - 8265073: XML transformation and indentation when using xml:space - 8269025: jsig/Testjsig.java doesn't check exit code - 8266518: Refactor and expand scatter/gather tests - 8268903: JFR: RecordingStream::dump is missing @since - 8265369: [macos-aarch64] java/net/MulticastSocket/Promiscuous.java failed with "SocketException: Cannot allocate memory" - 8268564: mark hotspot serviceability/attach tests which ignore external VM flags - ... and 13 more: https://git.openjdk.java.net/jdk/compare/8f2456e5...ed622f4b The merge commit only contains trivial merges, so no merge-specific webrevs have been generated. Changes: https://git.openjdk.java.net/jdk/pull/4533/files Stats: 12229 lines in 119 files changed: 6768 ins; 5337 del; 124 mod Patch: https://git.openjdk.java.net/jdk/pull/4533.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4533/head:pull/4533 PR: https://git.openjdk.java.net/jdk/pull/4533
Re: RFR: 8268427: Improve AlgorithmConstraints:checkAlgorithm performance [v3]
On Thu, 17 Jun 2021 08:16:42 GMT, Dongbo He wrote: >> Now AlgorithmConstraints:checkAlgorithm uses List to check if an algorithm >> has been disabled. It is less efficient when there are more disabled >> elements in the list, we can use Set instead of List to speed up the search. >> >> Patch contains a benchmark that can be run with `make test >> TEST="micro:java.security.AlgorithmConstraintsPermits"`. >> Baseline results before patch: >> >> Benchmark(algorithm) Mode CntScore >> Error Units >> AlgorithmConstraintsPermits.permitsSSLv3 avgt5 21.687 ? >> 1.118 ns/op >> AlgorithmConstraintsPermits.permits DES avgt5 324.216 ? >> 6.233 ns/op >> AlgorithmConstraintsPermits.permits NULL avgt5 709.462 ? >> 51.259 ns/op >> AlgorithmConstraintsPermits.permits TLS1.3 avgt5 687.497 ? >> 170.181 ns/op >> >> Benchmark results after patch: >> >> Benchmark(algorithm) Mode CntScore >> Error Units >> AlgorithmConstraintsPermits.permitsSSLv3 avgt5 46.407 ? >> 1.057 ns/op >> AlgorithmConstraintsPermits.permits DES avgt5 65.722 ? >> 0.578 ns/op >> AlgorithmConstraintsPermits.permits NULL avgt5 43.988 ? >> 1.264 ns/op >> AlgorithmConstraintsPermits.permits TLS1.3 avgt5 399.546 ? >> 11.194 ns/op >> >> SSLv3, DES, NULL are the first, middle and last element in >> `jdk.tls.disabledAlgorithms` from `conf/security/java.security`. >> >> Tomcat(maxKeepAliveRequests=1, which will disable HTTP/1.0 >> keep-alive)+Jmeter: >> Before patch: >> >> summary + 50349 in 00:00:30 = 1678.4/s Avg: 238 Min: 188 Max: 298 >> Err: 0 (0.00%) Active: 400 Started: 400 Finished: 0 >> summary = 135183 in 00:01:22 = 1654.5/s Avg: 226 Min:16 Max: 1281 >> Err: 0 (0.00%) >> summary + 50240 in 00:00:30 = 1674.1/s Avg: 238 Min: 200 Max: 308 >> Err: 0 (0.00%) Active: 400 Started: 400 Finished: 0 >> summary = 185423 in 00:01:52 = 1659.7/s Avg: 229 Min:16 Max: 1281 >> Err: 0 (0.00%) >> summary + 50351 in 00:00:30 = 1678.4/s Avg: 238 Min: 191 Max: 306 >> Err: 0 (0.00%) Active: 400 Started: 400 Finished: 0 >> summary = 235774 in 00:02:22 = 1663.7/s Avg: 231 Min:16 Max: 1281 >> Err: 0 (0.00%) >> summary + 50461 in 00:00:30 = 1681.9/s Avg: 237 Min: 174 Max: 303 >> Err: 0 (0.00%) Active: 400 Started: 400 Finished: 0 >> >> After patch: >> >> summary + 59003 in 00:00:30 = 1966.6/s Avg: 203 Min: 158 Max: 272 >> Err: 0 (0.00%) Active: 400 Started: 400 Finished: 0 >> summary = 146675 in 00:01:18 = 1884.6/s Avg: 198 Min:26 Max: 697 >> Err: 0 (0.00%) >> summary + 58965 in 00:00:30 = 1965.9/s Avg: 203 Min: 166 Max: 257 >> Err: 0 (0.00%) Active: 400 Started: 400 Finished: 0 >> summary = 205640 in 00:01:48 = 1907.2/s Avg: 199 Min:26 Max: 697 >> Err: 0 (0.00%) >> summary + 59104 in 00:00:30 = 1969.1/s Avg: 203 Min: 157 Max: 266 >> Err: 0 (0.00%) Active: 400 Started: 400 Finished: 0 >> summary = 264744 in 00:02:18 = 1920.7/s Avg: 200 Min:26 Max: 697 >> Err: 0 (0.00%) >> summary + 59323 in 00:00:30 = 1977.6/s Avg: 202 Min: 158 Max: 256 >> Err: 0 (0.00%) Active: 400 Started: 400 Finished: 0 >> >> >> Testing: tier1, tier2 > > Dongbo He has updated the pull request incrementally with one additional > commit since the last revision: > > Replace HashSet with TreeSet I will let Xuelei finish his review. From what I see in the code, the Comparator is used when the algorithm is fully disabled. The use of keywords later on in the Constraints constructor uses the ConstraintsEntry which I believe is stored with the upper and lower case. I think the code should be ok. - PR: https://git.openjdk.java.net/jdk/pull/4424
Re: RFR: 8268873: Unnecessary Vector usage in java.base
On Mon, 14 Jun 2021 11:34:50 GMT, Andrey Turbanov wrote: > Usage of thread-safe collection `Vector` is unnecessary. It's recommended to > use `ArrayList` if a thread-safe implementation is not needed. > I checked only places where `Vector` was used as local variable. The change to `PKCS7::verify(byte[])` looks fine. - Marked as reviewed by mullan (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/4482
Re: RFR: 8268698: Use Objects.check{Index, FromToIndex, FromIndexSize} where possible [v2]
On Fri, 18 Jun 2021 05:54:01 GMT, Yi Yang wrote:
>> After JDK-8265518(#3615), it's possible to replace all variants of
>> checkIndex by
>> Objects.checkIndex/Objects.checkFromToIndex/Objects.checkFromIndexSize in
>> the whole JDK codebase.
>
> Yi Yang has updated the pull request incrementally with one additional commit
> since the last revision:
>
> restore IndexOfOufBoundsException; split exception line
@kahatlen for cases earlier in VM startup you need to avoid method references
and lambda expressions. See the implementation of
`outOfBoundsExceptionFormatter`, and see the usage in `VarHandle` for two
examples.
Custom exception formaters should ideally be constants held in static final
fields.
I think the API complexity comes down to whether it is necessary to preserve
existing exception messages or not when converting existing code to use the
precondition methods. The API is designed to do that and composes reasonably
well for default exception messages with a non-default exceptions. We could
argue (i would!) it is preferable to have a consistent exception messages for
index out of bounds exceptions, thereby we could collapse and simplify, but
this is sometimes considered a change in behaviour.
src/java.base/share/classes/java/util/Base64.java line 935:
> 933: throw new IOException("Stream is closed");
> 934: Preconditions.checkFromIndexSize(len, off, b.length,
> 935:
> Preconditions.outOfBoundsExceptionFormatter(ArrayIndexOutOfBoundsException::new));
`outOfBoundsExceptionFormatter` will allocate. Store the result of
`Preconditions.outOfBoundsExceptionFormatter(ArrayIndexOutOfBoundsException::new))`
in a public static final on `Preconditions`, and replace the method ref with a
inner class (thereby making it usable earlier at VM startup.
-
PR: https://git.openjdk.java.net/jdk/pull/4507
Re: [jdk17] RFR: 8267100: [BACKOUT] JDK-8196415 Disable SHA-1 Signed JARs
On Fri, 18 Jun 2021 15:21:06 GMT, Sean Mullan wrote: > Please review this fix to backout the change to Disable SHA-1 Signed JARs > from JDK 17 due to a startup performance regression (see > https://bugs.openjdk.java.net/browse/JDK-8266971). The plan is to now address > the performance issue in JDK 18, which will also allow for more bake time. Marked as reviewed by xuelei (Reviewer). - PR: https://git.openjdk.java.net/jdk17/pull/100
[jdk17] RFR: 8267100: [BACKOUT] JDK-8196415 Disable SHA-1 Signed JARs
Please review this fix to backout the change to Disable SHA-1 Signed JARs from JDK 17 due to a startup performance regression (see https://bugs.openjdk.java.net/browse/JDK-8266971). The plan is to now address the performance issue in JDK 18, which will also allow for more bake time. - Commit messages: - [BACKOUT] JDK-8196415 Disable SHA-1 Signed JARs. Changes: https://git.openjdk.java.net/jdk17/pull/100/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk17=100=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8267100 Stats: 3 lines in 1 file changed: 0 ins; 1 del; 2 mod Patch: https://git.openjdk.java.net/jdk17/pull/100.diff Fetch: git fetch https://git.openjdk.java.net/jdk17 pull/100/head:pull/100 PR: https://git.openjdk.java.net/jdk17/pull/100
Re: [jdk17] RFR: 8268349: Provide clear run-time warnings about Security Manager deprecation [v5]
On Fri, 18 Jun 2021 02:30:22 GMT, Jaikiran Pai wrote:
>> Weijun Wang has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> verbose warning message test and renaming in System.java
>
> src/java.base/share/classes/java/lang/System.java line 381:
>
>> 379: if (allowSecurityManager()) {
>> 380: var caller = Reflection.getCallerClass();
>> 381: String signature = caller.getName() + " (" +
>> codeSource(caller) + ")";
>
> Hello Weijun,
>
> Given that the `codeSource()` method above is allowed to return `null`, there
> could be a case where the warning message printed would be something like:
>
>>
>> WARNING: A terminally deprecated method in java.lang.System has been called
>> WARNING: System::setSecurityManager has been called by foo.bar.Hello (null)
>> WARNING: Please consider reporting this to the maintainers of foo.bar.Hello
>> WARNING: System::setSecurityManager will be removed in a future release
>>
>
> Would that be OK or do you think the presence of "(null)" be unnecessary and
> confusing? Maybe in such cases that line should just say
> "System::setSecurityManager has been called by foo.bar.Hello"?
>
> Another minor nit - the variable is named `signature` which initially gave me
> an impression that it was the method signature of the caller code, but that
> isn't the case. Should this variable be renamed perhaps?
Good catches. Will fix it. Thanks.
-
PR: https://git.openjdk.java.net/jdk17/pull/13
Re: [jdk17] RFR: 8268349: Provide clear run-time warnings about Security Manager deprecation [v6]
> More loudly and precise warning messages when a security manager is either > enabled at startup or installed at runtime. > > This is new PR for the `openjdk/jdk17` repo copied from > https://github.com/openjdk/jdk/pull/4400. A new commit is added. Weijun Wang has updated the pull request incrementally with one additional commit since the last revision: no more null as codeSource, rename one variable - Changes: - all: https://git.openjdk.java.net/jdk17/pull/13/files - new: https://git.openjdk.java.net/jdk17/pull/13/files/1e1e7221..885a0072 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk17=13=05 - incr: https://webrevs.openjdk.java.net/?repo=jdk17=13=04-05 Stats: 9 lines in 1 file changed: 6 ins; 0 del; 3 mod Patch: https://git.openjdk.java.net/jdk17/pull/13.diff Fetch: git fetch https://git.openjdk.java.net/jdk17 pull/13/head:pull/13 PR: https://git.openjdk.java.net/jdk17/pull/13
Re: RFR: 8266182: Create a manual test for jdk/sun/security/pkcs12/ParamsTest.java [v3]
> ParamsTest is an interop test between keytool <-> openssl. There are some > manual steps listed in jdk/sun/security/pkcs12/params/README to perform after > the execution of jtreg execution. So this test is to perform that manual > steps. Abdul Kolarkunnu has updated the pull request incrementally with one additional commit since the last revision: 8266182: Create a manual test for jdk/sun/security/pkcs12/ParamsTest.java - Changes: - all: https://git.openjdk.java.net/jdk/pull/4413/files - new: https://git.openjdk.java.net/jdk/pull/4413/files/4969232f..20d6bb5f Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=4413=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4413=01-02 Stats: 175 lines in 3 files changed: 78 ins; 97 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/4413.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4413/head:pull/4413 PR: https://git.openjdk.java.net/jdk/pull/4413
Re: RFR: 8266182: Create a manual test for jdk/sun/security/pkcs12/ParamsTest.java [v2]
On Fri, 18 Jun 2021 06:06:01 GMT, Sibabrata Sahoo wrote:
>> Abdul Kolarkunnu has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> 8266182: Create a manual test for jdk/sun/security/pkcs12/ParamsTest.java
>
> test/jdk/sun/security/pkcs12/KeytoolOpensslInterop.sh line 25:
>
>> 23:
>> 24: # Use OpenSSL 1.1.0i or above versions, earlier versions may generate
>> different
>> 25: # info and test fail.
>
> It will be more helpful, if some instruction given about finding openssl, any
> pre-post requirement and any specific way to validate the result after
> execution for this manual Test execution.
Added more details.
> test/jdk/sun/security/pkcs12/KeytoolOpensslInteropTest.java line 43:
>
>> 41: */
>> 42:
>> 43: public class KeytoolOpensslInteropTest {
>
> This file is not necessary and the shell script can have @run tag instead.
Yes, corrected that way. Removed this file and done everything in the shell
script.
-
PR: https://git.openjdk.java.net/jdk/pull/4413
Re: RFR: JDK-8268464 : Remove dependancy of TestHttpsServer, HttpTransaction, HttpCallback from open/test/jdk/sun/net/www/protocol/https/ tests [v4]
On Thu, 17 Jun 2021 16:23:08 GMT, Mahendra Chhipa wrote: >> …HttpCallback from open/test/jdk/sun/net/www/protocol/https/ tests > > Mahendra Chhipa has updated the pull request incrementally with one > additional commit since the last revision: > > Implemented review comments Marked as reviewed by dfuchs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/4432
Re: [jdk17] RFR: 8268349: Provide clear run-time warnings about Security Manager deprecation [v5]
On 18/06/2021 03:51, Jaikiran Pai wrote: : Given all that, do you think that we should be printing the jar file paths in this WARNING message by default? I read the linked CSR, but I didn't see why the location of the jar or the name of the jar would be useful in this warning message. As long as the caller class (and perhaps the caller method) is printed, I think that should be enough of a summary on what's triggering this warning. It's not hugely different to the "Illegal reflective access" warnings except I wouldn't expect many libraries to call setSecurityManager in a running system. Instead, it tends to be the main application that installs the SM at startup. However if code in a library were to call it then it useful to identify where that code is. -Alan.
Re: RFR: 8266182: Create a manual test for jdk/sun/security/pkcs12/ParamsTest.java [v2]
On Fri, 18 Jun 2021 05:43:15 GMT, Abdul Kolarkunnu
wrote:
>> ParamsTest is an interop test between keytool <-> openssl. There are some
>> manual steps listed in jdk/sun/security/pkcs12/params/README to perform
>> after the execution of jtreg execution. So this test is to perform that
>> manual steps.
>
> Abdul Kolarkunnu has updated the pull request incrementally with one
> additional commit since the last revision:
>
> 8266182: Create a manual test for jdk/sun/security/pkcs12/ParamsTest.java
test/jdk/sun/security/pkcs12/KeytoolOpensslInterop.sh line 25:
> 23:
> 24: # Use OpenSSL 1.1.0i or above versions, earlier versions may generate
> different
> 25: # info and test fail.
It will be more helpful, if some instruction given about finding openssl, any
pre-post requirement and any specific way to validate the result after
execution for this manual Test execution.
test/jdk/sun/security/pkcs12/KeytoolOpensslInteropTest.java line 43:
> 41: */
> 42:
> 43: public class KeytoolOpensslInteropTest {
This file is not necessary and the shell script can have @run tag instead.
-
PR: https://git.openjdk.java.net/jdk/pull/4413
Re: RFR: 8268698: Use Objects.check{Index, FromToIndex, FromIndexSize} where possible [v2]
On Thu, 17 Jun 2021 15:45:47 GMT, Paul Sandoz wrote:
>> src/java.base/share/classes/java/util/Base64.java line 934:
>>
>>> 932: if (closed)
>>> 933: throw new IOException("Stream is closed");
>>> 934: Preconditions.checkFromIndexSize(len, off, b.length, (xa,
>>> xb) -> new ArrayIndexOutOfBoundsException());
>>
>> You might want to split this really long line too, to avoid inconsistent
>> line length in this source file.
>
> I would suggest factoring out `(xa, xb) -> new
> ArrayIndexOutOfBoundsException()` as a reusable component on `Preconditions`,
> and maybe even supplying an exception message (since it is rather useful, and
> way better than no message).
>
> See the usages of `Preconditions.outOfBoundsExceptionFormatter` (where there
> just happens to be many repeated cases of supplying AIOOBE with a message,
> that could also be consolidated, separate fix perhaps).
Ok, I've replaced
Preconditions.checkFromIndexSize(len, off, b.length, (xa, xb) -> new
ArrayIndexOutOfBoundsException());
with
Preconditions.checkFromIndexSize(len, off, b.length,
Preconditions.outOfBoundsExceptionFormatter(ArrayIndexOutOfBoundsException::new));
I am curious about the motivations of current APIs:
public static
int checkIndex(int index, int length,
BiFunction, X> oobef) {
if (index < 0 || index >= length)
throw outOfBoundsCheckIndex(oobef, index, length);
return index;
}
Are they over-engineered? When I checked all checkIndex-like patterns in the
whole jdk codebase, I found that in most cases, providing an API that accepts
custom exception should be enough for scalability:
int checkIndex(int index, int length, IndexOutOfBoundException iooe) {
if (index < 0 || index >= length)
throw iooe;
return index;
}
In addition to the ease-of-use problem, there is another problem with the
current API design.
Some methods in j.l.String are good replacement candidates for
Preconditions.check{Index,...}:
https://github.com/openjdk/jdk/blob/a051e735cda0d5ee5cb6ce0738aa549a7319a28c/src/java.base/share/classes/java/lang/String.java#L4558-L4604
But we **can not** do such replacement after my practice.
The third parameter of Preconditions.checkIndex is a BiFunction, which can be
passed a lambda as its argument. The generated lambda method exercises many
other codes like MethodHandles, j.l.Package, etc, eventually it called
j.l.String.checkIndex itself, so if we replace j.l.String.checkIndex with
`Preconditions.checkIndex(a,b,(x1,x2)->)`, a StackOverflowError would occur
at VM startup.
If there is an API that accepts user-defined exceptions, I think we can apply
Preconditions in more places.
-
PR: https://git.openjdk.java.net/jdk/pull/4507
Re: RFR: 8268698: Use Objects.check{Index, FromToIndex, FromIndexSize} where possible [v2]
On Fri, 18 Jun 2021 05:54:01 GMT, Yi Yang wrote: >> After JDK-8265518(#3615), it's possible to replace all variants of >> checkIndex by >> Objects.checkIndex/Objects.checkFromToIndex/Objects.checkFromIndexSize in >> the whole JDK codebase. > > Yi Yang has updated the pull request incrementally with one additional commit > since the last revision: > > restore IndexOfOufBoundsException; split exception line > _Mailing list message from [David Holmes](mailto:david.hol...@oracle.com) on > [serviceability-dev](mailto:serviceability-...@mail.openjdk.java.net):_ > > On 17/06/2021 8:50 pm, Alan Bateman wrote: > > > On Thu, 17 Jun 2021 05:16:14 GMT, David Holmes > > wrote: > > > There are a lot more tests than just tier1. :) I don't expect many, if > > > any, tests to be looking for a specific IOOBE message, and I can't see an > > > easy way to find such tests without running them. If core-libs folk are > > > okay with this update then I guess we can just handle any test failures > > > if they arise. But I'll run this through our tier 1-3 testing. > > > > > > It would be good to run tier 1-3. Off hand I can't think of any tests that > > are dependent on the exception message, I think I'm more concerned about > > changing behavior (once you throw a more specific IOOBE is some of the very > > core classes then it's hard to change it because libraries get dependent on > > the more specific exception). > > tiers 1-3 on Linux-x64 passed okay. > > Any change to the exact type of exception thrown should be affirmed > through a CSR request. > > Cheers, > David Thank you David for running these tests! - PR: https://git.openjdk.java.net/jdk/pull/4507
