Re: RFR: JDK-8266670: Better modeling of access flags in core reflection [v20]
On Wed, 1 Jun 2022 05:01:33 GMT, ExE Boss wrote:
>> src/java.base/share/classes/java/lang/reflect/Member.java line 96:
>>
>>> 94: */
>>> 95: public default Set accessFlags() {
>>> 96: return Set.of();
>>
>> Is is not better to throw a NoSuchMethodError instead of Set.of() if there
>> is no implementation.
>
> Or `AbstractMethodError`, which is what `Executable::getParameterCount()`
> does:
> https://github.com/openjdk/jdk/blob/e751b7b1b6f7269a1fe20c07748c726536388f6d/src/java.base/share/classes/java/lang/reflect/Executable.java#L248-L258
Actually, it should probably be `UnsupportedOperationException` instead.
-
PR: https://git.openjdk.java.net/jdk/pull/7445
Re: RFR: JDK-8266670: Better modeling of access flags in core reflection [v20]
On Tue, 31 May 2022 17:26:35 GMT, Rémi Forax wrote:
>> Joe Darcy 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 32 additional commits
>> since the last revision:
>>
>> - Target JDK 20 rather than 19.
>> - Merge branch 'master' into JDK-8266670
>> - Add mask values to constants' javadoc.
>> - Implement review feedback from mlchung.
>> - Fix type in @throws tag.
>> - Merge branch 'master' into JDK-8266670
>> - Respond to review feedback.
>> - Merge branch 'master' into JDK-8266670
>> - Make workding changes suggested in review feedback.
>> - Merge branch 'master' into JDK-8266670
>> - ... and 22 more:
>> https://git.openjdk.java.net/jdk/compare/db2b501c...05cf2d8b
>
> src/java.base/share/classes/java/lang/reflect/Member.java line 96:
>
>> 94: */
>> 95: public default Set accessFlags() {
>> 96: return Set.of();
>
> Is is not better to throw a NoSuchMethodError instead of Set.of() if there is
> no implementation.
Or `AbstractMethodError`, which is what `Executable::getParameterCount()` does:
https://github.com/openjdk/jdk/blob/e751b7b1b6f7269a1fe20c07748c726536388f6d/src/java.base/share/classes/java/lang/reflect/Executable.java#L248-L258
-
PR: https://git.openjdk.java.net/jdk/pull/7445
Re: RFR: 8236569: -Xss not multiple of 4K does not work for the main thread on macOS [v2]
On Tue, 31 May 2022 15:10:38 GMT, Adam Sotona wrote: >> This is continuation of PR #4256 >> >> The patch simply rounds up the specified stack size to multiple of the >> system page size, on systems where necessary. >> The patch is based on the original PR/branch, with reflected remaining >> recommendations. >> >> Please review. >> >> Thank you, >> Adam > > Adam Sotona has updated the pull request incrementally with one additional > commit since the last revision: > > Updated Java mannpage Looks good. - Marked as reviewed by stuefe (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8953
Re: RFR: JDK-8284858: Start of release updates for JDK 20 [v4]
On Wed, 1 Jun 2022 03:18:44 GMT, Joe Darcy wrote: >> Time to start getting ready for JDK 20... > > Joe Darcy 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 38 additional commits since > the last revision: > > - Adjust deprecated options test. > - Merge branch 'master' into JDK-8284858 > - Update deprecated options test. > - Merge branch 'master' into JDK-8284858 > - Respond to review feedback. > - Update symbol information for JDK 19 b24. > - Merge branch 'master' into JDK-8284858 > - Merge branch 'master' into JDK-8284858 > - Merge branch 'master' into JDK-8284858 > - Merge branch 'master' into JDK-8284858 > - ... and 28 more: > https://git.openjdk.java.net/jdk/compare/c1abb195...54e872b5 Updates look good. Thanks - Marked as reviewed by dholmes (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8236
Re: RFR: 8287340: Refactor old code using StringTokenizer in locale related code
On Tue, 31 May 2022 17:46:18 GMT, Naoto Sato wrote: > Refactoring some old code in locale providers. The test case data have also > been modified due to: > - There's a bug in `LocaleProviderAdapter.toLocaleArray()` where it did not > handle the case for `no-NO-NY`. > - `Locale.toLanguageTag()` won't handle legacy Java locales, e.g., `ja_JP_JP` > and falls back, so comparing locales using language tags does not work for > those locales. Changed to compare with `Locale.toString()` instead. src/java.base/share/classes/sun/util/cldr/CLDRLocaleProviderAdapter.java line 181: > 179: .toArray(Locale[]::new); > 180: } > 181: return AVAILABLE_LOCALES; This should probably clone the cached array: Suggestion: return AVAILABLE_LOCALES.clone(); Matching what `JRELocaleProviderAdapter` does[^1], since there’s no guarantee that the result of `getAvailableLocales()` won’t be mutated. [^1]: https://github.com/openjdk/jdk/blob/6b1169e266b9d21864f886ef574dd64116fa2cb0/src/java.base/share/classes/sun/util/locale/provider/JRELocaleProviderAdapter.java#L430-L439 - PR: https://git.openjdk.java.net/jdk/pull/8960
Re: RFR: JDK-8284858: Start of release updates for JDK 20 [v3]
On Tue, 31 May 2022 20:32:13 GMT, Joe Darcy wrote: >> Time to start getting ready for JDK 20... > > Joe Darcy 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 36 additional commits since > the last revision: > > - Update deprecated options test. > - Merge branch 'master' into JDK-8284858 > - Respond to review feedback. > - Update symbol information for JDK 19 b24. > - Merge branch 'master' into JDK-8284858 > - Merge branch 'master' into JDK-8284858 > - Merge branch 'master' into JDK-8284858 > - Merge branch 'master' into JDK-8284858 > - Merge branch 'master' into JDK-8284858 > - Update symbol information for JDK 19 b23. > - ... and 26 more: > https://git.openjdk.java.net/jdk/compare/44ff5c88...eedd36bd langtools files look OK, although I did skim through the auto-generated new data/symbols files. There are possibilities in the langtools file to simplify naming and default initialization of member fields in various places, in a separate PR, separate from these changes. - Marked as reviewed by jjg (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8236
Re: RFR: JDK-8284858: Start of release updates for JDK 20 [v4]
> Time to start getting ready for JDK 20... Joe Darcy 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 38 additional commits since the last revision: - Adjust deprecated options test. - Merge branch 'master' into JDK-8284858 - Update deprecated options test. - Merge branch 'master' into JDK-8284858 - Respond to review feedback. - Update symbol information for JDK 19 b24. - Merge branch 'master' into JDK-8284858 - Merge branch 'master' into JDK-8284858 - Merge branch 'master' into JDK-8284858 - Merge branch 'master' into JDK-8284858 - ... and 28 more: https://git.openjdk.java.net/jdk/compare/2ffc9a25...54e872b5 - Changes: - all: https://git.openjdk.java.net/jdk/pull/8236/files - new: https://git.openjdk.java.net/jdk/pull/8236/files/eedd36bd..54e872b5 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8236=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8236=02-03 Stats: 1101 lines in 21 files changed: 708 ins; 170 del; 223 mod Patch: https://git.openjdk.java.net/jdk/pull/8236.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8236/head:pull/8236 PR: https://git.openjdk.java.net/jdk/pull/8236
Re: RFR: 8287171: Refactor null caller tests to a single directory [v6]
> Created a test at test/jdk/jdk/nullCaller called NullCallerTest that creates > a test module with some resources in it for the actual tests that occur at > the native level. The native part was switched to c++ instead of c to make it > easier to create helper objects that reduce the redundant code performing > error checking. The two helper classes InstanceCall and StaticCall were > placed in an include file called CallHelper.hpp. The main part of the tests > are in exeNullCallerTest.cpp, and there is a separate function for each of > the bug reports. Tim Prinzing has updated the pull request incrementally with one additional commit since the last revision: move jni/nullCaller tests to jdk_lang group - Changes: - all: https://git.openjdk.java.net/jdk/pull/8934/files - new: https://git.openjdk.java.net/jdk/pull/8934/files/777c4f5f..8bf4f597 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8934=05 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8934=04-05 Stats: 4 lines in 1 file changed: 1 ins; 1 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/8934.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8934/head:pull/8934 PR: https://git.openjdk.java.net/jdk/pull/8934
Re: RFR: 8287171: Refactor null caller tests to a single directory [v5]
On Wed, 1 Jun 2022 02:45:24 GMT, Tim Prinzing wrote: >> Created a test at test/jdk/jdk/nullCaller called NullCallerTest that creates >> a test module with some resources in it for the actual tests that occur at >> the native level. The native part was switched to c++ instead of c to make >> it easier to create helper objects that reduce the redundant code performing >> error checking. The two helper classes InstanceCall and StaticCall were >> placed in an include file called CallHelper.hpp. The main part of the tests >> are in exeNullCallerTest.cpp, and there is a separate function for each of >> the bug reports. > > Tim Prinzing has updated the pull request incrementally with one additional > commit since the last revision: > > improve comment The tests are mostly java/lang, but also java/util so I was looking for something else. The jdk_lang seems fairly reasonable and they run tier1 as the tests used to run before the move if I put them there. I'll update TEST.groups. - PR: https://git.openjdk.java.net/jdk/pull/8934
Re: RFR: 8287171: Refactor null caller tests to a single directory [v5]
> Created a test at test/jdk/jdk/nullCaller called NullCallerTest that creates > a test module with some resources in it for the actual tests that occur at > the native level. The native part was switched to c++ instead of c to make it > easier to create helper objects that reduce the redundant code performing > error checking. The two helper classes InstanceCall and StaticCall were > placed in an include file called CallHelper.hpp. The main part of the tests > are in exeNullCallerTest.cpp, and there is a separate function for each of > the bug reports. Tim Prinzing has updated the pull request incrementally with one additional commit since the last revision: improve comment - Changes: - all: https://git.openjdk.java.net/jdk/pull/8934/files - new: https://git.openjdk.java.net/jdk/pull/8934/files/7eb5dbcb..777c4f5f Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8934=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8934=03-04 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/8934.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8934/head:pull/8934 PR: https://git.openjdk.java.net/jdk/pull/8934
Re: RFR: 8287171: Refactor null caller tests to a single directory [v4]
On Wed, 1 Jun 2022 00:59:33 GMT, Tim Prinzing wrote: >> Created a test at test/jdk/jdk/nullCaller called NullCallerTest that creates >> a test module with some resources in it for the actual tests that occur at >> the native level. The native part was switched to c++ instead of c to make >> it easier to create helper objects that reduce the redundant code performing >> error checking. The two helper classes InstanceCall and StaticCall were >> placed in an include file called CallHelper.hpp. The main part of the tests >> are in exeNullCallerTest.cpp, and there is a separate function for each of >> the bug reports. > > Tim Prinzing has updated the pull request incrementally with one additional > commit since the last revision: > > Moved the tests test/jdk/jni/nullCaller. Added to the jdk_other group > which runs as part of tier2. Made the other requested changes such as > not using the bugid as the function name for the test and using the > name of the main method being tested instead. jdk_other is for other modules. jdk_lang may be an option since this is to test when attached with JNI thread. - PR: https://git.openjdk.java.net/jdk/pull/8934
Re: RFR: 8273346: Expand library mappings to IEEE 754 operations [v2]
On Wed, 25 May 2022 08:56:27 GMT, Raffaello Giulietti
wrote:
>> Joe Darcy 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 four additional commits
>> since the last revision:
>>
>> - Respond to review feedback; make another pass to link methods to IEEE 754
>> operations.
>> - Merge branch 'master' into JDK-8273346
>> - Add floor and ceil.
>> - JDK-8273346: Examine use of "rounding mode" and "rounding policy" in Math
>> and StrictMath
>
> src/java.base/share/classes/java/lang/Math.java line 771:
>
>> 769: * This method corresponds to the convertToIntegerTiesToEven
>> 770: * operation defined in IEEE 754.
>> 771: *
>
> IEEE `convertToIntegerTiesToEven` rounds ties to the even integer.
> The method's spec, however, requires ties to round toward positive infinity.
> Unfortunately, IEEE 754 doesn't offer a `convertToIntegerTiesToPositive`
Good catch; found a different 754 operation that seems to fit.
> src/java.base/share/classes/java/math/RoundingMode.java line 49:
>
>> 47: * exponent range of {@code BigDecimal}, the mathematical result will
>> 48: * be exactly representable in the result precision or fall between
>> 49: * two representable values. In the case of falling between two
>
> Perhaps better would be
> `two adjacent representable values.`
Agreed; adjusted in subsequent push.
-
PR: https://git.openjdk.java.net/jdk/pull/8876
Re: RFR: 8236569: -Xss not multiple of 4K does not work for the main thread on macOS [v2]
On Tue, 31 May 2022 15:10:38 GMT, Adam Sotona wrote: >> This is continuation of PR #4256 >> >> The patch simply rounds up the specified stack size to multiple of the >> system page size, on systems where necessary. >> The patch is based on the original PR/branch, with reflected remaining >> recommendations. >> >> Please review. >> >> Thank you, >> Adam > > Adam Sotona has updated the pull request incrementally with one additional > commit since the last revision: > > Updated Java mannpage Thanks for the manpage update! - Marked as reviewed by dholmes (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8953
Re: RFR: 8273346: Expand library mappings to IEEE 754 operations [v2]
On Wed, 1 Jun 2022 02:05:40 GMT, Joe Darcy wrote: >> Generally add apiNote's to map from Java library methods to particular IEEE >> 754 operations. For now, I only added such notes to java.lang.Math and not >> java.lang.StrictMath. > > Joe Darcy 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 four additional commits since > the last revision: > > - Respond to review feedback; make another pass to link methods to IEEE 754 > operations. > - Merge branch 'master' into JDK-8273346 > - Add floor and ceil. > - JDK-8273346: Examine use of "rounding mode" and "rounding policy" in Math > and StrictMath I look another pass over the methods in Float, Double, and Math for opportunities to link to IEEE 754 operations. At least for now, I choose not to link to abs since IEEE 754 abs can have different behavior for NaN values that java.lang.Math.abs. If a link is made, this difference would need to be noted. - PR: https://git.openjdk.java.net/jdk/pull/8876
Re: RFR: 8273346: Expand library mappings to IEEE 754 operations [v2]
> Generally add apiNote's to map from Java library methods to particular IEEE > 754 operations. For now, I only added such notes to java.lang.Math and not > java.lang.StrictMath. Joe Darcy 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 four additional commits since the last revision: - Respond to review feedback; make another pass to link methods to IEEE 754 operations. - Merge branch 'master' into JDK-8273346 - Add floor and ceil. - JDK-8273346: Examine use of "rounding mode" and "rounding policy" in Math and StrictMath - Changes: - all: https://git.openjdk.java.net/jdk/pull/8876/files - new: https://git.openjdk.java.net/jdk/pull/8876/files/050e6ecd..9169ad9e Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8876=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8876=00-01 Stats: 46471 lines in 566 files changed: 22606 ins; 17922 del; 5943 mod Patch: https://git.openjdk.java.net/jdk/pull/8876.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8876/head:pull/8876 PR: https://git.openjdk.java.net/jdk/pull/8876
Re: RFR: 8286279: [vectorapi] Only check index of masked lanes if offset is out of array boundary for masked store [v2]
On Mon, 30 May 2022 01:17:00 GMT, Xiaohong Gong wrote: >> Xiaohong Gong has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Wrap the offset check into a static method > > @PaulSandoz, could you please help to check whether the current version is ok > for you? Thanks so much! > @XiaohongGong looks good, now the Vector API JEP has been integrated you will > get a merge conflict, but it should be easier to resolve. Great! I will resolve the conflict and update the patch, thanks so much! - PR: https://git.openjdk.java.net/jdk/pull/8620
Re: RFR: 8287171: Refactor null caller tests to a single directory [v4]
> Created a test at test/jdk/jdk/nullCaller called NullCallerTest that creates > a test module with some resources in it for the actual tests that occur at > the native level. The native part was switched to c++ instead of c to make it > easier to create helper objects that reduce the redundant code performing > error checking. The two helper classes InstanceCall and StaticCall were > placed in an include file called CallHelper.hpp. The main part of the tests > are in exeNullCallerTest.cpp, and there is a separate function for each of > the bug reports. Tim Prinzing has updated the pull request incrementally with one additional commit since the last revision: Moved the tests test/jdk/jni/nullCaller. Added to the jdk_other group which runs as part of tier2. Made the other requested changes such as not using the bugid as the function name for the test and using the name of the main method being tested instead. - Changes: - all: https://git.openjdk.java.net/jdk/pull/8934/files - new: https://git.openjdk.java.net/jdk/pull/8934/files/f1406a45..7eb5dbcb Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8934=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8934=02-03 Stats: 13 lines in 10 files changed: 1 ins; 0 del; 12 mod Patch: https://git.openjdk.java.net/jdk/pull/8934.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8934/head:pull/8934 PR: https://git.openjdk.java.net/jdk/pull/8934
Re: RFR: 8287496: Alternative virtual thread implementation that maps to OS thread [v2]
On Tue, 31 May 2022 15:39:39 GMT, Alan Bateman wrote: >> This patch adds an alternative virtual thread implementation where each >> virtual thread is backed by an OS thread. It doesn't scale but it can be >> used by ports that don't have continuations support in the VM. Aside from >> scalability, the lack of continuations support means: >> >> 1. JVM TI is not supported when running with --enable-preview (the JVM TI >> spec allows for this) >> 2. jshell --enable-preview can't be used (as jshell uses the debugger APIs >> and so needs JVM TI) >> >> The VM option "VMContinuations" is added as an experimental option so it can >> be used by tests. A number of tests are changed to re-run with >> -XX:-VMContinuations. A new jtreg property is added so that tests that need >> the underlying VM support to be present can use "@requires vm.continuations" >> in the test description. A follow-up change would be to add "@requires >> vm.continuations" to the ~70 serviceability/jvmti/vthread that run with >> preview features enabled. > > Alan Bateman has updated the pull request incrementally with one additional > commit since the last revision: > > Allowing linking without intrinsic stub, contributed-by: rehn Looks okay to me. src/java.management/share/classes/sun/management/ThreadImpl.java line 586: > 584: > 585: /** > 586: * Returns the ThreadInfo objects from the given array that > coresspond to platform typo: coresspond -> correspond src/java.management/share/classes/sun/management/Util.java line 92: > 90: > 91: /** > 92: * Returns true if the given Thread is a virutal thread. typo: virutal -> virtual this typo appears in other comments too. - Marked as reviewed by mchung (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8939
Re: RFR: 8285838: DST not applying properly with zone id offset set with TZ env variable [v5]
On Tue, 31 May 2022 20:10:44 GMT, Gaurav Chaudhari wrote: >> This fix ensures that when a lookup for a custom TZ code fails, and an >> attempt is made to find the GMT offset in order to get the current time, >> Daylight savings rules are applied correctly. > > Gaurav Chaudhari has updated the pull request incrementally with one > additional commit since the last revision: > > 8285838: Corrected month comparison check for TZ DST I tried out your patch on my local Linux machine, but the new test failed with the following exception: ACTION: main -- Failed. Execution failed: `main' threw exception: java.lang.RuntimeException: Expected to get exit value of [0] REASON: User specified action: run main/othervm CustomTzIDCheckDST TIME: 1.564 seconds messages: command: main CustomTzIDCheckDST reason: User specified action: run main/othervm CustomTzIDCheckDST Mode: othervm [/othervm specified] elapsed time (seconds): 1.564 configuration: STDOUT: Command line: [/home/nsato/projects/jdk/git/jdk/build/linux-x64/images/jdk/bin/java -cp /home/nsato/projects/jdk/git/jdk/build/linux-x64/test-support/jtreg_open_test_jdk_java_util_TimeZone/classes/0/java/util/TimeZone/CustomTzIDCheckDST.d:/home/nsato/projects/jdk/git/jdk/open/test/jdk/java/util/TimeZone:/home/nsato/projects/jdk/git/jdk/build/linux-x64/test-support/jtreg_open_test_jdk_java_util_TimeZone/classes/0/test/lib:/home/nsato/projects/jdk/git/jdk/open/test/lib:/var/tmp/jib-nsato/install/jtreg/6.1/1/bundles/jtreg-6.1+1.zip/jtreg/lib/javatest.jar:/var/tmp/jib-nsato/install/jtreg/6.1/1/bundles/jtreg-6.1+1.zip/jtreg/lib/jtreg.jar CustomTzIDCheckDST runTZTest ] [2022-05-31T22:27:05.958567816Z] Gathering output for process 14771 [2022-05-31T22:27:06.635595481Z] Waiting for completion for process 14771 [2022-05-31T22:27:06.635976964Z] Waiting for completion finished for process 14771 Output and diagnostic info for process 14771 was saved into 'pid-14771-output.log' [2022-05-31T22:27:06.663087767Z] Waiting for completion for process 14771 [2022-05-31T22:27:06.663360403Z] Waiting for completion finished for process 14771 [2022-05-31T22:27:06.663754609Z] Waiting for completion for process 14771 [2022-05-31T22:27:06.663869034Z] Waiting for completion finished for process 14771 STDERR: stdout: []; stderr: [Exception in thread "main" java.time.DateTimeException: Invalid ID for offset-based ZoneId: GMT-22:00 at java.base/java.time.ZoneId.ofWithPrefix(ZoneId.java:436) at java.base/java.time.ZoneId.of(ZoneId.java:406) at java.base/java.time.ZoneId.of(ZoneId.java:358) at java.base/java.time.ZoneId.of(ZoneId.java:314) at java.base/java.util.TimeZone.toZoneId0(TimeZone.java:581) at java.base/java.util.TimeZone.toZoneId(TimeZone.java:558) at java.base/java.util.TimeZone.toZoneId0(TimeZone.java:570) at java.base/java.util.TimeZone.toZoneId(TimeZone.java:558) at java.base/java.time.ZoneId.systemDefault(ZoneId.java:274) at CustomTzIDCheckDST.runTZTest(CustomTzIDCheckDST.java:64) at CustomTzIDCheckDST.main(CustomTzIDCheckDST.java:51) Caused by: java.time.DateTimeException: Zone offset hours not in valid range: value -22 is not in the range -18 to 18 at java.base/java.time.ZoneOffset.validate(ZoneOffset.java:373) at java.base/java.time.ZoneOffset.ofHoursMinutesSeconds(ZoneOffset.java:326) at java.base/java.time.ZoneOffset.of(ZoneOffset.java:257) at java.base/java.time.ZoneId.ofWithPrefix(ZoneId.java:430) ... 10 more ] exitValue = 1 java.lang.RuntimeException: Expected to get exit value of [0] at jdk.test.lib.process.OutputAnalyzer.shouldHaveExitValue(OutputAnalyzer.java:489) at CustomTzIDCheckDST.main(CustomTzIDCheckDST.java:49) at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:104) at java.base/java.lang.reflect.Method.invoke(Method.java:578) at com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapper.java:127) at java.base/java.lang.Thread.run(Thread.java:1585) JavaTest Message: Test threw exception: java.lang.RuntimeException: Expected to get exit value of [0] JavaTest Message: shutting down test STATUS:Failed.`main' threw exception: java.lang.RuntimeException: Expected to get exit value of [0] - PR: https://git.openjdk.java.net/jdk/pull/8661
Re: RFR: 4511638: Double.toString(double) sometimes produces incorrect results [v14]
On Tue, 31 May 2022 17:07:06 GMT, Raffaello Giulietti wrote: >> Hello, >> >> here's a PR for a patch submitted on March 2020 >> [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was >> a thing. >> >> The patch has been edited to adhere to OpenJDK code conventions about >> multi-line (block) comments. Nothing in the code proper has changed, except >> for the addition of redundant but clarifying parentheses in some expressions. >> >> >> Greetings >> Raffaello > > Raffaello Giulietti has updated the pull request incrementally with one > additional commit since the last revision: > > 4511638: Double.toString(double) sometimes produces incorrect results src/java.base/share/classes/jdk/internal/math/DoubleToDecimal.java line 72: > 70: static final int E_MAX = 309; > 71: > 72: /* Threshold to detect tiny values, as in section 8.2.1 of [1] */ Comments like this one pointing to specific places in the Schubfach paper are very helpful in understanding the constants and the various steps in the algorithm. - PR: https://git.openjdk.java.net/jdk/pull/3402
Re: RFR: 8178355: IdentityHashMap uses identity-based comparison for values everywhere except remove(K,V) and replace(K,V,V)
On Thu, 21 Apr 2022 00:48:00 GMT, Stuart Marks wrote: >> Explicitly implement `remove` and `replace` in `IdentityHashMap` to compare >> values by identity. Updated API documentation of these two methods >> ([Preview](https://cr.openjdk.java.net/~liach/8178355/IdentityHashMap.html#remove(java.lang.Object,java.lang.Object))) >> to mention such behavior. > > Thanks for working on this. Yes I can review and sponsor this change. > > Sorry I got a bit distracted. I started looking at the test here, and this > lead me to inquire about what other tests we have for `IdentityHashMap`, and > the answer is: not enough. See > [JDK-8285295](https://bugs.openjdk.java.net/browse/JDK-8285295). (But that > should be handled separately.) @stuart-marks Just curious, would this fix target 19 or 20 at the current state? - PR: https://git.openjdk.java.net/jdk/pull/8259
Re: RFR: 4511638: Double.toString(double) sometimes produces incorrect results [v14]
On Tue, 31 May 2022 17:07:06 GMT, Raffaello Giulietti
wrote:
>> Hello,
>>
>> here's a PR for a patch submitted on March 2020
>> [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was
>> a thing.
>>
>> The patch has been edited to adhere to OpenJDK code conventions about
>> multi-line (block) comments. Nothing in the code proper has changed, except
>> for the addition of redundant but clarifying parentheses in some expressions.
>>
>>
>> Greetings
>> Raffaello
>
> Raffaello Giulietti has updated the pull request incrementally with one
> additional commit since the last revision:
>
> 4511638: Double.toString(double) sometimes produces incorrect results
src/java.base/share/classes/jdk/internal/math/FloatToDecimal.java line 40:
> 38: final public class FloatToDecimal {
> 39: /*
> 40: * For full details about this code see the following references:
Same comment about ``.
src/java.base/share/classes/jdk/internal/math/FloatToDecimal.java line 97:
> 95: private static final int MASK_28 = (1 << 28) - 1;
> 96:
> 97: private static final int NON_SPECIAL= 0;
As these are shared with `DoubleToDecimal` would these constants be better
moved to a common location, e.g., `MathUtils` whether converted to an `enum` or
not?
src/java.base/share/classes/jdk/internal/math/FloatToDecimal.java line 118:
> 116: private int index;
> 117:
> 118: private FloatToDecimal() {
Same comment about preventing instantiation.
-
PR: https://git.openjdk.java.net/jdk/pull/3402
Re: RFR: 4511638: Double.toString(double) sometimes produces incorrect results [v14]
On Tue, 31 May 2022 17:07:06 GMT, Raffaello Giulietti
wrote:
>> Hello,
>>
>> here's a PR for a patch submitted on March 2020
>> [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was
>> a thing.
>>
>> The patch has been edited to adhere to OpenJDK code conventions about
>> multi-line (block) comments. Nothing in the code proper has changed, except
>> for the addition of redundant but clarifying parentheses in some expressions.
>>
>>
>> Greetings
>> Raffaello
>
> Raffaello Giulietti has updated the pull request incrementally with one
> additional commit since the last revision:
>
> 4511638: Double.toString(double) sometimes produces incorrect results
src/java.base/share/classes/jdk/internal/math/DoubleToDecimal.java line 97:
> 95: private static final int MASK_28 = (1 << 28) - 1;
> 96:
> 97: private static final int NON_SPECIAL= 0;
Would these constants be better as an enum?
src/java.base/share/classes/jdk/internal/math/DoubleToDecimal.java line 118:
> 116: private int index;
> 117:
> 118: private DoubleToDecimal() {
Maybe add a comment like
/**
* Prevent instantiation.
*/
or
// Prevent instantiation of this class.
-
PR: https://git.openjdk.java.net/jdk/pull/3402
Re: RFR: 4511638: Double.toString(double) sometimes produces incorrect results [v14]
On Tue, 31 May 2022 17:07:06 GMT, Raffaello Giulietti wrote: >> Hello, >> >> here's a PR for a patch submitted on March 2020 >> [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was >> a thing. >> >> The patch has been edited to adhere to OpenJDK code conventions about >> multi-line (block) comments. Nothing in the code proper has changed, except >> for the addition of redundant but clarifying parentheses in some expressions. >> >> >> Greetings >> Raffaello > > Raffaello Giulietti has updated the pull request incrementally with one > additional commit since the last revision: > > 4511638: Double.toString(double) sometimes produces incorrect results src/java.base/share/classes/jdk/internal/math/DoubleToDecimal.java line 47: > 45: * [2] IEEE Computer Society, "IEEE Standard for Floating-Point > Arithmetic" > 46: * > 47: * [3] Bouvier & Zimmermann, "Division-Free Binary-to-Decimal > Conversion" Similar comment concerning `` tag as in `MathUtils`. - PR: https://git.openjdk.java.net/jdk/pull/3402
Re: RFR: JDK-8284858: Start of release updates for JDK 20 [v3]
On Tue, 31 May 2022 20:32:13 GMT, Joe Darcy wrote: >> Time to start getting ready for JDK 20... > > Joe Darcy 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 36 additional commits since > the last revision: > > - Update deprecated options test. > - Merge branch 'master' into JDK-8284858 > - Respond to review feedback. > - Update symbol information for JDK 19 b24. > - Merge branch 'master' into JDK-8284858 > - Merge branch 'master' into JDK-8284858 > - Merge branch 'master' into JDK-8284858 > - Merge branch 'master' into JDK-8284858 > - Merge branch 'master' into JDK-8284858 > - Update symbol information for JDK 19 b23. > - ... and 26 more: > https://git.openjdk.java.net/jdk/compare/d01d01b5...eedd36bd Marked as reviewed by iris (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8236
Re: RFR: 4511638: Double.toString(double) sometimes produces incorrect results [v14]
On Tue, 31 May 2022 17:07:06 GMT, Raffaello Giulietti
wrote:
>> Hello,
>>
>> here's a PR for a patch submitted on March 2020
>> [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was
>> a thing.
>>
>> The patch has been edited to adhere to OpenJDK code conventions about
>> multi-line (block) comments. Nothing in the code proper has changed, except
>> for the addition of redundant but clarifying parentheses in some expressions.
>>
>>
>> Greetings
>> Raffaello
>
> Raffaello Giulietti has updated the pull request incrementally with one
> additional commit since the last revision:
>
> 4511638: Double.toString(double) sometimes produces incorrect results
src/java.base/share/classes/jdk/internal/math/MathUtils.java line 38:
> 36: *
> 37: * Giulietti, "The Schubfach way to render doubles",
> 38: *
> https://drive.google.com/file/d/1gp5xv4CAa78SVgCeWfGqqI4FfYYYuNFb
Even though not public, should the reference use the `` tag and perhaps
be in a `@see` annotation?
@see https://drive.google.com/file/d/1gp5xv4CAa78SVgCeWfGqqI4FfYYYuNFb”>
The Schubfach way to render doubles
src/java.base/share/classes/jdk/internal/math/MathUtils.java line 193:
> 191: */
> 192: private static final long[] g = {
> 193: /* -324 */ 0x4F0C_EDC9_5A71_8DD4L, 0x5B01_E8B0_9AA0_D1B5L,
Not significant, but might this be clearer instead to comment the array
elements on the right?
0x4F0C_EDC9_5A71_8DD4L, 0x5B01_E8B0_9AA0_D1B5L, // -324
-
PR: https://git.openjdk.java.net/jdk/pull/3402
Re: RFR: 8284780: Need methods to create pre-sized HashSet and LinkedHashSet [v17]
On Fri, 27 May 2022 18:40:32 GMT, XenoAmess wrote: >> as title. > > XenoAmess has updated the pull request incrementally with one additional > commit since the last revision: > > do it as naotoj said Reviewers, could I get a review for CSR [JDK-8287419](https://bugs.openjdk.java.net/browse/JDK-8287419) please? Thanks. - PR: https://git.openjdk.java.net/jdk/pull/8302
Integrated: 8287544: Replace uses of StringBuffer with StringBuilder in java.naming
On Sun, 29 May 2022 21:57:46 GMT, Andrey Turbanov wrote: > StringBuffer is a legacy synchronized class. StringBuilder is a direct > replacement to StringBuffer which generally have better performance. > There are some code that still uses StringBuffer in java.naming which could > be migrated to `StringBuilder`. This pull request has now been integrated. Changeset: f5bbade9 Author:Andrey Turbanov URL: https://git.openjdk.java.net/jdk/commit/f5bbade9e40ed99d53d50c828d672b4eaab35018 Stats: 9 lines in 2 files changed: 0 ins; 0 del; 9 mod 8287544: Replace uses of StringBuffer with StringBuilder in java.naming Reviewed-by: rriggs, aefimov, vtewari - PR: https://git.openjdk.java.net/jdk/pull/8942
Re: RFR: 8282662: Use List.of() factory method to reduce memory consumption [v8]
On Tue, 31 May 2022 19:30:07 GMT, Сергей Цыпанов wrote: >> `List.of()` along with `Set.of()` create unmodifiable `List/Set` but with >> smaller footprint comparing to `Arrays.asList()` / `new HashSet()` when >> called with vararg of size 0, 1, 2. >> >> In general replacement of `Arrays.asList()` with `List.of()` is dubious as >> the latter is null-hostile, however in some cases we are sure that arguments >> are non-null. Into this PR I've included the following cases (in addition to >> those where the argument is proved to be non-null at compile-time): >> - `MethodHandles.longestParameterList()` never returns null >> - parameter types are never null >> - interfaces used for proxy construction and returned from >> `Class.getInterfaces()` are never null >> - exceptions types of method signature are never null > > Сергей Цыпанов has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains six commits: > > - 8282662: Revert wrong copyright year change > - 8282662: Revert ProxyGenerator > - 8282662: Revert ProxyGenerator > - 8282662: Revert dubious changes in MethodType > - 8282662: Revert dubious changes > - 8282662: Use List/Set.of() factory methods to save memory Merging is preferable, it doesn't disturb the history. Usually an explicit merge is not necessary; Skara will indicate when an automatic merge is needed due to conflicts. If you want to run your own tests then use a merge, not rebase. Thanks - PR: https://git.openjdk.java.net/jdk/pull/7729
Re: RFR: 8282662: Use List.of() factory method to reduce memory consumption [v8]
On Tue, 31 May 2022 19:33:15 GMT, Roger Riggs wrote: >> Сергей Цыпанов 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: >> >> - 8282662: Revert wrong copyright year change >> - 8282662: Revert ProxyGenerator >> - 8282662: Revert ProxyGenerator >> - 8282662: Revert dubious changes in MethodType >> - 8282662: Revert dubious changes >> - 8282662: Use List/Set.of() factory methods to save memory > > Why the force push? They are discouraged, making it harder to review. @RogerRiggs I've rebased my changes onto master to incorporate the latest changes. - PR: https://git.openjdk.java.net/jdk/pull/7729
Re: RFR: 8287340: Refactor old code using StringTokenizer in locale related code
On Tue, 31 May 2022 17:46:18 GMT, Naoto Sato wrote: > Refactoring some old code in locale providers. The test case data have also > been modified due to: > - There's a bug in `LocaleProviderAdapter.toLocaleArray()` where it did not > handle the case for `no-NO-NY`. > - `Locale.toLanguageTag()` won't handle legacy Java locales, e.g., `ja_JP_JP` > and falls back, so comparing locales using language tags does not work for > those locales. Changed to compare with `Locale.toString()` instead. Marked as reviewed by joehw (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8960
Re: RFR: JDK-8284858: Start of release updates for JDK 20 [v3]
> Time to start getting ready for JDK 20... Joe Darcy 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 36 additional commits since the last revision: - Update deprecated options test. - Merge branch 'master' into JDK-8284858 - Respond to review feedback. - Update symbol information for JDK 19 b24. - Merge branch 'master' into JDK-8284858 - Merge branch 'master' into JDK-8284858 - Merge branch 'master' into JDK-8284858 - Merge branch 'master' into JDK-8284858 - Merge branch 'master' into JDK-8284858 - Update symbol information for JDK 19 b23. - ... and 26 more: https://git.openjdk.java.net/jdk/compare/57d97d36...eedd36bd - Changes: - all: https://git.openjdk.java.net/jdk/pull/8236/files - new: https://git.openjdk.java.net/jdk/pull/8236/files/96be1787..eedd36bd Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8236=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8236=01-02 Stats: 41686 lines in 344 files changed: 18832 ins; 17644 del; 5210 mod Patch: https://git.openjdk.java.net/jdk/pull/8236.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8236/head:pull/8236 PR: https://git.openjdk.java.net/jdk/pull/8236
Re: RFR: 8284942: Proxy building can just iterate superinterfaces once [v6]
On Tue, 31 May 2022 19:33:49 GMT, Mandy Chung wrote: >> liach has updated the pull request incrementally with one additional commit >> since the last revision: >> >> Fixes suggested by mandy > > Looks good. Thanks. @mlchung Would you mind sponsoring this patch? - PR: https://git.openjdk.java.net/jdk/pull/8281
Re: RFR: 8285838: DST not applying properly with zone id offset set with TZ env variable [v5]
> This fix ensures that when a lookup for a custom TZ code fails, and an > attempt is made to find the GMT offset in order to get the current time, > Daylight savings rules are applied correctly. Gaurav Chaudhari has updated the pull request incrementally with one additional commit since the last revision: 8285838: Corrected month comparison check for TZ DST - Changes: - all: https://git.openjdk.java.net/jdk/pull/8661/files - new: https://git.openjdk.java.net/jdk/pull/8661/files/d7831659..9e3b1bb4 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8661=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8661=03-04 Stats: 3 lines in 1 file changed: 1 ins; 1 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/8661.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8661/head:pull/8661 PR: https://git.openjdk.java.net/jdk/pull/8661
Re: RFR: 8287340: Refactor old code using StringTokenizer in locale related code
On Tue, 31 May 2022 17:46:18 GMT, Naoto Sato wrote: > Refactoring some old code in locale providers. The test case data have also > been modified due to: > - There's a bug in `LocaleProviderAdapter.toLocaleArray()` where it did not > handle the case for `no-NO-NY`. > - `Locale.toLanguageTag()` won't handle legacy Java locales, e.g., `ja_JP_JP` > and falls back, so comparing locales using language tags does not work for > those locales. Changed to compare with `Locale.toString()` instead. Very nice code moderization. - Marked as reviewed by iris (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8960
Re: RFR: 8285838: DST not applying properly with zone id offset set with TZ env variable [v4]
On Tue, 31 May 2022 17:21:50 GMT, Naoto Sato wrote:
>> Gaurav Chaudhari has updated the pull request incrementally with two
>> additional commits since the last revision:
>>
>> - Merge branch '8285838' of github.com:Deigue/jdk into 8285838
>> - Merge branch '8285838' of github.com:Deigue/jdk into 8285838
>
> test/jdk/java/util/TimeZone/CustomTzIDCheckDST.java line 67:
>
>> 65: if ((month > Month.MARCH.getValue() && month <
>> Month.OCTOBER.getValue()) ||
>> 66: (month == Month.MARCH.getValue() &&
>> date.isAfter(getLastSundayOfMonth(date))) ||
>> 67: (month == Month.OCTOBER.getValue() &&
>> date.isBefore(getLastSundayOfMonth(date {
>
> I don't think these conditions are correct, as `month` is zero-based, and
> comparing it with `Month.MARCH` will be incorrect. The same holds for October.
Oh thats true, just verified this by printing out the values. Will fix by
adding 1 to the month for sensible direct comparison against the singleton
instances.
-
PR: https://git.openjdk.java.net/jdk/pull/8661
Re: RFR: 8284942: Proxy building can just iterate superinterfaces once [v6]
On Thu, 26 May 2022 23:20:27 GMT, liach wrote: >> Currently, in ProxyBuilder::mapToModule and ProxyBuilder::defineProxyClass, >> the interfaces are iterated twice. The two passes can be merged into one, >> yielding the whole proxy definition context (module, package, whether >> there's package-private interface) when determining the module. >> >> Split from #8278. Helpful for moving proxies to hidden classes, but is a >> good cleanup on its own. > > liach has updated the pull request incrementally with one additional commit > since the last revision: > > Fixes suggested by mandy Looks good. Thanks. - Marked as reviewed by mchung (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8281
Re: RFR: 8282662: Use List.of() factory method to reduce memory consumption [v8]
On Tue, 31 May 2022 19:30:07 GMT, Сергей Цыпанов wrote: >> `List.of()` along with `Set.of()` create unmodifiable `List/Set` but with >> smaller footprint comparing to `Arrays.asList()` / `new HashSet()` when >> called with vararg of size 0, 1, 2. >> >> In general replacement of `Arrays.asList()` with `List.of()` is dubious as >> the latter is null-hostile, however in some cases we are sure that arguments >> are non-null. Into this PR I've included the following cases (in addition to >> those where the argument is proved to be non-null at compile-time): >> - `MethodHandles.longestParameterList()` never returns null >> - parameter types are never null >> - interfaces used for proxy construction and returned from >> `Class.getInterfaces()` are never null >> - exceptions types of method signature are never null > > Сергей Цыпанов has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains six commits: > > - 8282662: Revert wrong copyright year change > - 8282662: Revert ProxyGenerator > - 8282662: Revert ProxyGenerator > - 8282662: Revert dubious changes in MethodType > - 8282662: Revert dubious changes > - 8282662: Use List/Set.of() factory methods to save memory Why the force push? They are discouraged, making it harder to review. - PR: https://git.openjdk.java.net/jdk/pull/7729
Re: RFR: 8282662: Use List.of() factory method to reduce memory consumption [v8]
> `List.of()` along with `Set.of()` create unmodifiable `List/Set` but with > smaller footprint comparing to `Arrays.asList()` / `new HashSet()` when > called with vararg of size 0, 1, 2. > > In general replacement of `Arrays.asList()` with `List.of()` is dubious as > the latter is null-hostile, however in some cases we are sure that arguments > are non-null. Into this PR I've included the following cases (in addition to > those where the argument is proved to be non-null at compile-time): > - `MethodHandles.longestParameterList()` never returns null > - parameter types are never null > - interfaces used for proxy construction and returned from > `Class.getInterfaces()` are never null > - exceptions types of method signature are never null Сергей Цыпанов has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains six commits: - 8282662: Revert wrong copyright year change - 8282662: Revert ProxyGenerator - 8282662: Revert ProxyGenerator - 8282662: Revert dubious changes in MethodType - 8282662: Revert dubious changes - 8282662: Use List/Set.of() factory methods to save memory - Changes: https://git.openjdk.java.net/jdk/pull/7729/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=7729=07 Stats: 13 lines in 5 files changed: 1 ins; 2 del; 10 mod Patch: https://git.openjdk.java.net/jdk/pull/7729.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7729/head:pull/7729 PR: https://git.openjdk.java.net/jdk/pull/7729
Re: RFR: 8284942: Proxy building can just iterate superinterfaces once [v3]
On Fri, 20 May 2022 00:10:01 GMT, Mandy Chung wrote: >> liach 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 four additional commits since >> the last revision: >> >> - Updates suggested by mandy >> - Merge branch 'master' into fix/proxy-single-pass >> - Don't need to complexify module cache >> - 8284942: Proxy building can just iterate superinterfaces once > > Looks good with a couple comments. @mlchung Would you review again that I've updated per your suggestions? In addition, I've moved all proxy class context-related validation into the record compact constructor. - PR: https://git.openjdk.java.net/jdk/pull/8281
Integrated: 8285401: Proxy class initializer should use 3-arg `Class.forName` to avoid unnecessary class initialization
On Fri, 20 May 2022 03:50:58 GMT, liach wrote: > Simplify calls `Class.forName(String, boolean, ClassLoader)` instead of > `Class.forName(String)`. `make test > TEST="jtreg:test/jdk/java/lang/reflect/Proxy"` passes, with the new > `LazyInitializationTest` failing the eager initialization check on the > baseline and passing with this patch. > > On a side note, this might reduce the number of methods that can be encoded > in a proxy due to code attribute size restrictions; we probably would address > that in another issue, as we never mandated a count of methods that the proxy > must be able to implement. > > Mandy, would you mind review this? This pull request has now been integrated. Changeset: e0382c55 Author:liach Committer: Roger Riggs URL: https://git.openjdk.java.net/jdk/commit/e0382c552348d108e906792ad8ca7067f9f805ec Stats: 76 lines in 2 files changed: 72 ins; 0 del; 4 mod 8285401: Proxy class initializer should use 3-arg `Class.forName` to avoid unnecessary class initialization Reviewed-by: rriggs, mchung - PR: https://git.openjdk.java.net/jdk/pull/8800
Integrated: 8287064: Modernize ProxyGenerator.PrimitiveTypeInfo
On Fri, 20 May 2022 04:55:37 GMT, liach wrote: > Simplify opcode handling, use `final` in `PrimitiveTypeInfo`, and replace the > hash map with a simple lookup, similar to what's done in > [JDK-8284880](https://bugs.openjdk.java.net/browse/JDK-8284880) (#8242) This pull request has now been integrated. Changeset: 37a51300 Author:liach Committer: Roger Riggs URL: https://git.openjdk.java.net/jdk/commit/37a513003c654297d81fc71b64c604f0ab8075cb Stats: 86 lines in 1 file changed: 19 ins; 39 del; 28 mod 8287064: Modernize ProxyGenerator.PrimitiveTypeInfo Reviewed-by: rriggs, mchung - PR: https://git.openjdk.java.net/jdk/pull/8801
Re: RFR: 8287171: Refactor null caller tests to a single directory [v3]
On Mon, 30 May 2022 00:10:50 GMT, Tim Prinzing wrote:
>> Created a test at test/jdk/jdk/nullCaller called NullCallerTest that creates
>> a test module with some resources in it for the actual tests that occur at
>> the native level. The native part was switched to c++ instead of c to make
>> it easier to create helper objects that reduce the redundant code performing
>> error checking. The two helper classes InstanceCall and StaticCall were
>> placed in an include file called CallHelper.hpp. The main part of the tests
>> are in exeNullCallerTest.cpp, and there is a separate function for each of
>> the bug reports.
>
> Tim Prinzing has updated the pull request incrementally with one additional
> commit since the last revision:
>
> problem with iostream on Windows, use printf
test/jdk/jdk/nullCaller/CallHelper.hpp line 176:
> 174: }
> 175:
> 176: // call an method returning an object checking for exceptions and
`s/an method/a method/`
test/jdk/jdk/nullCaller/exeNullCallerTest.cpp line 29:
> 27: * ResourceBundle::getBundle may throw NPE when invoked by JNI code with
> no caller frame
> 28: */
> 29: void JDK_8280902(JNIEnv* env) {
A descriptive method name would be more helpful than the bug ID, for example,
`getResourceBundle` and `registerClassLoaderAsParallelCapable`
test/jdk/jdk/nullCaller/exeNullCallerTest.cpp line 49:
> 47:
> 48: // check the message
> 49: if (std::string("Hello!") != env->GetStringUTFChars(msg,NULL)) {
nit: a space after the comma `(msg, NULL)` consistent with the format in this
local file.
-
PR: https://git.openjdk.java.net/jdk/pull/8934
Re: RFR: 8287171: Refactor null caller tests to a single directory [v3]
On Mon, 30 May 2022 05:37:16 GMT, Alan Bateman wrote: > I don't think jdk/nullCaller is right location for this. Maybe jni/nullCaller > could work. You'll probably need to add the location to an appropriate > group/tier in test/jdk/TEST.groups, otherwise it won't be run. I also think `test/jdk/jni/nullCaller` is better. - PR: https://git.openjdk.java.net/jdk/pull/8934
Re: RFR: 8287384: Speed up jdk.test.lib.util.ForceGC
On Thu, 26 May 2022 18:50:07 GMT, Xue-Lei Andrew Fan wrote: > Hi, > > May I have this test update reviewed? > > The ForceGC could be enhanced by using smaller wait/sleep time, and shared > cleaner. > > Thanks, > Xuelei Sorry, I was working on a history webpage while submit the PR and did not notice new comments. I will address the missed comments in the following up RFE. - PR: https://git.openjdk.java.net/jdk/pull/8907
Re: RFR: 8287384: Speed up jdk.test.lib.util.ForceGC
On Tue, 31 May 2022 17:41:08 GMT, Mandy Chung wrote: > Hmm... one benefit of Cleaner is the ease of use to avoid the need of > managing the reference queue. If the performance of the Cleaner API is a > concern, perhaps we should look into reducing its overhead? The code using a Cleaner here creates a CountDownLatch to wait on; that along with the Cleaner seems very similar to the complexity of the suggested alternative directly using a ReferenceQueue. But I'm fine with it either way Each Cleaner creates a new OS thread, used for a single instance of ForceGC. The caching of the Cleaner in a Static mostly reduces the per ForceGC overhead. When Loom is a feature, a Virtual Thread would be sufficient. - PR: https://git.openjdk.java.net/jdk/pull/8907
RFR: 8287340: Refactor old code using StringTokenizer in locale related code
Refactoring some old code in locale providers. The test case data have also been modified due to: - There's a bug in `LocaleProviderAdapter.toLocaleArray()` where it did not handle the case for `no-NO-NY`. - `Locale.toLanguageTag()` won't handle legacy Java locales, e.g., `ja_JP_JP` and falls back, so comparing locales using language tags does not work for those locales. Changed to compare with `Locale.toString()` instead. - Commit messages: - 8287340: Refactor old code using StringTokenizer in locale related code Changes: https://git.openjdk.java.net/jdk/pull/8960/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8960=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8287340 Stats: 175 lines in 4 files changed: 6 ins; 61 del; 108 mod Patch: https://git.openjdk.java.net/jdk/pull/8960.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8960/head:pull/8960 PR: https://git.openjdk.java.net/jdk/pull/8960
Integrated: 8287384: Speed up jdk.test.lib.util.ForceGC
On Thu, 26 May 2022 18:50:07 GMT, Xue-Lei Andrew Fan wrote: > Hi, > > May I have this test update reviewed? > > The ForceGC could be enhanced by using smaller wait/sleep time, and shared > cleaner. > > Thanks, > Xuelei This pull request has now been integrated. Changeset: d5b6c7bd Author:Xue-Lei Andrew Fan URL: https://git.openjdk.java.net/jdk/commit/d5b6c7bde1ae1ddcc9ad31b99480b67a913ff20a Stats: 21 lines in 1 file changed: 8 ins; 1 del; 12 mod 8287384: Speed up jdk.test.lib.util.ForceGC Reviewed-by: rriggs, bchristi, dfuchs, mchung - PR: https://git.openjdk.java.net/jdk/pull/8907
Re: RFR: 8287496: Alternative virtual thread implementation that maps to OS thread [v2]
On Tue, 31 May 2022 15:39:39 GMT, Alan Bateman wrote: >> This patch adds an alternative virtual thread implementation where each >> virtual thread is backed by an OS thread. It doesn't scale but it can be >> used by ports that don't have continuations support in the VM. Aside from >> scalability, the lack of continuations support means: >> >> 1. JVM TI is not supported when running with --enable-preview (the JVM TI >> spec allows for this) >> 2. jshell --enable-preview can't be used (as jshell uses the debugger APIs >> and so needs JVM TI) >> >> The VM option "VMContinuations" is added as an experimental option so it can >> be used by tests. A number of tests are changed to re-run with >> -XX:-VMContinuations. A new jtreg property is added so that tests that need >> the underlying VM support to be present can use "@requires vm.continuations" >> in the test description. A follow-up change would be to add "@requires >> vm.continuations" to the ~70 serviceability/jvmti/vthread that run with >> preview features enabled. > > Alan Bateman has updated the pull request incrementally with one additional > commit since the last revision: > > Allowing linking without intrinsic stub, contributed-by: rehn Looks good, tested zero, thanks! - Marked as reviewed by rehn (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8939
Re: RFR: 8287384: Speed up jdk.test.lib.util.ForceGC
On Thu, 26 May 2022 21:40:42 GMT, Xue-Lei Andrew Fan wrote: > > Even using a Cleaner is a more overhead than necessary. I would have > > skipped the overhead of a cleaner and Atomic classes with something more > > self contained as a static method: > > I agreed that the using of Cleaner is still heavy, but it may be acceptable > as the code is for testing only. If a new static method is introduced, test > cases using the current API would also need update so as to benefit from it. > I was hesitate for test cases update as it may involve more evaluations (and > risks). But let me check. I evaluated the use of ForceGC (10+ use). It looks like that the update should be straightforward for most of them. Most testing should be placed in case I missed something. I would like to file a new RFE and submit this PR, so that if something is wrong with new update, we could rollback to this. - PR: https://git.openjdk.java.net/jdk/pull/8907
Re: RFR: 8287384: Speed up jdk.test.lib.util.ForceGC
On Thu, 26 May 2022 18:50:07 GMT, Xue-Lei Andrew Fan wrote: > Hi, > > May I have this test update reviewed? > > The ForceGC could be enhanced by using smaller wait/sleep time, and shared > cleaner. > > Thanks, > Xuelei Marked as reviewed by mchung (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8907
Re: RFR: 8287384: Speed up jdk.test.lib.util.ForceGC
On Thu, 26 May 2022 21:40:42 GMT, Xue-Lei Andrew Fan wrote: > Even using a Cleaner is a more overhead than necessary. > I would have skipped the overhead of a cleaner and Atomic classes with > something more self contained as a static method: Hmm... one benefit of Cleaner is the ease of use to avoid the need of managing the reference queue. If the performance of the Cleaner API is a concern, perhaps we should look into reducing its overhead? - PR: https://git.openjdk.java.net/jdk/pull/8907
Re: RFR: 8287384: Speed up jdk.test.lib.util.ForceGC
On Tue, 31 May 2022 13:26:17 GMT, Daniel Fuchs wrote:
>> Hi,
>>
>> May I have this test update reviewed?
>>
>> The ForceGC could be enhanced by using smaller wait/sleep time, and shared
>> cleaner.
>>
>> Thanks,
>> Xuelei
>
> test/lib/jdk/test/lib/util/ForceGC.java line 50:
>
>> 48: for (int i = 0; i < 100; i++) {
>> 49: System.gc();
>> 50: System.out.println("doIt() iter: " + iter + ", gc " + i);
>
> Maybe the trace should be printed only if `(i % 10 == 0) && (iter % 100 ==
> 0)` to avoid having too many traces in the log?
This log was added to help debugging some past issues. With this change, the
volume of this trace would not be that helpful. I think we can remove this
log at this time.
-
PR: https://git.openjdk.java.net/jdk/pull/8907
Re: RFR: JDK-8266670: Better modeling of access flags in core reflection [v20]
On Wed, 25 May 2022 00:35:24 GMT, Joe Darcy wrote:
>> This is an early review of changes to better model JVM access flags, that is
>> "modifiers" like public, protected, etc. but explicitly at a VM level.
>>
>> Language level modifiers and JVM level access flags are closely related, but
>> distinct. There are concepts that overlap in the two domains (public,
>> private, etc.), others that only have a language-level modifier (sealed),
>> and still others that only have an access flag (synthetic).
>>
>> The existing java.lang.reflect.Modifier class is inadequate to model these
>> subtleties. For example, the bit positions used by access flags on different
>> kinds of elements overlap (such as "volatile" for fields and "bridge" for
>> methods. Just having a raw integer does not provide sufficient context to
>> decode the corresponding language-level string. Methods like
>> Modifier.methodModifiers() were introduced to cope with this situation.
>>
>> With additional modifiers and flags on the horizon with projects like
>> Valhalla, addressing the existent modeling deficiency now ahead of time is
>> reasonable before further strain is introduced.
>>
>> This PR in its current form is meant to give the overall shape of the API.
>> It is missing implementations to map from, say, method modifiers to access
>> flags, taking into account overlaps in bit positions.
>>
>> The CSR https://bugs.openjdk.java.net/browse/JDK-8281660 will be filled in
>> once the API is further along.
>
> Joe Darcy 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 32 additional commits since
> the last revision:
>
> - Target JDK 20 rather than 19.
> - Merge branch 'master' into JDK-8266670
> - Add mask values to constants' javadoc.
> - Implement review feedback from mlchung.
> - Fix type in @throws tag.
> - Merge branch 'master' into JDK-8266670
> - Respond to review feedback.
> - Merge branch 'master' into JDK-8266670
> - Make workding changes suggested in review feedback.
> - Merge branch 'master' into JDK-8266670
> - ... and 22 more:
> https://git.openjdk.java.net/jdk/compare/74cd2687...05cf2d8b
src/java.base/share/classes/java/lang/reflect/Member.java line 96:
> 94: */
> 95: public default Set accessFlags() {
> 96: return Set.of();
Is is not better to throw a NoSuchMethodError instead of Set.of() if there is
no implementation.
-
PR: https://git.openjdk.java.net/jdk/pull/7445
Re: RFR: JDK-8266670: Better modeling of access flags in core reflection [v4]
On Tue, 15 Feb 2022 21:05:08 GMT, Joe Darcy wrote: >> Note that the presence or absence of `ACC_SUPER` has no effect since **Java >> 8**, which always treats it as set regardless of the actual contents of the >> binary class file. > > For completeness, I think including SUPER is reasonable, even though has been > a no-op for some time. (Some time in the future, there could be a class file > version aware additions to this enum class.) Well spotted omission. `ACC_SUPER`may be reconed as `ACC_IDENTITY`by Valhalla, so i'm not sure it's a good idea to add ACC_SUPER. - PR: https://git.openjdk.java.net/jdk/pull/7445
Re: RFR: JDK-8266670: Better modeling of access flags in core reflection [v20]
On Wed, 25 May 2022 00:35:24 GMT, Joe Darcy wrote:
>> This is an early review of changes to better model JVM access flags, that is
>> "modifiers" like public, protected, etc. but explicitly at a VM level.
>>
>> Language level modifiers and JVM level access flags are closely related, but
>> distinct. There are concepts that overlap in the two domains (public,
>> private, etc.), others that only have a language-level modifier (sealed),
>> and still others that only have an access flag (synthetic).
>>
>> The existing java.lang.reflect.Modifier class is inadequate to model these
>> subtleties. For example, the bit positions used by access flags on different
>> kinds of elements overlap (such as "volatile" for fields and "bridge" for
>> methods. Just having a raw integer does not provide sufficient context to
>> decode the corresponding language-level string. Methods like
>> Modifier.methodModifiers() were introduced to cope with this situation.
>>
>> With additional modifiers and flags on the horizon with projects like
>> Valhalla, addressing the existent modeling deficiency now ahead of time is
>> reasonable before further strain is introduced.
>>
>> This PR in its current form is meant to give the overall shape of the API.
>> It is missing implementations to map from, say, method modifiers to access
>> flags, taking into account overlaps in bit positions.
>>
>> The CSR https://bugs.openjdk.java.net/browse/JDK-8281660 will be filled in
>> once the API is further along.
>
> Joe Darcy 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 32 additional commits since
> the last revision:
>
> - Target JDK 20 rather than 19.
> - Merge branch 'master' into JDK-8266670
> - Add mask values to constants' javadoc.
> - Implement review feedback from mlchung.
> - Fix type in @throws tag.
> - Merge branch 'master' into JDK-8266670
> - Respond to review feedback.
> - Merge branch 'master' into JDK-8266670
> - Make workding changes suggested in review feedback.
> - Merge branch 'master' into JDK-8266670
> - ... and 22 more:
> https://git.openjdk.java.net/jdk/compare/1545e825...05cf2d8b
src/java.base/share/classes/java/lang/module/ModuleDescriptor.java line 130:
> 128: MANDATED(AccessFlag.MANDATED.mask());
> 129:
> 130: private int mask;
it should be declared final ?
src/java.base/share/classes/java/lang/module/ModuleDescriptor.java line 134:
> 132: this.mask = mask;
> 133: }
> 134: private int mask() {return mask;}
seem useless, `mask` can be accessed directly
-
PR: https://git.openjdk.java.net/jdk/pull/7445
Re: RFR: 8285838: DST not applying properly with zone id offset set with TZ env variable [v4]
On Mon, 30 May 2022 15:40:37 GMT, Gaurav Chaudhari
wrote:
>> This fix ensures that when a lookup for a custom TZ code fails, and an
>> attempt is made to find the GMT offset in order to get the current time,
>> Daylight savings rules are applied correctly.
>
> Gaurav Chaudhari has updated the pull request incrementally with two
> additional commits since the last revision:
>
> - Merge branch '8285838' of github.com:Deigue/jdk into 8285838
> - Merge branch '8285838' of github.com:Deigue/jdk into 8285838
test/jdk/java/util/TimeZone/CustomTzIDCheckDST.java line 67:
> 65: if ((month > Month.MARCH.getValue() && month <
> Month.OCTOBER.getValue()) ||
> 66: (month == Month.MARCH.getValue() &&
> date.isAfter(getLastSundayOfMonth(date))) ||
> 67: (month == Month.OCTOBER.getValue() &&
> date.isBefore(getLastSundayOfMonth(date {
I don't think these conditions are correct, as `month` is zero-based, and
comparing it with `Month.MARCH` will be incorrect. The same holds for October.
-
PR: https://git.openjdk.java.net/jdk/pull/8661
Re: RFR: JDK-8266670: Better modeling of access flags in core reflection [v20]
On Wed, 25 May 2022 00:35:24 GMT, Joe Darcy wrote: >> This is an early review of changes to better model JVM access flags, that is >> "modifiers" like public, protected, etc. but explicitly at a VM level. >> >> Language level modifiers and JVM level access flags are closely related, but >> distinct. There are concepts that overlap in the two domains (public, >> private, etc.), others that only have a language-level modifier (sealed), >> and still others that only have an access flag (synthetic). >> >> The existing java.lang.reflect.Modifier class is inadequate to model these >> subtleties. For example, the bit positions used by access flags on different >> kinds of elements overlap (such as "volatile" for fields and "bridge" for >> methods. Just having a raw integer does not provide sufficient context to >> decode the corresponding language-level string. Methods like >> Modifier.methodModifiers() were introduced to cope with this situation. >> >> With additional modifiers and flags on the horizon with projects like >> Valhalla, addressing the existent modeling deficiency now ahead of time is >> reasonable before further strain is introduced. >> >> This PR in its current form is meant to give the overall shape of the API. >> It is missing implementations to map from, say, method modifiers to access >> flags, taking into account overlaps in bit positions. >> >> The CSR https://bugs.openjdk.java.net/browse/JDK-8281660 will be filled in >> once the API is further along. > > Joe Darcy 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 32 additional commits since > the last revision: > > - Target JDK 20 rather than 19. > - Merge branch 'master' into JDK-8266670 > - Add mask values to constants' javadoc. > - Implement review feedback from mlchung. > - Fix type in @throws tag. > - Merge branch 'master' into JDK-8266670 > - Respond to review feedback. > - Merge branch 'master' into JDK-8266670 > - Make workding changes suggested in review feedback. > - Merge branch 'master' into JDK-8266670 > - ... and 22 more: > https://git.openjdk.java.net/jdk/compare/3d558a34...05cf2d8b I think `Modifier` API is as good as it is right now. The question is whether we keep `Modifier` API as is for the existing usage or enhancing it for the new modifiers such as `value` and `primitive`. No benefit of replacing existing uses of `Modifier` to the new `AccessFlag` API as most existing use are simple tests to determine if it's public, static, final, non-public etc. - PR: https://git.openjdk.java.net/jdk/pull/7445
Re: RFR: 8287496: Alternative virtual thread implementation that maps to OS thread [v2]
On Tue, 31 May 2022 17:03:54 GMT, Aleksey Shipilev wrote: > I expected this change to fix the broken ARM32 port, but it doesn't work. There is work required to get the arm32 port working again, currently tracked as JDK-828636 but there may be further issues beyond that. - PR: https://git.openjdk.java.net/jdk/pull/8939
Re: RFR: 8287520: Shrink x86_32 problemlists after JDK-8287437 [v2]
> [JDK-8287137](https://bugs.openjdk.java.net/browse/JDK-8287137) added a bunch > of tests into problemlist. Those lists basically exclude every test that runs > with --enable-preview. > [JDK-8287437](https://bugs.openjdk.java.net/browse/JDK-8287437) makes it > better: it only disables Loom parts, even with --enable-preview. This allows > testing x86_32 on other preview features and avoids accidental bug slippage > in non-Loom code paths. We can and should shrink the problem lists now. > > Additional testing: > - [x] Removed tests with Linux x86_32 fastdebug; mostly pass, some non-Loom > failures > - [x] Linux x86_32 fastdebug `tier1` > - [x] GHA run Aleksey Shipilev 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 three additional commits since the last revision: - Merge branch 'master' into JDK-8287520-loom-x86-32-shrink - Keep GetEventWriter on problemlist - Fix - Changes: - all: https://git.openjdk.java.net/jdk/pull/8946/files - new: https://git.openjdk.java.net/jdk/pull/8946/files/7c90ead6..a3c3db2b Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8946=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8946=00-01 Stats: 38619 lines in 252 files changed: 17091 ins; 16971 del; 4557 mod Patch: https://git.openjdk.java.net/jdk/pull/8946.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8946/head:pull/8946 PR: https://git.openjdk.java.net/jdk/pull/8946
Re: RFR: 8287496: Alternative virtual thread implementation that maps to OS thread [v2]
On Tue, 31 May 2022 16:54:41 GMT, Boris Ulasevich wrote: > I expected this change to fix the broken ARM32 port, but it doesn't work. It would not fix ARM32, because the interpreter stubs need to be predicated on `Continuations::enabled()`. Also, as my ARM32 experiments show (https://github.com/openjdk/jdk/pull/8634/files#diff-027490ce3f4a92be9b489d9d2e54c7baaea87b7489399b198543c79f1ce1e2e3R4208-R4216) -- there is a breakage somewhere in C2 as well, which this patch would not seem to resolve as well. So, this is a stepping stone for *some* support, but it does not resolve porting situation fully. - PR: https://git.openjdk.java.net/jdk/pull/8939
Re: RFR: 4511638: Double.toString(double) sometimes produces incorrect results [v14]
> Hello, > > here's a PR for a patch submitted on March 2020 > [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a > thing. > > The patch has been edited to adhere to OpenJDK code conventions about > multi-line (block) comments. Nothing in the code proper has changed, except > for the addition of redundant but clarifying parentheses in some expressions. > > > Greetings > Raffaello Raffaello Giulietti has updated the pull request incrementally with one additional commit since the last revision: 4511638: Double.toString(double) sometimes produces incorrect results - Changes: - all: https://git.openjdk.java.net/jdk/pull/3402/files - new: https://git.openjdk.java.net/jdk/pull/3402/files/31ca4e10..ad146ec4 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=3402=13 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3402=12-13 Stats: 19594 lines in 1 file changed: 0 ins; 19594 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/3402.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3402/head:pull/3402 PR: https://git.openjdk.java.net/jdk/pull/3402
Re: RFR: 8287496: Alternative virtual thread implementation that maps to OS thread [v2]
On Tue, 31 May 2022 15:39:39 GMT, Alan Bateman wrote: >> This patch adds an alternative virtual thread implementation where each >> virtual thread is backed by an OS thread. It doesn't scale but it can be >> used by ports that don't have continuations support in the VM. Aside from >> scalability, the lack of continuations support means: >> >> 1. JVM TI is not supported when running with --enable-preview (the JVM TI >> spec allows for this) >> 2. jshell --enable-preview can't be used (as jshell uses the debugger APIs >> and so needs JVM TI) >> >> The VM option "VMContinuations" is added as an experimental option so it can >> be used by tests. A number of tests are changed to re-run with >> -XX:-VMContinuations. A new jtreg property is added so that tests that need >> the underlying VM support to be present can use "@requires vm.continuations" >> in the test description. A follow-up change would be to add "@requires >> vm.continuations" to the ~70 serviceability/jvmti/vthread that run with >> preview features enabled. > > Alan Bateman has updated the pull request incrementally with one additional > commit since the last revision: > > Allowing linking without intrinsic stub, contributed-by: rehn I expected this change to fix the broken ARM32 port, but it doesn't work. # A fatal error has been detected by the Java Runtime Environment: # # Internal Error (templateInterpreterGenerator_arm.cpp:732), pid=27345, tid=27346 # Error: Unimplemented() # # JRE version: (19.0) (build ) # Java VM: OpenJDK Server VM (19-commit6f6486e9-adhoc.re.jdk, mixed mode, g1 gc, linux-arm) # Problematic frame: # V [libjvm.so+0xa14fe0] TemplateInterpreterGenerator::generate_Continuation_doYield_entry()+0x2c - PR: https://git.openjdk.java.net/jdk/pull/8939
Re: RFR: 8286279: [vectorapi] Only check index of masked lanes if offset is out of array boundary for masked store [v2]
On Mon, 30 May 2022 01:17:00 GMT, Xiaohong Gong wrote: >> Xiaohong Gong has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Wrap the offset check into a static method > > @PaulSandoz, could you please help to check whether the current version is ok > for you? Thanks so much! @XiaohongGong looks good, now the Vector API JEP has been integrated you will get a merge conflict, but it should be easier to resolve. - PR: https://git.openjdk.java.net/jdk/pull/8620
Re: RFR: 8287544: Replace uses of StringBuffer with StringBuilder in java.naming
On Sun, 29 May 2022 21:57:46 GMT, Andrey Turbanov wrote: > StringBuffer is a legacy synchronized class. StringBuilder is a direct > replacement to StringBuffer which generally have better performance. > There are some code that still uses StringBuffer in java.naming which could > be migrated to `StringBuilder`. > Looks good to me +1 `java.naming` regression tests also happy with the change - no new failures - PR: https://git.openjdk.java.net/jdk/pull/8942
Re: RFR: 8287544: Replace uses of StringBuffer with StringBuilder in java.naming
On Sun, 29 May 2022 21:57:46 GMT, Andrey Turbanov wrote: > StringBuffer is a legacy synchronized class. StringBuilder is a direct > replacement to StringBuffer which generally have better performance. > There are some code that still uses StringBuffer in java.naming which could > be migrated to `StringBuilder`. Looks ok. - Marked as reviewed by vtewari (Committer). PR: https://git.openjdk.java.net/jdk/pull/8942
Integrated: 8284960: Integration of JEP 426: Vector API (Fourth Incubator)
On Wed, 27 Apr 2022 11:03:48 GMT, Jatin Bhateja wrote: > Hi All, > > Patch adds the planned support for new vector operations and APIs targeted > for [JEP 426: Vector API (Fourth > Incubator).](https://bugs.openjdk.java.net/browse/JDK-8280173) > > Following is the brief summary of changes:- > > 1) Extends the scope of existing lanewise API for following new vector > operations. >- VectorOperations.BIT_COUNT: counts the number of one-bits >- VectorOperations.LEADING_ZEROS_COUNT: counts the number of leading zero > bits >- VectorOperations.TRAILING_ZEROS_COUNT: counts the number of trailing > zero bits >- VectorOperations.REVERSE: reversing the order of bits >- VectorOperations.REVERSE_BYTES: reversing the order of bytes >- compress and expand bits: Semantics are based on Hacker's Delight > section 7-4 Compress, or Generalized Extract. > > 2) Adds following new APIs to perform cross lane vector compress and > expansion operations under the influence of a mask. >- Vector.compress >- Vector.expand >- VectorMask.compress > > 3) Adds predicated and non-predicated versions of following new APIs to load > and store the contents of vector from foreign MemorySegments. > - Vector.fromMemorySegment > - Vector.intoMemorySegment > > 4) C2 Compiler IR enhancements and optimized X86 and AARCH64 backend support > for each newly added operation. > > > Patch has been regressed over AARCH64 and X86 targets different AVX levels. > > Kindly review and share your feedback. > > Best Regards, > Jatin This pull request has now been integrated. Changeset: 6f6486e9 Author:Jatin Bhateja URL: https://git.openjdk.java.net/jdk/commit/6f6486e97743eadfb20b4175e1b4b2b05b59a17a Stats: 38021 lines in 228 files changed: 16652 ins; 16924 del; 4445 mod 8284960: Integration of JEP 426: Vector API (Fourth Incubator) Co-authored-by: Jatin Bhateja Co-authored-by: Paul Sandoz Co-authored-by: Sandhya Viswanathan Co-authored-by: Smita Kamath Co-authored-by: Joshua Zhu Co-authored-by: Xiaohong Gong Co-authored-by: John R Rose Co-authored-by: Eric Liu Co-authored-by: Ningsheng Jian Reviewed-by: ngasson, vlivanov, mcimadamore, jlahoda, kvn - PR: https://git.openjdk.java.net/jdk/pull/8425
Re: RFR: 8284960: Integration of JEP 426: Vector API (Fourth Incubator) [v10]
On Wed, 25 May 2022 06:29:23 GMT, Jatin Bhateja wrote: >> Hi All, >> >> Patch adds the planned support for new vector operations and APIs targeted >> for [JEP 426: Vector API (Fourth >> Incubator).](https://bugs.openjdk.java.net/browse/JDK-8280173) >> >> Following is the brief summary of changes:- >> >> 1) Extends the scope of existing lanewise API for following new vector >> operations. >>- VectorOperations.BIT_COUNT: counts the number of one-bits >>- VectorOperations.LEADING_ZEROS_COUNT: counts the number of leading zero >> bits >>- VectorOperations.TRAILING_ZEROS_COUNT: counts the number of trailing >> zero bits >>- VectorOperations.REVERSE: reversing the order of bits >>- VectorOperations.REVERSE_BYTES: reversing the order of bytes >>- compress and expand bits: Semantics are based on Hacker's Delight >> section 7-4 Compress, or Generalized Extract. >> >> 2) Adds following new APIs to perform cross lane vector compress and >> expansion operations under the influence of a mask. >>- Vector.compress >>- Vector.expand >>- VectorMask.compress >> >> 3) Adds predicated and non-predicated versions of following new APIs to load >> and store the contents of vector from foreign MemorySegments. >> - Vector.fromMemorySegment >> - Vector.intoMemorySegment >> >> 4) C2 Compiler IR enhancements and optimized X86 and AARCH64 backend support >> for each newly added operation. >> >> >> Patch has been regressed over AARCH64 and X86 targets different AVX levels. >> >> Kindly review and share your feedback. >> >> Best Regards, >> Jatin > > Jatin Bhateja has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains 20 commits: > > - 8284960: Post merge cleanups. > - Merge branch 'master' of http://github.com/openjdk/jdk into JDK-8284960 > - 8284960: Review comments resolved. > - 8284960: Integrating incremental patches. > - Merge branch 'master' of http://github.com/openjdk/jdk into JDK-8284960 > - 8284960: Changes to enable jdk.incubator.vector to be treated as preview > participant. Code re-organization related to Reverse/ReverseByte IR > transforms. > - 8284960: Adding --enable-preview in vectorAPI benchmarks. > - Merge branch 'master' of http://github.com/openjdk/jdk into JDK-8284960 > - 8284960: Review comments resolution. > - Merge branch 'master' of http://github.com/openjdk/jdk into JDK-8284960 > - ... and 10 more: > https://git.openjdk.java.net/jdk/compare/742644e2...0f6e1584 Thanks reviewers for your comments. - PR: https://git.openjdk.java.net/jdk/pull/8425
Re: RFR: 8287496: Alternative virtual thread implementation that maps to OS thread [v2]
> This patch adds an alternative virtual thread implementation where each > virtual thread is backed by an OS thread. It doesn't scale but it can be used > by ports that don't have continuations support in the VM. Aside from > scalability, the lack of continuations support means: > > 1. JVM TI is not supported when running with --enable-preview (the JVM TI > spec allows for this) > 2. jshell --enable-preview can't be used (as jshell uses the debugger APIs > and so needs JVM TI) > > The VM option "VMContinuations" is added as an experimental option so it can > be used by tests. A number of tests are changed to re-run with > -XX:-VMContinuations. A new jtreg property is added so that tests that need > the underlying VM support to be present can use "@requires vm.continuations" > in the test description. A follow-up change would be to add "@requires > vm.continuations" to the ~70 serviceability/jvmti/vthread that run with > preview features enabled. Alan Bateman has updated the pull request incrementally with one additional commit since the last revision: Allowing linking without intrinsic stub, contributed-by: rehn - Changes: - all: https://git.openjdk.java.net/jdk/pull/8939/files - new: https://git.openjdk.java.net/jdk/pull/8939/files/7989708b..126e38b6 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8939=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8939=00-01 Stats: 5 lines in 1 file changed: 4 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/8939.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8939/head:pull/8939 PR: https://git.openjdk.java.net/jdk/pull/8939
Re: RFR: 8236569: -Xss not multiple of 4K does not work for the main thread on macOS [v2]
On Tue, 31 May 2022 13:28:30 GMT, David Holmes wrote: >> Adam Sotona has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Updated Java mannpage > > src/java.base/share/classes/sun/launcher/resources/launcher.properties line > 176: > >> 174: \-Xssset java thread stack size\n\ >> 175: \ The actual size may be rounded up to a multiple >> of the\n\ >> 176: \ system page size as required by the operating >> system.\n\ > > The Java manpage will also need this update. Thanks for reminder, I've update Java manpage at both locations. - PR: https://git.openjdk.java.net/jdk/pull/8953
Re: RFR: 8236569: -Xss not multiple of 4K does not work for the main thread on macOS [v2]
> This is continuation of PR #4256 > > The patch simply rounds up the specified stack size to multiple of the system > page size, on systems where necessary. > The patch is based on the original PR/branch, with reflected remaining > recommendations. > > Please review. > > Thank you, > Adam Adam Sotona has updated the pull request incrementally with one additional commit since the last revision: Updated Java mannpage - Changes: - all: https://git.openjdk.java.net/jdk/pull/8953/files - new: https://git.openjdk.java.net/jdk/pull/8953/files/d082aed0..7712f700 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8953=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8953=00-01 Stats: 2 lines in 1 file changed: 2 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/8953.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8953/head:pull/8953 PR: https://git.openjdk.java.net/jdk/pull/8953
Re: RFR: 8287520: Shrink x86_32 problemlists after JDK-8287437
On Tue, 31 May 2022 11:44:27 GMT, Alan Bateman wrote: >> [JDK-8287137](https://bugs.openjdk.java.net/browse/JDK-8287137) added a >> bunch of tests into problemlist. Those lists basically exclude every test >> that runs with --enable-preview. >> [JDK-8287437](https://bugs.openjdk.java.net/browse/JDK-8287437) makes it >> better: it only disables Loom parts, even with --enable-preview. This allows >> testing x86_32 on other preview features and avoids accidental bug slippage >> in non-Loom code paths. We can and should shrink the problem lists now. >> >> Additional testing: >> - [x] Removed tests with Linux x86_32 fastdebug; mostly pass, some non-Loom >> failures >> - [x] Linux x86_32 fastdebug `tier1` >> - [x] GHA run > > This looks okay. If we get PR8939 in then it should be possible to do more > clear out of the exclude lists. Thank you, @AlanBateman. Any other comments? I plan to integrate it soon. - PR: https://git.openjdk.java.net/jdk/pull/8946
Re: RFR: JDK-8266670: Better modeling of access flags in core reflection [v20]
On Fri, 27 May 2022 20:21:12 GMT, Mandy Chung wrote: > With the `AccessFlag` API, what is the role of the `Modifier` API going > forward? [Value Objects JEP](https://openjdk.java.net/jeps/8277163) defines > the new `identity` and `value` modifiers. [PR > #698](https://github.com/openjdk/valhalla/pull/698) proposes to add > `Modifier.IDENTITY` and `Modifier.VALUE` constants as the `identity` and > `value` modifiers are encoded in a class file using the `ACC_IDENTITY` and > `ACC_VALUE` flags. However, with the new improved `AccessFlag` API, the new > flags will be defined in the `AccessFlag` API. I think we should avoid adding > the new flags in `Modifier` and leave it for the existing usage. Use > `AccessFlag` for new features. Looking over the existing uses of Modifier, as relates to Class.getModifiers(), there are simple uses testing for an individual access flag and others that expect a full set of modifier bits that apply to the class. There are a few places that need all the bits to compose a new set of modifier bits and a few others that test for combinations of bits. `Class.getModifiers()` is intrinsified, testing against various bit patterns using the static methods of `Modifier` is very performant. The `Modifer.toString()` method cannot distinguish the flags overloaded by the Class vs Member cases. Modifier.toString() is used by both Class and Member `toString` methods. Methods to convert a Set to a mask and to produce the 'toString' of the set would be useful. To replace all uses of Modifier the existing use cases will need some attention. - PR: https://git.openjdk.java.net/jdk/pull/7445
Re: RFR: 8236569: -Xss not multiple of 4K does not work for the main thread on macOS
On Tue, 31 May 2022 08:37:19 GMT, Adam Sotona wrote: > This is continuation of PR #4256 > > The patch simply rounds up the specified stack size to multiple of the system > page size, on systems where necessary. > The patch is based on the original PR/branch, with reflected remaining > recommendations. > > Please review. > > Thank you, > Adam Changes looks good. But the java manpage needs an update too. Thanks, David src/java.base/share/classes/sun/launcher/resources/launcher.properties line 176: > 174: \-Xssset java thread stack size\n\ > 175: \ The actual size may be rounded up to a multiple > of the\n\ > 176: \ system page size as required by the operating > system.\n\ The Java manpage will also need this update. - Marked as reviewed by dholmes (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8953
Re: RFR: 8287384: Speed up jdk.test.lib.util.ForceGC
On Thu, 26 May 2022 18:50:07 GMT, Xue-Lei Andrew Fan wrote:
> Hi,
>
> May I have this test update reviewed?
>
> The ForceGC could be enhanced by using smaller wait/sleep time, and shared
> cleaner.
>
> Thanks,
> Xuelei
LGTM - but it may be good to have an other reviewer (@mlchung ?)
test/lib/jdk/test/lib/util/ForceGC.java line 50:
> 48: for (int i = 0; i < 100; i++) {
> 49: System.gc();
> 50: System.out.println("doIt() iter: " + iter + ", gc " + i);
Maybe the trace should be printed only if `(i % 10 == 0) && (iter % 100 == 0)`
to avoid having too many traces in the log?
-
Marked as reviewed by dfuchs (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/8907
Re: RFR: 8287544: Replace uses of StringBuffer with StringBuilder in java.naming
On Sun, 29 May 2022 21:57:46 GMT, Andrey Turbanov wrote: > StringBuffer is a legacy synchronized class. StringBuilder is a direct > replacement to StringBuffer which generally have better performance. > There are some code that still uses StringBuffer in java.naming which could > be migrated to `StringBuilder`. Looks good to me - Marked as reviewed by aefimov (Committer). PR: https://git.openjdk.java.net/jdk/pull/8942
Re: RFR: 8287544: Replace uses of StringBuffer with StringBuilder in java.naming
On Sun, 29 May 2022 21:57:46 GMT, Andrey Turbanov wrote: > StringBuffer is a legacy synchronized class. StringBuilder is a direct > replacement to StringBuffer which generally have better performance. > There are some code that still uses StringBuffer in java.naming which could > be migrated to `StringBuilder`. Marked as reviewed by rriggs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8942
Re: RFR: 8282662: Use List.of() factory method to reduce memory consumption [v7]
On Tue, 31 May 2022 07:40:56 GMT, Сергей Цыпанов wrote: >> `List.of()` along with `Set.of()` create unmodifiable `List/Set` but with >> smaller footprint comparing to `Arrays.asList()` / `new HashSet()` when >> called with vararg of size 0, 1, 2. >> >> In general replacement of `Arrays.asList()` with `List.of()` is dubious as >> the latter is null-hostile, however in some cases we are sure that arguments >> are non-null. Into this PR I've included the following cases (in addition to >> those where the argument is proved to be non-null at compile-time): >> - `MethodHandles.longestParameterList()` never returns null >> - parameter types are never null >> - interfaces used for proxy construction and returned from >> `Class.getInterfaces()` are never null >> - exceptions types of method signature are never null > > Сергей Цыпанов 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: > > - 8282662: Revert wrong copyright year change > - 8282662: Revert ProxyGenerator > - 8282662: Revert ProxyGenerator > - 8282662: Revert dubious changes in MethodType > - 8282662: Revert dubious changes > - 8282662: Use List/Set.of() factory methods to save memory Marked as reviewed by rriggs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/7729
Re: RFR: 8287520: Shrink x86_32 problemlists after JDK-8287437
On Mon, 30 May 2022 13:20:17 GMT, Aleksey Shipilev wrote: > [JDK-8287137](https://bugs.openjdk.java.net/browse/JDK-8287137) added a bunch > of tests into problemlist. Those lists basically exclude every test that runs > with --enable-preview. > [JDK-8287437](https://bugs.openjdk.java.net/browse/JDK-8287437) makes it > better: it only disables Loom parts, even with --enable-preview. This allows > testing x86_32 on other preview features and avoids accidental bug slippage > in non-Loom code paths. We can and should shrink the problem lists now. > > Additional testing: > - [x] Removed tests with Linux x86_32 fastdebug; mostly pass, some non-Loom > failures > - [x] Linux x86_32 fastdebug `tier1` > - [ ] GHA run This looks okay. If we get PR8939 in then it should be possible to do more clear out of the exclude lists. - Marked as reviewed by alanb (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8946
Re: RFR: 8287496: Alternative virtual thread implementation that maps to OS thread
On Sun, 29 May 2022 14:46:39 GMT, Alan Bateman wrote: > This patch adds an alternative virtual thread implementation where each > virtual thread is backed by an OS thread. It doesn't scale but it can be used > by ports that don't have continuations support in the VM. Aside from > scalability, the lack of continuations support means: > > 1. JVM TI is not supported when running with --enable-preview (the JVM TI > spec allows for this) > 2. jshell --enable-preview can't be used (as jshell uses the debugger APIs > and so needs JVM TI) > > The VM option "VMContinuations" is added as an experimental option so it can > be used by tests. A number of tests are changed to re-run with > -XX:-VMContinuations. A new jtreg property is added so that tests that need > the underlying VM support to be present can use "@requires vm.continuations" > in the test description. A follow-up change would be to add "@requires > vm.continuations" to the ~70 serviceability/jvmti/vthread that run with > preview features enabled. > Since the package `jdk.internal.access` is > exported[1](#user-content-fn-1-97aea7d7960164849e591e42b91fb5c4) to the > `java.management` module, this can use `MethodHandle`s obtained using the > trusted lookup. That export is for another reason, and probably should be re-examined so that java.base doesn't need to export this package to java.management. In any case, we expect there will be compiler support soon to allow java.management be compiled with code that makes use of preview APIs in java.base. That will at least us reference Thread::isVirtual from code. - PR: https://git.openjdk.java.net/jdk/pull/8939
RFR: 8287496: Alternative virtual thread implementation that maps to OS thread
This patch adds an alternative virtual thread implementation where each virtual thread is backed by an OS thread. It doesn't scale but it can be used by ports that don't have continuations support in the VM. Aside from scalability, the lack of continuations support means: 1. JVM TI is not supported when running with --enable-preview (the JVM TI spec allows for this) 2. jshell --enable-preview can't be used (as jshell uses the debugger APIs and so needs JVM TI) The VM option "VMContinuations" is added as an experimental option so it can be used by tests. A number of tests are changed to re-run with -XX:-VMContinuations. A new jtreg property is added so that tests that need the underlying VM support to be present can use "@requires vm.continuations" in the test description. A follow-up change would be to add "@requires vm.continuations" to the ~70 serviceability/jvmti/vthread that run with preview features enabled. - Commit messages: - Continuation clinit should fail if no continuations support - Fix merge issue with test - Change VMContinuations to be experimental option, ensure JNI GetEnv returns EVERSION - Update - Expend test coverage - Merge - Initial commit Changes: https://git.openjdk.java.net/jdk/pull/8939/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8939=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8287496 Stats: 742 lines in 71 files changed: 570 ins; 53 del; 119 mod Patch: https://git.openjdk.java.net/jdk/pull/8939.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8939/head:pull/8939 PR: https://git.openjdk.java.net/jdk/pull/8939
Re: RFR: 8287496: Alternative virtual thread implementation that maps to OS thread
On Sun, 29 May 2022 14:46:39 GMT, Alan Bateman wrote:
> This patch adds an alternative virtual thread implementation where each
> virtual thread is backed by an OS thread. It doesn't scale but it can be used
> by ports that don't have continuations support in the VM. Aside from
> scalability, the lack of continuations support means:
>
> 1. JVM TI is not supported when running with --enable-preview (the JVM TI
> spec allows for this)
> 2. jshell --enable-preview can't be used (as jshell uses the debugger APIs
> and so needs JVM TI)
>
> The VM option "VMContinuations" is added as an experimental option so it can
> be used by tests. A number of tests are changed to re-run with
> -XX:-VMContinuations. A new jtreg property is added so that tests that need
> the underlying VM support to be present can use "@requires vm.continuations"
> in the test description. A follow-up change would be to add "@requires
> vm.continuations" to the ~70 serviceability/jvmti/vthread that run with
> preview features enabled.
Since the package `jdk.internal.access` is exported[^1] to the
`java.management` module, this can use `MethodHandle`s obtained using the
trusted lookup.
[^1]:
https://github.com/openjdk/jdk/blob/73ba7fdce838ba8a2c227a972c176311e6cc0b41/src/java.base/share/classes/module-info.java#L151-L154
src/java.management/share/classes/java/lang/management/ThreadInfo.java line 971:
> 969: throw new InternalError(e);
> 970: }
> 971: }
Suggestion:
private static boolean isVirtual(Thread thread) {
try {
return (boolean) IS_VIRTUAL.invokeExact(thread);
} catch (Error | RuntimeException e) {
throw e;
} catch (Throwable t) {
throw new InternalError(t);
}
}
private static final MethodHandle IS_VIRTUAL;
static {
final JavaLangInvokeAccess JLIA =
SharedSecrets.getJavaLangInvokeAccess();
try {
MethodHandle m = JLIA.findVirtual(Thread.class, "isVirtual",
MethodType.methodType(boolean.class));
assert m != null;
IS_VIRTUAL = m;
} catch (Exception e) {
throw new InternalError(e);
}
}
src/java.management/share/classes/sun/management/ThreadImpl.java line 661:
> 659:
> 660: private static final Method THREAD_IS_VIRTUAL = threadIsVirtual();
> 661: private static final Field THREADINFO_VIRTUAL = threadInfoVirtual();
Suggestion:
private static boolean isVirtual(Thread thread) {
try {
return (boolean) THREAD_IS_VIRTUAL.invokeExact(thread);
} catch (Error | RuntimeException e) {
throw e;
} catch (Throwable t) {
throw new InternalError(t);
}
}
/**
* Returns true if the given ThreadInfo is for a virutal thread.
*/
private static boolean isVirtual(ThreadInfo threadInfo) {
try {
return (boolean) THREADINFO_VIRTUAL.invokeExact(threadInfo);
} catch (Error | RuntimeException e) {
throw e;
} catch (Throwable t) {
throw new InternalError(t);
}
}
private static final JavaLangInvokeAccess JLIA =
SharedSecrets.getJavaLangInvokeAccess();
private static MethodHandle threadIsVirtual() {
try {
MethodHandle m = JLIA.findVirtual(Thread.class, "isVirtual",
MethodType.methodType(boolean.class));
assert m != null;
return m;
} catch (Exception e) {
throw new InternalError(e);
}
}
@SuppressWarnings("removal")
private static MethodHandle threadInfoVirtual() {
PrivilegedExceptionAction pa = () ->
ThreadInfo.class.getDeclaredField("virtual");
try {
return JLIA.unreflectField(AccessController.doPrivileged(pa),
false);
} catch (Exception e) {
throw new InternalError(e);
}
}
private static final MethodHandle THREAD_IS_VIRTUAL = threadIsVirtual();
private static final MethodHandle THREADINFO_VIRTUAL = threadInfoVirtual();
-
PR: https://git.openjdk.java.net/jdk/pull/8939
Re: RFR: 8287520: Shrink x86_32 problemlists after JDK-8287437
On Mon, 30 May 2022 13:20:17 GMT, Aleksey Shipilev wrote: > [JDK-8287137](https://bugs.openjdk.java.net/browse/JDK-8287137) added a bunch > of tests into problemlist. Those lists basically exclude every test that runs > with --enable-preview. > [JDK-8287437](https://bugs.openjdk.java.net/browse/JDK-8287437) makes it > better: it only disables Loom parts, even with --enable-preview. This allows > testing x86_32 on other preview features and avoids accidental bug slippage > in non-Loom code paths. We can and should shrink the problem lists now. > > Additional testing: > - [x] Removed tests with Linux x86_32 fastdebug; mostly pass, some non-Loom > failures > - [x] Linux x86_32 fastdebug `tier1` > - [ ] GHA run Open for reviews. The test failures on Windows x64 are known and are solved separately. - PR: https://git.openjdk.java.net/jdk/pull/8946
Re: RFR: 8282662: Use List.of() factory method to reduce memory consumption [v7]
On Tue, 31 May 2022 07:40:56 GMT, Сергей Цыпанов wrote: >> `List.of()` along with `Set.of()` create unmodifiable `List/Set` but with >> smaller footprint comparing to `Arrays.asList()` / `new HashSet()` when >> called with vararg of size 0, 1, 2. >> >> In general replacement of `Arrays.asList()` with `List.of()` is dubious as >> the latter is null-hostile, however in some cases we are sure that arguments >> are non-null. Into this PR I've included the following cases (in addition to >> those where the argument is proved to be non-null at compile-time): >> - `MethodHandles.longestParameterList()` never returns null >> - parameter types are never null >> - interfaces used for proxy construction and returned from >> `Class.getInterfaces()` are never null >> - exceptions types of method signature are never null > > Сергей Цыпанов has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains six commits: > > - 8282662: Revert wrong copyright year change > - 8282662: Revert ProxyGenerator > - 8282662: Revert ProxyGenerator > - 8282662: Revert dubious changes in MethodType > - 8282662: Revert dubious changes > - 8282662: Use List/Set.of() factory methods to save memory Marked as reviewed by redestad (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/7729
Re: RFR: 8286571: java source launcher from a minimal jdk image containing jdk.compiler fails with --enable-preview option [v3]
On Wed, 18 May 2022 06:30:34 GMT, Adam Sotona wrote: >> ### Problem description >> Minimal jdk image created by jlink with the only jdk.compiler module and its >> dependencies >> fails to run java source launcher correctly (for example when --source N is >> specified). >> Failing source launcher is only one the expressions of internal jdk.compiler >> malfunction, another example is failure to run javac with --release option. >> >> ### Root cause >> Module jdk.compiler requires zip filesystem (jdk.zipfs module) to parse >> ct.sym file and so to provide full functionality. >> Module jdk.zipfs is not included in the minimal JDK image, because module >> jdk.compiler does not declare it as "requires" in its module info. >> >> ### Alternative patch >> The problem can be alternatively resolved by complex code checks in >> jdk.compiler to provide user with valid error message, however that solution >> would be just a workaround for jdk.compiler dual functionality (with or >> without presence of jdk.zipfs module). Compiler functionality without access >> to ct.sym through jdk.zipfs is very limited. >> >> ### Proposed fix >> This patch fixes the problem by explicit declaration of jdk.compiler >> dependency on jdk.zipfs. >> Plus added specific test case. >> >> Please review the patch or raise objections against declaration of >> jdk.compiler dependent on jdk.zipfs. >> >> Thanks, >> Adam > > Adam Sotona has updated the pull request incrementally with one additional > commit since the last revision: > > corrected LimitedImage test description Followup issues to narrow the situation with optional module dependencies have been filled: - [8287559: Jlink should warn user about unsatisfied optional modules dependencies](https://bugs.openjdk.java.net/browse/JDK-8287559) - [8287560: jdk.compiler dependency on jdk.zipfs should be declared as optional](https://bugs.openjdk.java.net/browse/JDK-8287560) Please review if you agree with actual patch adding jdk.compiler dependency on jdk.zips. - PR: https://git.openjdk.java.net/jdk/pull/8747
RFR: 8236569: -Xss not multiple of 4K does not work for the main thread on macOS
This is continuation of PR #4256 The patch simply rounds up the specified stack size to multiple of the system page size, on systems where necessary. The patch is based on the original PR/branch, with reflected remaining recommendations. Please review. Thank you, Adam - Commit messages: - puting EINVAL on the RHS of the == as recommended - Update help text - Cast type - Change java -X output for -Xss - Only try to round-up when current value failed - Avoid overflow on page size - JDK-8236569: -Xss not multiple of 4K does not work for the main thread on macOS Changes: https://git.openjdk.java.net/jdk/pull/8953/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8953=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8236569 Stats: 41 lines in 3 files changed: 39 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/8953.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8953/head:pull/8953 PR: https://git.openjdk.java.net/jdk/pull/8953
Re: RFR: 8236569: -Xss not multiple of 4K does not work for the main thread on macOS [v6]
On Tue, 8 Jun 2021 16:53:38 GMT, Henry Jen wrote:
>> …d on macOS
>>
>> This patch simply round up the specified stack size to multiple of the
>> system page size.
>>
>> Test is trivial, simply run java with -Xss option against following code. On
>> MacOS, before the fix, running with `-Xss159k` and `-Xss160k` would get
>> `7183` and `649` respectively. After fix, both would output `649`, while
>> `-Xss161k` would be same as `-Xss164k` and see 691 as the output.
>>
>> ```code:java
>> public class StackLeak {
>> public int depth = 0;
>> public void stackLeak() {
>> depth++;
>> stackLeak();
>> }
>>
>> public static void main(String[] args) {
>> var test = new StackLeak();
>> try {
>> test.stackLeak();
>> } catch (Throwable e) {
>> System.out.println(test.depth);
>> }
>> }
>> }
>
> Henry Jen has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Update help text
Continuation in #8953
-
PR: https://git.openjdk.java.net/jdk/pull/4256
Re: RFR: 8287544: Replace uses of StringBuffer with StringBuilder in java.naming
On Sun, 29 May 2022 21:57:46 GMT, Andrey Turbanov wrote: > StringBuffer is a legacy synchronized class. StringBuilder is a direct > replacement to StringBuffer which generally have better performance. > There are some code that still uses StringBuffer in java.naming which could > be migrated to `StringBuilder`. _not a reviewer_ LGTM - PR: https://git.openjdk.java.net/jdk/pull/8942
Re: RFR: 8282662: Use List.of() factory method to reduce memory consumption [v7]
> `List.of()` along with `Set.of()` create unmodifiable `List/Set` but with > smaller footprint comparing to `Arrays.asList()` / `new HashSet()` when > called with vararg of size 0, 1, 2. > > In general replacement of `Arrays.asList()` with `List.of()` is dubious as > the latter is null-hostile, however in some cases we are sure that arguments > are non-null. Into this PR I've included the following cases (in addition to > those where the argument is proved to be non-null at compile-time): > - `MethodHandles.longestParameterList()` never returns null > - parameter types are never null > - interfaces used for proxy construction and returned from > `Class.getInterfaces()` are never null > - exceptions types of method signature are never null Сергей Цыпанов has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains six commits: - 8282662: Revert wrong copyright year change - 8282662: Revert ProxyGenerator - 8282662: Revert ProxyGenerator - 8282662: Revert dubious changes in MethodType - 8282662: Revert dubious changes - 8282662: Use List/Set.of() factory methods to save memory - Changes: https://git.openjdk.java.net/jdk/pull/7729/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=7729=06 Stats: 12 lines in 4 files changed: 1 ins; 2 del; 9 mod Patch: https://git.openjdk.java.net/jdk/pull/7729.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7729/head:pull/7729 PR: https://git.openjdk.java.net/jdk/pull/7729
RFR: 8287544: Replace uses of StringBuffer with StringBuilder in java.naming
StringBuffer is a legacy synchronized class. StringBuilder is a direct replacement to StringBuffer which generally have better performance. There are some code that still uses StringBuffer in java.naming which could be migrated to `StringBuilder`. - Commit messages: - [PATCH] Replace uses of StringBuffer with StringBuilder in java.naming Changes: https://git.openjdk.java.net/jdk/pull/8942/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8942=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8287544 Stats: 10 lines in 2 files changed: 0 ins; 0 del; 10 mod Patch: https://git.openjdk.java.net/jdk/pull/8942.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8942/head:pull/8942 PR: https://git.openjdk.java.net/jdk/pull/8942
Re: RFR: 8287497: Use String.contains() instead of String.indexOf() in java.naming
On Sun, 29 May 2022 14:00:20 GMT, Andrey Turbanov wrote: > `String.contains` was introduced in Java 5. > Some code in java.naming still uses old approach with `String.indexOf` to > check if String contains specified substring. > I propose to migrate such usages. Makes code shorter and easier to read. looks ok to me. - PR: https://git.openjdk.java.net/jdk/pull/8938
Integrated: 8287497: Use String.contains() instead of String.indexOf() in java.naming
On Sun, 29 May 2022 14:00:20 GMT, Andrey Turbanov wrote: > `String.contains` was introduced in Java 5. > Some code in java.naming still uses old approach with `String.indexOf` to > check if String contains specified substring. > I propose to migrate such usages. Makes code shorter and easier to read. This pull request has now been integrated. Changeset: 8a9aeff1 Author:Andrey Turbanov URL: https://git.openjdk.java.net/jdk/commit/8a9aeff18cd7b26f62934e5892fc87d25f249595 Stats: 13 lines in 4 files changed: 0 ins; 0 del; 13 mod 8287497: Use String.contains() instead of String.indexOf() in java.naming Reviewed-by: aefimov, rriggs, jpai - PR: https://git.openjdk.java.net/jdk/pull/8938
Re: RFR: 8287497: Use String.contains() instead of String.indexOf() in java.naming
On Sun, 29 May 2022 14:00:20 GMT, Andrey Turbanov wrote: > `String.contains` was introduced in Java 5. > Some code in java.naming still uses old approach with `String.indexOf` to > check if String contains specified substring. > I propose to migrate such usages. Makes code shorter and easier to read. Thank you for review! - PR: https://git.openjdk.java.net/jdk/pull/8938
