Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman wrote: >> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which >> JDK version to target. >> >> We will refresh this PR periodically to pick up changes and fixes from the >> loom repo. >> >> Most of the new mechanisms in the HotSpot VM are disabled by default and >> require running with `--enable-preview` to enable. >> >> The patch has support for x64 and aarch64 on the usual operating systems >> (Linux, macOS, and Windows). There are stubs (calling Unimplemented) for >> zero and some of the other ports. Additional ports can be contributed via >> PRs against the fibers branch in the loom repo. >> >> There are changes in many areas. To reduce notifications/mails, the labels >> have been trimmed down for now to hotspot, serviceability and core-libs. >> We'll add the complete set of labels when the PR is further along. >> >> The changes include a refresh of java.util.concurrent and ForkJoinPool from >> Doug Lea's CVS. These changes will probably be proposed and integrated in >> advance of this PR. >> >> The changes include some non-exposed and low-level infrastructure to support >> the (in draft) JEPs for Structured Concurrency and Scope Locals. This is to >> make life a bit easier and avoid having to separate VM changes and juggle >> branches at this time. > > Alan Bateman has updated the pull request incrementally with one additional > commit since the last revision: > > Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e Reviewed java.io, java.lang, java.lang.ref. - Marked as reviewed by smarks (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8285745: Re-examine PushbackInputStream mark/reset
On Wed, 27 Apr 2022 20:10:03 GMT, Brian Burkhalter wrote: > Please review this request to remove the `synchronized` keyword from the > `mark(int)` and `reset()` methods of `java.io.PushbackInputStream`. Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8433
Re: RFR: 8285097: Duplicate XML keys in XPATHErrorResources.java and XSLTErrorResources.java
On Wed, 20 Apr 2022 15:37:13 GMT, Shruthi wrote: > Removing the Duplicate keys present in XSLTErrorResources.java and > XPATHErrorResources.java > > The bug report for the same: https://bugs.openjdk.java.net/browse/JDK-8285097 src/java.xml/share/classes/com/sun/org/apache/xpath/internal/res/XPATHErrorResources.java line 598: > 596:"rtf() not supported by XRTreeFragSelectWrapper"}, > 597: > 598: { ER_ASNODEITERATOR_NOT_SUPPORTED_XRTREEFRAGSELECTWRAPPER, Please replace the usage in XRTreeFragSelectWrapper, and also update the LastModified tag. - PR: https://git.openjdk.java.net/jdk/pull/8318
Re: RFR: 8285745: Re-examine PushbackInputStream mark/reset
On Wed, 27 Apr 2022 20:10:03 GMT, Brian Burkhalter wrote: > Please review this request to remove the `synchronized` keyword from the > `mark(int)` and `reset()` methods of `java.io.PushbackInputStream`. Looks fine to me. Given that `mark()` currently is a no-op and `reset()` throws an exception, the `synchronized` wouldn't be doing anything when it comes to calls on an exact instance of `PushbackInputStream`. Calls on subclasses of `PushbackInputStream` which did support mark/reset, would be overriding this method already and handling any synchronization themselves. - Marked as reviewed by jpai (Committer). PR: https://git.openjdk.java.net/jdk/pull/8433
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 19:00:52 GMT, Coleen Phillimore wrote: >> Alan Bateman has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e > > src/hotspot/share/prims/jvmtiEnvBase.cpp line 2388: > >> 2386: } >> 2387: >> 2388: void > > @sspitsyn We should clean up this aberrant style of putting the return type > on the line before the rest of the function declaration after this > integration. It looks so strange. I noticed that it's pre-existing for > other functions in this file. I'm okay to follow any style. We can do it after the integration. - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: JDK-8285676: Add missing @param tags for type parameters on classes and interfaces [v3]
On Wed, 27 Apr 2022 23:24:57 GMT, Stuart Marks wrote: >> I said "keys maintained", omitting "by this map" to finesse the question of >> if the SimpleEntry class *is* a map, or is used to implement a map, etc. I >> can change it to include "by this map" if the map/entry distinction is okay >> to be blurred. > > Whoops, sorry, this is `SimpleEntry`, which is _not_ a `Map`. In this case > I'd follow `Map.Entry` which says "the type of the key" and "the type of the > map". Will change to the Map.Entry wording "the type of key" and "the type of the value", respectively. - PR: https://git.openjdk.java.net/jdk/pull/8410
Re: RFR: JDK-8285676: Add missing @param tags for type parameters on classes and interfaces [v3]
> To enable more complete doclint checking (courtesy @jonathan-gibbons), please > review this PR to add type-level @param tags where they are missing. > > To the maintainers of java.util.concurrent, those changes could be separated > out in another bug if that would ease maintenance of that code. > > Making these library fixes is a blocker for correcting and expanding the > doclint checks (JDK-8285496). > > I'll update copyright years before pushing. Joe Darcy has updated the pull request incrementally with one additional commit since the last revision: Respond to more review feedback. - Changes: - all: https://git.openjdk.java.net/jdk/pull/8410/files - new: https://git.openjdk.java.net/jdk/pull/8410/files/e0ac5dcb..db4919a9 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8410=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8410=01-02 Stats: 4 lines in 1 file changed: 0 ins; 0 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/8410.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8410/head:pull/8410 PR: https://git.openjdk.java.net/jdk/pull/8410
Re: RFR: JDK-8285676: Add missing @param tags for type parameters on classes and interfaces [v3]
On Thu, 28 Apr 2022 01:31:13 GMT, Joe Darcy wrote: >> To enable more complete doclint checking (courtesy @jonathan-gibbons), >> please review this PR to add type-level @param tags where they are missing. >> >> To the maintainers of java.util.concurrent, those changes could be separated >> out in another bug if that would ease maintenance of that code. >> >> Making these library fixes is a blocker for correcting and expanding the >> doclint checks (JDK-8285496). >> >> I'll update copyright years before pushing. > > Joe Darcy has updated the pull request incrementally with one additional > commit since the last revision: > > Respond to more review feedback. Marked as reviewed by prr (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8410
Re: RFR: 8284992: Fix misleading Vector API doc for LSHR operator [v3]
On Thu, 28 Apr 2022 00:08:41 GMT, Paul Sandoz wrote: > I created one, filled it in, and assigned it to you (for other examples you > can search in the issue tracker, this one quite is simple so i thought it was > quicker to do myself to show you). For any specification change we need to > review and track that change (independent of any implementation changes, if > any). > > If you are ok with it I can add myself as reviewer, then you can "Finalize" > it (see button on same line as "Edit"), triggering a review request, from > which we may receive comments to address, and once addressed and final, it > will unblock the PR for integration. Thanks @PaulSandoz for your help. Yes, I think it's good enough. I made a small change which just adding a `(`. https://user-images.githubusercontent.com/19923746/165657177-f44f7f7d-44da-4921-a98c-5f6b9a3d9e36.png;> Thanks. - PR: https://git.openjdk.java.net/jdk/pull/8291
Re: RFR: 8283667: [vectorapi] Vectorization for masked load with IOOBE with predicate feature [v2]
On Fri, 22 Apr 2022 07:08:24 GMT, Xiaohong Gong wrote: >> Currently the vector load with mask when the given index happens out of the >> array boundary is implemented with pure java scalar code to avoid the IOOBE >> (IndexOutOfBoundaryException). This is necessary for architectures that do >> not support the predicate feature. Because the masked load is implemented >> with a full vector load and a vector blend applied on it. And a full vector >> load will definitely cause the IOOBE which is not valid. However, for >> architectures that support the predicate feature like SVE/AVX-512/RVV, it >> can be vectorized with the predicated load instruction as long as the >> indexes of the masked lanes are within the bounds of the array. For these >> architectures, loading with unmasked lanes does not raise exception. >> >> This patch adds the vectorization support for the masked load with IOOBE >> part. Please see the original java implementation (FIXME: optimize): >> >> >> @ForceInline >> public static >> ByteVector fromArray(VectorSpecies species, >>byte[] a, int offset, >>VectorMask m) { >> ByteSpecies vsp = (ByteSpecies) species; >> if (offset >= 0 && offset <= (a.length - species.length())) { >> return vsp.dummyVector().fromArray0(a, offset, m); >> } >> >> // FIXME: optimize >> checkMaskFromIndexSize(offset, vsp, m, 1, a.length); >> return vsp.vOp(m, i -> a[offset + i]); >> } >> >> Since it can only be vectorized with the predicate load, the hotspot must >> check whether the current backend supports it and falls back to the java >> scalar version if not. This is different from the normal masked vector load >> that the compiler will generate a full vector load and a vector blend if the >> predicate load is not supported. So to let the compiler make the expected >> action, an additional flag (i.e. `usePred`) is added to the existing >> "loadMasked" intrinsic, with the value "true" for the IOOBE part while >> "false" for the normal load. And the compiler will fail to intrinsify if the >> flag is "true" and the predicate load is not supported by the backend, which >> means that normal java path will be executed. >> >> Also adds the same vectorization support for masked: >> - fromByteArray/fromByteBuffer >> - fromBooleanArray >> - fromCharArray >> >> The performance for the new added benchmarks improve about `1.88x ~ 30.26x` >> on the x86 AVX-512 system: >> >> Benchmark before After Units >> LoadMaskedIOOBEBenchmark.byteLoadArrayMaskIOOBE 737.542 1387.069 ops/ms >> LoadMaskedIOOBEBenchmark.doubleLoadArrayMaskIOOBE 118.366 330.776 ops/ms >> LoadMaskedIOOBEBenchmark.floatLoadArrayMaskIOOBE 233.832 6125.026 ops/ms >> LoadMaskedIOOBEBenchmark.intLoadArrayMaskIOOBE233.816 7075.923 ops/ms >> LoadMaskedIOOBEBenchmark.longLoadArrayMaskIOOBE 119.771 330.587 ops/ms >> LoadMaskedIOOBEBenchmark.shortLoadArrayMaskIOOBE 431.961 939.301 ops/ms >> >> Similar performance gain can also be observed on 512-bit SVE system. > > Xiaohong Gong has updated the pull request incrementally with one additional > commit since the last revision: > > Rename the "usePred" to "offsetInRange" Rest of the patch looks good to me. src/hotspot/share/opto/vectorIntrinsics.cpp line 1232: > 1230: // out when current case uses the predicate feature. > 1231: if (!supports_predicate) { > 1232: bool use_predicate = false; If we rename this to needs_predicate it will be easier to understand. - PR: https://git.openjdk.java.net/jdk/pull/8035
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman wrote: >> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which >> JDK version to target. >> >> We will refresh this PR periodically to pick up changes and fixes from the >> loom repo. >> >> Most of the new mechanisms in the HotSpot VM are disabled by default and >> require running with `--enable-preview` to enable. >> >> The patch has support for x64 and aarch64 on the usual operating systems >> (Linux, macOS, and Windows). There are stubs (calling Unimplemented) for >> zero and some of the other ports. Additional ports can be contributed via >> PRs against the fibers branch in the loom repo. >> >> There are changes in many areas. To reduce notifications/mails, the labels >> have been trimmed down for now to hotspot, serviceability and core-libs. >> We'll add the complete set of labels when the PR is further along. >> >> The changes include a refresh of java.util.concurrent and ForkJoinPool from >> Doug Lea's CVS. These changes will probably be proposed and integrated in >> advance of this PR. >> >> The changes include some non-exposed and low-level infrastructure to support >> the (in draft) JEPs for Structured Concurrency and Scope Locals. This is to >> make life a bit easier and avoid having to separate VM changes and juggle >> branches at this time. > > Alan Bateman has updated the pull request incrementally with one additional > commit since the last revision: > > Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e I've reviewed the new files in Hotspot runtime and the cpu directories, as well as the changes to existing files in runtime, oops, utilities, interpreter and classfile. I'm happy to approve this PR. Thank you to everyone for their hard work in reviewing this code, and well done to @pron and @AlanBateman and all the loom team for bringing us this significant and exciting new feature for Java! src/hotspot/share/prims/jvmtiEnvBase.cpp line 2388: > 2386: } > 2387: > 2388: void @sspitsyn We should clean up this aberrant style of putting the return type on the line before the rest of the function declaration after this integration. It looks so strange. I noticed that it's pre-existing for other functions in this file. - Marked as reviewed by coleenp (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284992: Fix misleading Vector API doc for LSHR operator [v3]
On Wed, 27 Apr 2022 09:06:12 GMT, Jie Fu wrote: >> Hi all, >> >> The Current Vector API doc for `LSHR` is >> >> Produce a>>>(n&(ESIZE*8-1)). Integral only. >> >> >> This is misleading which may lead to bugs for Java developers. >> This is because for negative byte/short elements, the results computed by >> `LSHR` will be different from that of `>>>`. >> For more details, please see >> https://github.com/openjdk/jdk/pull/8276#issue-1206391831 . >> >> After the patch, the doc for `LSHR` is >> >> Produce zero-extended right shift of a by (n&(ESIZE*8-1)) bits. Integral >> only. >> >> >> Thanks. >> Best regards, >> Jie > > Jie Fu 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: > > - Address review comments > - Merge branch 'master' into JDK-8284992 > - Merge branch 'master' into JDK-8284992 > - Address review comments > - Merge branch 'master' into JDK-8284992 > - 8284992: Fix misleading Vector API doc for LSHR operator I created one, filled it in, and assigned it to you (for other examples you can search in the issue tracker, this one quite is simple so i thought it was quicker to do myself to show you). For any specification change we need to review and track that change (independent of any implementation changes, if any). If you are ok with it I can add myself as reviewer, then you can "Finalize" it (see button on same line as "Edit"), triggering a review request, from which we may receive comments to address, and once addressed and final, it will unblock the PR for integration. - PR: https://git.openjdk.java.net/jdk/pull/8291
Re: RFR: JDK-8285676: Add missing @param tags for type parameters on classes and interfaces [v2]
On Wed, 27 Apr 2022 23:04:47 GMT, Joe Darcy wrote: >> src/java.base/share/classes/java/util/AbstractMap.java line 601: >> >>> 599: * {@code Map.entrySet().toArray}. >>> 600: * >>> 601: * @param the type of keys maintained >> >> Please update to match java.util.Map, which says "the type of keys >> maintained by this map" > > I said "keys maintained", omitting "by this map" to finesse the question of > if the SimpleEntry class *is* a map, or is used to implement a map, etc. I > can change it to include "by this map" if the map/entry distinction is okay > to be blurred. Whoops, sorry, this is `SimpleEntry`, which is _not_ a `Map`. In this case I'd follow `Map.Entry` which says "the type of the key" and "the type of the map". >> src/java.base/share/classes/java/util/Dictionary.java line 44: >> >>> 42: * @param the type of keys >>> 43: * @param the type of mapped values >>> 44: * >> >> Urgh. This class is obsolete, but it was retrofitted to implement Map and >> was subsequently generified, so I'd update these to match java.util.Map. > > The javadoc of Dictionary states "The Dictionary class [...] maps keys to > values." which was my guide for the wording, but I don't mind changing it. My bad, `Dictionary` was not retrofitted to implement `Map` but it gained `K` and `V` type parameters to align with `Map`. No need to change this; it doesn't really matter. - PR: https://git.openjdk.java.net/jdk/pull/8410
Re: RFR: 8272352: Java launcher can not parse Chinese character when system locale is set to UTF-8
On Wed, 27 Apr 2022 23:12:59 GMT, Jie Fu wrote: > Is it possible to write a jtreg test for this fix? Thanks. I am happy to write a test if possible, but AFAIK it requires configuring Windows settings (and reboot), so I don't think jtreg is capable of doing so. Thus I added `noreg-hard` to the JBS issue. - PR: https://git.openjdk.java.net/jdk/pull/8434
Re: RFR: JDK-8285676: Add missing @param tags for type parameters on classes and interfaces [v2]
On Wed, 27 Apr 2022 10:55:22 GMT, Daniel Fuchs 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 three additional commits >> since the last revision: >> >> - Respond to review feedback. >> - Merge branch 'master' into JDK-8285676 >> - JDK-8285676: Add missing @param tags for type parameters on classes and >> interfaces > > src/java.management/share/classes/javax/management/openmbean/SimpleType.java > line 60: > >> (failed to retrieve contents of file, check the PR for context) > I would suggest to say "that values described by this type"... Will change to "the Java type that values described by this SimpleType must have." - PR: https://git.openjdk.java.net/jdk/pull/8410
Re: RFR: JDK-8285676: Add missing @param tags for type parameters on classes and interfaces [v2]
> To enable more complete doclint checking (courtesy @jonathan-gibbons), please > review this PR to add type-level @param tags where they are missing. > > To the maintainers of java.util.concurrent, those changes could be separated > out in another bug if that would ease maintenance of that code. > > Making these library fixes is a blocker for correcting and expanding the > doclint checks (JDK-8285496). > > I'll update copyright years before pushing. 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 three additional commits since the last revision: - Respond to review feedback. - Merge branch 'master' into JDK-8285676 - JDK-8285676: Add missing @param tags for type parameters on classes and interfaces - Changes: - all: https://git.openjdk.java.net/jdk/pull/8410/files - new: https://git.openjdk.java.net/jdk/pull/8410/files/fe47dd2f..e0ac5dcb Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8410=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8410=00-01 Stats: 5958 lines in 128 files changed: 4827 ins; 485 del; 646 mod Patch: https://git.openjdk.java.net/jdk/pull/8410.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8410/head:pull/8410 PR: https://git.openjdk.java.net/jdk/pull/8410
Re: RFR: 8272352: Java launcher can not parse Chinese character when system locale is set to UTF-8
On Wed, 27 Apr 2022 20:23:32 GMT, Naoto Sato wrote: > Java runtime has been detecting the Windows system locale encoding using > `GetLocaleInfo(GetSystemDefaultLCID(), LOCALE_IDEFAULTANSICODEPAGE, ...)`, > but it returns the *legacy* ANSI code page value, e.g, 1252 for US-English. > In order to detect whether the user has selected `UTF-8` as the default, the > code page has to be queried with `GetACP()`. > Also, the case if the call to `GetLocaleInfo` fails changed to fall back to > `UTF-8` instead of `Cp1252`. Is it possible to write a jtreg test for this fix? Thanks. - PR: https://git.openjdk.java.net/jdk/pull/8434
Re: RFR: JDK-8285676: Add missing @param tags for type parameters on classes and interfaces
On Wed, 27 Apr 2022 10:54:00 GMT, Daniel Fuchs wrote: >> To enable more complete doclint checking (courtesy @jonathan-gibbons), >> please review this PR to add type-level @param tags where they are missing. >> >> To the maintainers of java.util.concurrent, those changes could be separated >> out in another bug if that would ease maintenance of that code. >> >> Making these library fixes is a blocker for correcting and expanding the >> doclint checks (JDK-8285496). >> >> I'll update copyright years before pushing. > > src/java.management/share/classes/javax/management/openmbean/ArrayType.java > line 96: > >> 94: * >> 95: * @param the Java type that instances described by this type must >> 96: * have. > > Instead of "instances" - would it be more correct to say "array elements"? Will change to "the Java component type that arrays described by ArrayType must have" - PR: https://git.openjdk.java.net/jdk/pull/8410
Re: RFR: 8285097: Duplicate XML keys in XPATHErrorResources.java and XSLTErrorResources.java
On Wed, 20 Apr 2022 15:37:13 GMT, Shruthi wrote: > Removing the Duplicate keys present in XSLTErrorResources.java and > XPATHErrorResources.java > > The bug report for the same: https://bugs.openjdk.java.net/browse/JDK-8285097 This contribution is on behalf of my employer, IBM, which is a corporate OCA signatory. `/covered` - PR: https://git.openjdk.java.net/jdk/pull/8318
Re: RFR: 8285097: Duplicate XML keys in XPATHErrorResources.java and XSLTErrorResources.java
On Wed, 20 Apr 2022 15:37:13 GMT, Shruthi wrote: > Removing the Duplicate keys present in XSLTErrorResources.java and > XPATHErrorResources.java > > The bug report for the same: https://bugs.openjdk.java.net/browse/JDK-8285097 @shruacha1234, a few notes specific to openJDK PRs: - Please change the issue name to: "8285097: Duplicate XML keys in XPATHErrorResources.java and XSLTErrorResources.java" this will correctly link the issue in bugs.openjdk.net - Please enable github actions. This allows the pre-tests to run. - PR: https://git.openjdk.java.net/jdk/pull/8318
RFR: 8285097: Duplicate XML keys in XPATHErrorResources.java and XSLTErrorResources.java
Removing the Duplicate keys present in XSLTErrorResources.java and XPATHErrorResources.java The bug report for the same: https://bugs.openjdk.java.net/browse/JDK-8285097 - Commit messages: - Removing Duplicate keys present in XSLTErrorResources.java and XPATHErrorResources.java Changes: https://git.openjdk.java.net/jdk/pull/8318/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8318=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8285097 Stats: 7 lines in 2 files changed: 0 ins; 6 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/8318.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8318/head:pull/8318 PR: https://git.openjdk.java.net/jdk/pull/8318
Re: RFR: JDK-8285676: Add missing @param tags for type parameters on classes and interfaces
On Wed, 27 Apr 2022 01:39:27 GMT, Stuart Marks wrote: >> To enable more complete doclint checking (courtesy @jonathan-gibbons), >> please review this PR to add type-level @param tags where they are missing. >> >> To the maintainers of java.util.concurrent, those changes could be separated >> out in another bug if that would ease maintenance of that code. >> >> Making these library fixes is a blocker for correcting and expanding the >> doclint checks (JDK-8285496). >> >> I'll update copyright years before pushing. > > src/java.base/share/classes/java/util/AbstractMap.java line 601: > >> 599: * {@code Map.entrySet().toArray}. >> 600: * >> 601: * @param the type of keys maintained > > Please update to match java.util.Map, which says "the type of keys maintained > by this map" I said "keys maintained", omitting "by this map" to finesse the question of if the SimpleEntry class *is* a map, or is used to implement a map, etc. I can change it to include "by this map" if the map/entry distinction is okay to be blurred. > src/java.base/share/classes/java/util/Dictionary.java line 44: > >> 42: * @param the type of keys >> 43: * @param the type of mapped values >> 44: * > > Urgh. This class is obsolete, but it was retrofitted to implement Map and was > subsequently generified, so I'd update these to match java.util.Map. The javadoc of Dictionary states "The Dictionary class [...] maps keys to values." which was my guide for the wording, but I don't mind changing it. - PR: https://git.openjdk.java.net/jdk/pull/8410
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman wrote: >> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which >> JDK version to target. >> >> We will refresh this PR periodically to pick up changes and fixes from the >> loom repo. >> >> Most of the new mechanisms in the HotSpot VM are disabled by default and >> require running with `--enable-preview` to enable. >> >> The patch has support for x64 and aarch64 on the usual operating systems >> (Linux, macOS, and Windows). There are stubs (calling Unimplemented) for >> zero and some of the other ports. Additional ports can be contributed via >> PRs against the fibers branch in the loom repo. >> >> There are changes in many areas. To reduce notifications/mails, the labels >> have been trimmed down for now to hotspot, serviceability and core-libs. >> We'll add the complete set of labels when the PR is further along. >> >> The changes include a refresh of java.util.concurrent and ForkJoinPool from >> Doug Lea's CVS. These changes will probably be proposed and integrated in >> advance of this PR. >> >> The changes include some non-exposed and low-level infrastructure to support >> the (in draft) JEPs for Structured Concurrency and Scope Locals. This is to >> make life a bit easier and avoid having to separate VM changes and juggle >> branches at this time. > > Alan Bateman has updated the pull request incrementally with one additional > commit since the last revision: > > Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e The changes to the `java.io` package and related files in `libjava`, and the changes to the non-networking parts of the `java.nio.channels`, `sun.nio.ch`, and `sun.nio.fs` packages and related files in `libnio` all look fine to me. - Marked as reviewed by bpb (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284992: Fix misleading Vector API doc for LSHR operator [v3]
On Wed, 27 Apr 2022 17:17:55 GMT, Paul Sandoz wrote: > Thanks, looks good, we will need to create a CSR. Have you done that before? No, and I don't know much about a CSR. Is there any example for a doc fix CSR to follow? Thanks. - PR: https://git.openjdk.java.net/jdk/pull/8291
RFR: JDK-8285764: Add system property for Java SE specification maintenance version
Add a new system property, java.specification.maintenance.version, to return the maintenance release number of the Java SE specification being implemented. The property is unset, optional in the terminology of System.getProperties, for an initial release of a specification. Please also review the CSR https://bugs.openjdk.java.net/browse/JDK-8285764 I'll update copyright years before an integration. - Commit messages: - Update comment in template. - JDK-8285497: Add system property for Java SE specification maintenance version Changes: https://git.openjdk.java.net/jdk/pull/8437/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8437=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8285764 Stats: 7 lines in 2 files changed: 7 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/8437.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8437/head:pull/8437 PR: https://git.openjdk.java.net/jdk/pull/8437
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman wrote: >> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which >> JDK version to target. >> >> We will refresh this PR periodically to pick up changes and fixes from the >> loom repo. >> >> Most of the new mechanisms in the HotSpot VM are disabled by default and >> require running with `--enable-preview` to enable. >> >> The patch has support for x64 and aarch64 on the usual operating systems >> (Linux, macOS, and Windows). There are stubs (calling Unimplemented) for >> zero and some of the other ports. Additional ports can be contributed via >> PRs against the fibers branch in the loom repo. >> >> There are changes in many areas. To reduce notifications/mails, the labels >> have been trimmed down for now to hotspot, serviceability and core-libs. >> We'll add the complete set of labels when the PR is further along. >> >> The changes include a refresh of java.util.concurrent and ForkJoinPool from >> Doug Lea's CVS. These changes will probably be proposed and integrated in >> advance of this PR. >> >> The changes include some non-exposed and low-level infrastructure to support >> the (in draft) JEPs for Structured Concurrency and Scope Locals. This is to >> make life a bit easier and avoid having to separate VM changes and juggle >> branches at this time. > > Alan Bateman has updated the pull request incrementally with one additional > commit since the last revision: > > Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e It's great to see end game in play for this multi-year effort, impressive work by all involved. I looked through code in the base module. Generally looks well structured and documented. The fork join code is naturally hard to review. I did try! (it takes the prize for the highest Java code density in the JDK). I mostly looked for regular and repeated structure rather than trying to fully understand/review the intricate concurrency details. IMO there is some post integration work to do around the architecture of thread containers, i presume the structured concurrency implementation (use of thread flock) will help with that. - Marked as reviewed by psandoz (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman wrote: >> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which >> JDK version to target. >> >> We will refresh this PR periodically to pick up changes and fixes from the >> loom repo. >> >> Most of the new mechanisms in the HotSpot VM are disabled by default and >> require running with `--enable-preview` to enable. >> >> The patch has support for x64 and aarch64 on the usual operating systems >> (Linux, macOS, and Windows). There are stubs (calling Unimplemented) for >> zero and some of the other ports. Additional ports can be contributed via >> PRs against the fibers branch in the loom repo. >> >> There are changes in many areas. To reduce notifications/mails, the labels >> have been trimmed down for now to hotspot, serviceability and core-libs. >> We'll add the complete set of labels when the PR is further along. >> >> The changes include a refresh of java.util.concurrent and ForkJoinPool from >> Doug Lea's CVS. These changes will probably be proposed and integrated in >> advance of this PR. >> >> The changes include some non-exposed and low-level infrastructure to support >> the (in draft) JEPs for Structured Concurrency and Scope Locals. This is to >> make life a bit easier and avoid having to separate VM changes and juggle >> branches at this time. > > Alan Bateman has updated the pull request incrementally with one additional > commit since the last revision: > > Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e I reviewed most of the source files in `java.base` except `java.util.concurrent`. I also reviewed `java.logging`, `java.management` and `jdk.management` modules. The changes are easy to follow and clean. Looks good to me. - Marked as reviewed by mchung (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: JDK-8285730: unify _WIN32_WINNT settings
On Wed, 27 Apr 2022 14:57:41 GMT, Matthias Baesken wrote: > Currently we set _WIN32_WINNT at various places in the codebase; this is used > to target a minimum Windows version we want to support. See also for more > detailled information : > https://docs.microsoft.com/en-us/windows/win32/winprog/using-the-windows-headers?redirectedfrom=MSDN#setting-winver-or-_win32_winnt > Macros for Conditional Declarations > "Certain functions that depend on a particular version of Windows are > declared using conditional code. This enables you to use the compiler to > detect whether your application uses functions that are not supported on its > target version(s) of Windows." > > However currently we have quite a lot of differing settings of _WIN32_WINNT > in the codebase ; setting _WIN32_WINNT to 0x0601 (Windows 7) where possible > would make sense because we have this setting already at java_props_md.c > (so targeting older Windows versions at other places is most likely not > useful). There is obviously a lot of history here and different parts of the codebase had hit different minimum OS version requirements at different times. While at one time we would have had to cater for building on systems with and without a given API those days are long gone for the code being touched here (AFAICS). As Erik states we should be able to set a minimum _WIN32_WINNT_ value needed to build all the codebase and simply have that checked and set at configure time. The code itself would not need to define _WIN32_WINNT. - PR: https://git.openjdk.java.net/jdk/pull/8428
Re: RFR: 8284640: CollectorImpl class could be a record class
On Mon, 11 Apr 2022 13:08:43 GMT, altrisi wrote: > Changes the definition of `CollectorImpl` to be a record. src/java.base/share/classes/java/util/stream/Collectors.java line 197: > 195: * @param the type of the result > 196: */ > 197: static record CollectorImpl(Supplier supplier, Suggestion: record CollectorImpl(Supplier supplier, [Nested records are implicitly static.](https://docs.oracle.com/javase/specs/jls/se18/html/jls-8.html#jls-8.10-220) - PR: https://git.openjdk.java.net/jdk/pull/8179
RFR: 8284640: CollectorImpl class could be a record class
Changes the definition of `CollectorImpl` to be a record. - Commit messages: - Remove explicit `static` modifier - 8284640: CollectorImpl class could be a record class Changes: https://git.openjdk.java.net/jdk/pull/8179/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8179=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8284640 Stats: 43 lines in 1 file changed: 0 ins; 37 del; 6 mod Patch: https://git.openjdk.java.net/jdk/pull/8179.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8179/head:pull/8179 PR: https://git.openjdk.java.net/jdk/pull/8179
RFR: 8272352: Java launcher can not parse Chinese character when system locale is set to UTF-8
Java runtime has been detecting the Windows system locale encoding using `GetLocaleInfo(GetSystemDefaultLCID(), LOCALE_IDEFAULTANSICODEPAGE, ...)`, but it returns the *legacy* ANSI code page value, e.g, 1252 for US-English. In order to detect whether the user has selected `UTF-8` as the default, the code page has to be queried with `GetACP()`. Also, the case if the call to `GetLocaleInfo` fails changed to fall back to `UTF-8` instead of `Cp1252`. - Commit messages: - 8272352: Java launcher can not parse Chinese character when system locale is set to UTF-8 Changes: https://git.openjdk.java.net/jdk/pull/8434/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8434=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8272352 Stats: 13 lines in 1 file changed: 3 ins; 4 del; 6 mod Patch: https://git.openjdk.java.net/jdk/pull/8434.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8434/head:pull/8434 PR: https://git.openjdk.java.net/jdk/pull/8434
Re: RFR: 8285745: Re-examine PushbackInputStream mark/reset
On Wed, 27 Apr 2022 20:10:03 GMT, Brian Burkhalter wrote: > Please review this request to remove the `synchronized` keyword from the > `mark(int)` and `reset()` methods of `java.io.PushbackInputStream`. Please see also https://github.com/openjdk/jdk/pull/8309. - PR: https://git.openjdk.java.net/jdk/pull/8433
RFR: 8285745: Re-examine PushbackInputStream mark/reset
Please review this request to remove the `synchronized` keyword from the `mark(int)` and `reset()` methods of `java.io.PushbackInputStream`. - Commit messages: - 8285745: Re-examine PushbackInputStream mark/reset Changes: https://git.openjdk.java.net/jdk/pull/8433/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8433=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8285745 Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/8433.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8433/head:pull/8433 PR: https://git.openjdk.java.net/jdk/pull/8433
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman wrote: >> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which >> JDK version to target. >> >> We will refresh this PR periodically to pick up changes and fixes from the >> loom repo. >> >> Most of the new mechanisms in the HotSpot VM are disabled by default and >> require running with `--enable-preview` to enable. >> >> The patch has support for x64 and aarch64 on the usual operating systems >> (Linux, macOS, and Windows). There are stubs (calling Unimplemented) for >> zero and some of the other ports. Additional ports can be contributed via >> PRs against the fibers branch in the loom repo. >> >> There are changes in many areas. To reduce notifications/mails, the labels >> have been trimmed down for now to hotspot, serviceability and core-libs. >> We'll add the complete set of labels when the PR is further along. >> >> The changes include a refresh of java.util.concurrent and ForkJoinPool from >> Doug Lea's CVS. These changes will probably be proposed and integrated in >> advance of this PR. >> >> The changes include some non-exposed and low-level infrastructure to support >> the (in draft) JEPs for Structured Concurrency and Scope Locals. This is to >> make life a bit easier and avoid having to separate VM changes and juggle >> branches at this time. > > Alan Bateman has updated the pull request incrementally with one additional > commit since the last revision: > > Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e src/java.base/share/classes/java/lang/Thread.java line 116: > 114: * carrier threads. Locking and I/O operations are examples of > operations > 115: * where a carrier thread may be re-scheduled from one virtual thread to > another. > 116: * Code executing in a virtual thread is not aware of underlying carrier > thread. Suggestion: * Code executing in a virtual thread is not aware of the underlying carrier thread. - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman wrote: >> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which >> JDK version to target. >> >> We will refresh this PR periodically to pick up changes and fixes from the >> loom repo. >> >> Most of the new mechanisms in the HotSpot VM are disabled by default and >> require running with `--enable-preview` to enable. >> >> The patch has support for x64 and aarch64 on the usual operating systems >> (Linux, macOS, and Windows). There are stubs (calling Unimplemented) for >> zero and some of the other ports. Additional ports can be contributed via >> PRs against the fibers branch in the loom repo. >> >> There are changes in many areas. To reduce notifications/mails, the labels >> have been trimmed down for now to hotspot, serviceability and core-libs. >> We'll add the complete set of labels when the PR is further along. >> >> The changes include a refresh of java.util.concurrent and ForkJoinPool from >> Doug Lea's CVS. These changes will probably be proposed and integrated in >> advance of this PR. >> >> The changes include some non-exposed and low-level infrastructure to support >> the (in draft) JEPs for Structured Concurrency and Scope Locals. This is to >> make life a bit easier and avoid having to separate VM changes and juggle >> branches at this time. > > Alan Bateman has updated the pull request incrementally with one additional > commit since the last revision: > > Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e src/java.base/share/classes/java/util/concurrent/ThreadPerTaskExecutor.java line 45: > 43: * threads is unbounded. > 44: */ > 45: class ThreadPerTaskExecutor implements ExecutorService { This class manages the set of per-task threads which arguably should be the job of the thread container, and it awkwardly overrides the container's set of threads by setting a field on `SharedThreadContainer.threadsSupplier`. `SharedThreadContainer` supports a number of different shared container policies that could be made clearer. Architecturally i think this could be better layered but it can be iterated on later. - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 18:16:54 GMT, Mandy Chung wrote: >> I was wondering that too, but held off commenting since it's not super >> performance critical given `classToHashes` is synchronized on. However, it >> does make the code a little clearer. > > It's not critical and no problem if this is cleaned up as a follow-up. Good idea, this could be improved. As Paul says, it's not performance critical as this is a diagnostic option for printing the stack trace when pinned. - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman wrote: >> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which >> JDK version to target. >> >> We will refresh this PR periodically to pick up changes and fixes from the >> loom repo. >> >> Most of the new mechanisms in the HotSpot VM are disabled by default and >> require running with `--enable-preview` to enable. >> >> The patch has support for x64 and aarch64 on the usual operating systems >> (Linux, macOS, and Windows). There are stubs (calling Unimplemented) for >> zero and some of the other ports. Additional ports can be contributed via >> PRs against the fibers branch in the loom repo. >> >> There are changes in many areas. To reduce notifications/mails, the labels >> have been trimmed down for now to hotspot, serviceability and core-libs. >> We'll add the complete set of labels when the PR is further along. >> >> The changes include a refresh of java.util.concurrent and ForkJoinPool from >> Doug Lea's CVS. These changes will probably be proposed and integrated in >> advance of this PR. >> >> The changes include some non-exposed and low-level infrastructure to support >> the (in draft) JEPs for Structured Concurrency and Scope Locals. This is to >> make life a bit easier and avoid having to separate VM changes and juggle >> branches at this time. > > Alan Bateman has updated the pull request incrementally with one additional > commit since the last revision: > > Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e The compiler changes look good to me. Marked as reviewed by dlong (Reviewer). - Marked as reviewed by dlong (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8279598: Provide adapter from RandomGenerator to Random [v20]
On Thu, 21 Apr 2022 03:48:21 GMT, Yasser Bazzi wrote: >> Hi, could i get a review on this implementation proposed by Stuart Marks, i >> decided to use the >> https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/util/random/RandomGenerator.html >> interface to create the default method `asRandom()` that wraps around the >> newer algorithms to be used on classes that do not accept the new interface. >> >> Some things to note as proposed by the bug report, the protected method >> next(int bits) is not overrided and setSeed() method if left blank up to >> discussion on what to do with it. >> >> Small test done on >> https://gist.github.com/YShow/da678561419cda8e32fccf3a27a649d4 > > Yasser Bazzi has updated the pull request incrementally with two additional > commits since the last revision: > > - Update setSeed docs on Random class > - Add nicer toString method to random wrapper Hi, thanks for the updates. I've made some modifications to the specs of `setSeed` and `next(bits)` for good measure, since the latter also needed to be updated to accommodate overrides made by subclasses. If these changes are satisfactory, please pull them into this PR, and I'll update the CSR correspondingly. Changes are in this branch: https://github.com/stuart-marks/jdk/tree/JDK-8279598-RandomGenerator-adapter - PR: https://git.openjdk.java.net/jdk/pull/7001
Re: Floating Point Repair?
On 27.04.22 12:47, sminervini.prism wrote: To the Java OpenJDK Java Community Process, [...] You could propose a new data type that follows BCDs from IEE754-2008. You would have to define conversions of course. You would have to of course define type conversions, find somebody doing whatever a JSR is today, extend hotspot and the java compiler, or prototype with a software emulation. bye Jochen
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 17:53:01 GMT, Mandy Chung wrote: >> Alan Bateman has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e > > src/java.base/share/classes/jdk/internal/misc/UnsafeConstants.java line 122: > >> 120: */ >> 121: >> 122: public static final int SCOPED_CACHE_SHIFT; > > I can't find this constant being used. If added for future, maybe keep > `UnsafeConstants` class and this static field package-private for now. It originally configured the number of lines in extent local cache but the implementation has changed. @theRealAph would be best to comment on this, it may be possible to delete it. > src/java.management/share/classes/sun/management/ThreadImpl.java line 447: > >> 445: if (threads != null) { >> 446: long[] tids = Stream.of(threads) >> 447: .filter(t -> !(t instanceof >> jdk.internal.misc.CarrierThread)) > > Returning an array of thread IDs of just the platform threads is good. The > javadoc needs to be updated to say "Returns an array of thread identifiers > for the platform threads" I think you mean the comment on the private method. Yes, that could be clearer that it just returns platform threads. - PR: https://git.openjdk.java.net/jdk/pull/8166
Integrated: 8285658: Fix two typos in the spec of j.u.random.RandomGenerator
On Tue, 26 Apr 2022 16:38:37 GMT, Raffaello Giulietti wrote: > The spec of the interface `java.util.random.RandomGenerator` is slightly > incorrect when it discusses `float` and `double` random values. This pull request has now been integrated. Changeset: 1f868f1d Author:Raffaello Giulietti Committer: Joe Darcy URL: https://git.openjdk.java.net/jdk/commit/1f868f1d091602cc462ee0fe5fa613a3638a5f1c Stats: 9 lines in 1 file changed: 1 ins; 0 del; 8 mod 8285658: Fix two typos in the spec of j.u.random.RandomGenerator Reviewed-by: bpb, darcy - PR: https://git.openjdk.java.net/jdk/pull/8404
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman wrote: >> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which >> JDK version to target. >> >> We will refresh this PR periodically to pick up changes and fixes from the >> loom repo. >> >> Most of the new mechanisms in the HotSpot VM are disabled by default and >> require running with `--enable-preview` to enable. >> >> The patch has support for x64 and aarch64 on the usual operating systems >> (Linux, macOS, and Windows). There are stubs (calling Unimplemented) for >> zero and some of the other ports. Additional ports can be contributed via >> PRs against the fibers branch in the loom repo. >> >> There are changes in many areas. To reduce notifications/mails, the labels >> have been trimmed down for now to hotspot, serviceability and core-libs. >> We'll add the complete set of labels when the PR is further along. >> >> The changes include a refresh of java.util.concurrent and ForkJoinPool from >> Doug Lea's CVS. These changes will probably be proposed and integrated in >> advance of this PR. >> >> The changes include some non-exposed and low-level infrastructure to support >> the (in draft) JEPs for Structured Concurrency and Scope Locals. This is to >> make life a bit easier and avoid having to separate VM changes and juggle >> branches at this time. > > Alan Bateman has updated the pull request incrementally with one additional > commit since the last revision: > > Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e Marked as reviewed by dlong (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: JDK-8285730: unify _WIN32_WINNT settings
On Wed, 27 Apr 2022 14:57:41 GMT, Matthias Baesken wrote: > Currently we set _WIN32_WINNT at various places in the codebase; this is used > to target a minimum Windows version we want to support. See also for more > detailled information : > https://docs.microsoft.com/en-us/windows/win32/winprog/using-the-windows-headers?redirectedfrom=MSDN#setting-winver-or-_win32_winnt > Macros for Conditional Declarations > "Certain functions that depend on a particular version of Windows are > declared using conditional code. This enables you to use the compiler to > detect whether your application uses functions that are not supported on its > target version(s) of Windows." > > However currently we have quite a lot of differing settings of _WIN32_WINNT > in the codebase ; setting _WIN32_WINNT to 0x0601 (Windows 7) where possible > would make sense because we have this setting already at java_props_md.c > (so targeting older Windows versions at other places is most likely not > useful). Requiring different windows versions in different parts of the product doesn't make much sense, but I think it's even worse trying to keep all these different locations in sync. At least most of these have a comment explaining why that particular Windows version is required there. Changing these values invalidates those comments. If we are to do something about this, then we need to define this macro in a central location, and verify the value in configure. Then we would provide it either on compile command lines, or in a globally included config.h. - PR: https://git.openjdk.java.net/jdk/pull/8428
Re: RFR: 8285690: CloneableReference subtest should not throw CloneNotSupportedException [v2]
On Wed, 27 Apr 2022 18:25:27 GMT, Mandy Chung wrote: >> That was my initial thought, but it doesn't work - CNSE is a checked >> exception so must be handled. > >> test() and main will need to declare this checked exception. > > > diff --git a/test/jdk/java/lang/ref/ReferenceClone.java > b/test/jdk/java/lang/ref/ReferenceClone.java > index bd1ead81bec..2f9386b81e4 100644 > --- a/test/jdk/java/lang/ref/ReferenceClone.java > +++ b/test/jdk/java/lang/ref/ReferenceClone.java > @@ -31,12 +31,12 @@ import java.lang.ref.*; > > public class ReferenceClone { > private static final ReferenceQueue QUEUE = new > ReferenceQueue<>(); > -public static void main(String... args) { > +public static void main(String... args) throws Exception { > ReferenceClone refClone = new ReferenceClone(); > refClone.test(); > } > > -public void test() { > +public void test() throws CloneNotSupportedException { > // test Reference::clone that throws CNSE > Object o = new Object(); > assertCloneNotSupported(new SoftRef(o)); > @@ -45,9 +45,7 @@ public class ReferenceClone { > > // Reference subclass may override the clone method > CloneableReference ref = new CloneableReference(o); > -try { > ref.clone(); > -} catch (CloneNotSupportedException e) {} > } > > private void assertCloneNotSupported(CloneableRef ref) { > ``` Yes, that would work. But I'd rather keep the code for that subtest close together, i.e. as currently written. - PR: https://git.openjdk.java.net/jdk/pull/8418
Re: RFR: 8285690: CloneableReference subtest should not throw CloneNotSupportedException [v2]
On Wed, 27 Apr 2022 18:19:57 GMT, Kim Barrett wrote: >> test/jdk/java/lang/ref/ReferenceClone.java line 52: >> >>> 50: } catch (CloneNotSupportedException e) { >>> 51: throw new RuntimeException("CloneableReference::clone >>> should not throw CloneNotSupportedException"); >>> 52: } >> >> Alternatively, it could simply let CNSE propagate. >> >> >> CloneableReference ref = new CloneableReference(o); >> ref.clone(); >> >> >> `test()` and `main` will need to declare this checked exception. > > That was my initial thought, but it doesn't work - CNSE is a checked > exception so must be handled. > test() and main will need to declare this checked exception. diff --git a/test/jdk/java/lang/ref/ReferenceClone.java b/test/jdk/java/lang/ref/ReferenceClone.java index bd1ead81bec..2f9386b81e4 100644 --- a/test/jdk/java/lang/ref/ReferenceClone.java +++ b/test/jdk/java/lang/ref/ReferenceClone.java @@ -31,12 +31,12 @@ import java.lang.ref.*; public class ReferenceClone { private static final ReferenceQueue QUEUE = new ReferenceQueue<>(); -public static void main(String... args) { +public static void main(String... args) throws Exception { ReferenceClone refClone = new ReferenceClone(); refClone.test(); } -public void test() { +public void test() throws CloneNotSupportedException { // test Reference::clone that throws CNSE Object o = new Object(); assertCloneNotSupported(new SoftRef(o)); @@ -45,9 +45,7 @@ public class ReferenceClone { // Reference subclass may override the clone method CloneableReference ref = new CloneableReference(o); -try { ref.clone(); -} catch (CloneNotSupportedException e) {} } private void assertCloneNotSupported(CloneableRef ref) { ``` - PR: https://git.openjdk.java.net/jdk/pull/8418
Re: RFR: 8285690: CloneableReference subtest should not throw CloneNotSupportedException [v2]
On Wed, 27 Apr 2022 15:57:34 GMT, Mandy Chung wrote: >> Kim Barrett has updated the pull request incrementally with one additional >> commit since the last revision: >> >> update copyright, @bug list > > test/jdk/java/lang/ref/ReferenceClone.java line 52: > >> 50: } catch (CloneNotSupportedException e) { >> 51: throw new RuntimeException("CloneableReference::clone should >> not throw CloneNotSupportedException"); >> 52: } > > Alternatively, it could simply let CNSE propagate. > > > CloneableReference ref = new CloneableReference(o); > ref.clone(); > > > `test()` and `main` will need to declare this checked exception. That was my initial thought, but it doesn't work - CNSE is a checked exception so must be handled. - PR: https://git.openjdk.java.net/jdk/pull/8418
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 18:13:15 GMT, Paul Sandoz wrote: >> src/java.base/share/classes/java/lang/PinnedThreadPrinter.java line 58: >> >>> 56: >>> 57: // maps a class to the hashes of stack traces pinned by that code >>> in that class >>> 58: private static final Map, Hashes> classToHashes = new >>> WeakHashMap<>(); >> >> Can you use `ClassValue` in this case? > > I was wondering that too, but held off commenting since it's not super > performance critical given `classToHashes` is synchronized on. However, it > does make the code a little clearer. It's not critical and no problem if this is cleaned up as a follow-up. - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 17:27:56 GMT, Mandy Chung wrote: >> Alan Bateman has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e > > src/java.base/share/classes/java/lang/PinnedThreadPrinter.java line 58: > >> 56: >> 57: // maps a class to the hashes of stack traces pinned by that code in >> that class >> 58: private static final Map, Hashes> classToHashes = new >> WeakHashMap<>(); > > Can you use `ClassValue` in this case? I was wondering that too, but held off commenting since it's not super performance critical given `classToHashes` is synchronized on. However, it does make the code a little clearer. - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman wrote: >> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which >> JDK version to target. >> >> We will refresh this PR periodically to pick up changes and fixes from the >> loom repo. >> >> Most of the new mechanisms in the HotSpot VM are disabled by default and >> require running with `--enable-preview` to enable. >> >> The patch has support for x64 and aarch64 on the usual operating systems >> (Linux, macOS, and Windows). There are stubs (calling Unimplemented) for >> zero and some of the other ports. Additional ports can be contributed via >> PRs against the fibers branch in the loom repo. >> >> There are changes in many areas. To reduce notifications/mails, the labels >> have been trimmed down for now to hotspot, serviceability and core-libs. >> We'll add the complete set of labels when the PR is further along. >> >> The changes include a refresh of java.util.concurrent and ForkJoinPool from >> Doug Lea's CVS. These changes will probably be proposed and integrated in >> advance of this PR. >> >> The changes include some non-exposed and low-level infrastructure to support >> the (in draft) JEPs for Structured Concurrency and Scope Locals. This is to >> make life a bit easier and avoid having to separate VM changes and juggle >> branches at this time. > > Alan Bateman has updated the pull request incrementally with one additional > commit since the last revision: > > Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e src/java.management/share/classes/sun/management/ThreadImpl.java line 447: > 445: if (threads != null) { > 446: long[] tids = Stream.of(threads) > 447: .filter(t -> !(t instanceof > jdk.internal.misc.CarrierThread)) Returning an array of thread IDs of just the platform threads is good. The javadoc needs to be updated to say "Returns an array of thread identifiers for the platform threads" - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8285633: Take better advantage of generic MethodType cache [v2]
On Tue, 26 Apr 2022 20:47:39 GMT, Claes Redestad wrote: >> The `MethodType.genericMethodType` methods provide convenience methods for >> certain common method types and also provide `@Stable` cache that allows for >> constant folding. This patch enhances the more generic `methodType` methods >> to take advantage of this cache, when possible. This allows calls like >> `MethodType.methodType(Object.class, Object.class, ...)` to constant fold, >> making them 50x+ faster and allocation-free. >> >> Baseline: >> >> BenchmarkMode Cnt Score Error Units >> MethodTypeAcquire.baselineRawavgt 15 0.673 ? 0.017 ns/op >> MethodTypeAcquire.testGenericObject avgt 15 0.579 ? 0.125 ns/op >> MethodTypeAcquire.testMultiPType avgt 15 46.671 ? 1.134 ns/op >> MethodTypeAcquire.testMultiPType_Arg avgt 15 27.019 ? 0.437 ns/op >> MethodTypeAcquire.testMultiPType_ObjectAndA avgt 15 57.217 ? 0.505 ns/op >> MethodTypeAcquire.testMultiPType_ObjectOnly avgt 15 57.114 ? 1.214 ns/op >> MethodTypeAcquire.testObjectObject avgt 15 33.380 ? 1.810 ns/op >> MethodTypeAcquire.testReturnInt avgt 15 28.043 ? 0.187 ns/op >> MethodTypeAcquire.testReturnObject avgt 15 24.313 ? 0.074 ns/op >> MethodTypeAcquire.testReturnVoid avgt 15 24.360 ? 0.155 ns/op >> MethodTypeAcquire.testSinglePTypeavgt 15 33.572 ? 1.101 ns/op >> >> >> Patched: >> >> BenchmarkMode Cnt Score Error Units >> MethodTypeAcquire.baselineRawavgt 15 0.683 ? 0.024 ns/op >> MethodTypeAcquire.testGenericObject avgt 15 0.547 ? 0.047 ns/op >> MethodTypeAcquire.testMultiPType avgt 15 48.532 ? 0.982 ns/op >> MethodTypeAcquire.testMultiPType_Arg avgt 15 31.390 ? 5.269 ns/op >> MethodTypeAcquire.testMultiPType_ObjectAndA avgt 15 60.165 ? 2.756 ns/op >> MethodTypeAcquire.testMultiPType_ObjectOnly avgt 15 0.599 ? 0.166 ns/op >> MethodTypeAcquire.testObjectObject avgt 15 0.541 ? 0.045 ns/op >> MethodTypeAcquire.testReturnInt avgt 15 28.174 ? 0.257 ns/op >> MethodTypeAcquire.testReturnObject avgt 15 0.542 ? 0.045 ns/op >> MethodTypeAcquire.testReturnVoid avgt 15 24.621 ? 0.202 ns/op >> MethodTypeAcquire.testSinglePTypeavgt 15 33.614 ? 1.109 ns/op >> >> >> The cost on method types that don't match are in the noise (0-2ns/op). Even >> on a test I constructed to act as a worst case (`testMultiPType_ObjectAndA`) >> there's barely any appreciable cost (~3ns/op, with overlapping error). > > Claes Redestad has updated the pull request incrementally with one additional > commit since the last revision: > > Add micros using non-constant arguments to assess performance in absense of > constant folding Good work. - PR: https://git.openjdk.java.net/jdk/pull/8398
Integrated: 8285633: Take better advantage of generic MethodType cache
On Tue, 26 Apr 2022 10:57:04 GMT, Claes Redestad wrote: > The `MethodType.genericMethodType` methods provide convenience methods for > certain common method types and also provide `@Stable` cache that allows for > constant folding. This patch enhances the more generic `methodType` methods > to take advantage of this cache, when possible. This allows calls like > `MethodType.methodType(Object.class, Object.class, ...)` to constant fold, > making them 50x+ faster and allocation-free. > > Baseline: > > BenchmarkMode Cnt Score Error Units > MethodTypeAcquire.baselineRawavgt 15 0.673 ? 0.017 ns/op > MethodTypeAcquire.testGenericObject avgt 15 0.579 ? 0.125 ns/op > MethodTypeAcquire.testMultiPType avgt 15 46.671 ? 1.134 ns/op > MethodTypeAcquire.testMultiPType_Arg avgt 15 27.019 ? 0.437 ns/op > MethodTypeAcquire.testMultiPType_ObjectAndA avgt 15 57.217 ? 0.505 ns/op > MethodTypeAcquire.testMultiPType_ObjectOnly avgt 15 57.114 ? 1.214 ns/op > MethodTypeAcquire.testObjectObject avgt 15 33.380 ? 1.810 ns/op > MethodTypeAcquire.testReturnInt avgt 15 28.043 ? 0.187 ns/op > MethodTypeAcquire.testReturnObject avgt 15 24.313 ? 0.074 ns/op > MethodTypeAcquire.testReturnVoid avgt 15 24.360 ? 0.155 ns/op > MethodTypeAcquire.testSinglePTypeavgt 15 33.572 ? 1.101 ns/op > > > Patched: > > BenchmarkMode Cnt Score Error Units > MethodTypeAcquire.baselineRawavgt 15 0.683 ? 0.024 ns/op > MethodTypeAcquire.testGenericObject avgt 15 0.547 ? 0.047 ns/op > MethodTypeAcquire.testMultiPType avgt 15 48.532 ? 0.982 ns/op > MethodTypeAcquire.testMultiPType_Arg avgt 15 31.390 ? 5.269 ns/op > MethodTypeAcquire.testMultiPType_ObjectAndA avgt 15 60.165 ? 2.756 ns/op > MethodTypeAcquire.testMultiPType_ObjectOnly avgt 15 0.599 ? 0.166 ns/op > MethodTypeAcquire.testObjectObject avgt 15 0.541 ? 0.045 ns/op > MethodTypeAcquire.testReturnInt avgt 15 28.174 ? 0.257 ns/op > MethodTypeAcquire.testReturnObject avgt 15 0.542 ? 0.045 ns/op > MethodTypeAcquire.testReturnVoid avgt 15 24.621 ? 0.202 ns/op > MethodTypeAcquire.testSinglePTypeavgt 15 33.614 ? 1.109 ns/op > > > The cost on method types that don't match are in the noise (0-2ns/op). Even > on a test I constructed to act as a worst case (`testMultiPType_ObjectAndA`) > there's barely any appreciable cost (~3ns/op, with overlapping error). This pull request has now been integrated. Changeset: 6c79671e Author:Claes Redestad URL: https://git.openjdk.java.net/jdk/commit/6c79671e50d572f3da3a286d34a98dcb83b8d906 Stats: 119 lines in 8 files changed: 96 ins; 1 del; 22 mod 8285633: Take better advantage of generic MethodType cache Reviewed-by: jvernee - PR: https://git.openjdk.java.net/jdk/pull/8398
Re: RFR: 8285517: System.getenv() returns unexpected value if environment variable has non ASCII character
On Sun, 24 Apr 2022 09:18:54 GMT, Ichiroh Takiguchi wrote: > On JDK19 with Linux ja_JP.eucjp locale, > System.getenv() returns unexpected value if environment variable has Japanese > EUC characters. > It seems this issue happens because of JEP 400. > Arguments for ProcessBuilder have same kind of issue. src/java.base/unix/classes/java/lang/ProcessEnvironment.java line 68: > 66: private static final Map theUnmodifiableEnvironment; > 67: static final int MIN_NAME_LENGTH = 0; > 68: static final Charset cs = > Charset.forName(StaticProperty.nativeEncoding(), cs should have an all-CAPS name to make it clear its a static value throughout the implementation. And `private` too. test/jdk/java/lang/System/i18nEnvArg.java line 41: > 39: > 40: public class i18nEnvArg { > 41: final static String text = "\u6F22\u5B57"; Can this be renamed to indicate it is the string under test? like KANJI_TEXT or EUC_JP_TEXT or similar. test/jdk/java/lang/System/i18nEnvArg.java line 49: > 47: final static int maxSize = 4096; > 48: > 49: public static void main(String[] args) throws Exception { There are 3 main()'s in this test; it would be useful to add a comment indicating the purpose of each. test/jdk/java/lang/System/i18nEnvArg.java line 60: > 58: Process p = pb.start(); > 59: InputStream is = p.getInputStream(); > 60: byte[] ba = new byte[maxSize]; You can use `InputStream.readAllBytes()` here to return a byte buffer. Its not clear why all the bytes are read? I assume its only for a possible error from the child. test/jdk/java/lang/System/i18nEnvArg.java line 128: > 126: Method enviorn_mid = cls.getDeclaredMethod("environ"); > 127: enviorn_mid.setAccessible(true); > 128: byte[][] environ = (byte[][]) enviorn_mid.invoke(null, typo: "enviorn_mid" -> "environ_mid". test/jdk/java/lang/System/i18nEnvArg.java line 138: > 136: bb.put(environ[i+1]); > 137: byte[] envb = Arrays.copyOf(ba, bb.position()); > 138: if (Arrays.equals(kanji, envb)) return; It seems odd to exit the loop on the first match. I think AIX always has environment variables not set by the caller. test/jdk/java/lang/System/i18nEnvArg.java line 146: > 144: sb.append((char)b); > 145: } else { > 146: sb.append(String.format("\\x%02X", (int)b & > 0xFF)); The methods of java.util.HexFormat may be useful here. It seems that the "5C" would fit into this "else" clause and not need a separate statement. - PR: https://git.openjdk.java.net/jdk/pull/8378
Re: RFR: JDK-8285730: unify _WIN32_WINNT settings
On Wed, 27 Apr 2022 14:57:41 GMT, Matthias Baesken wrote: > Currently we set _WIN32_WINNT at various places in the codebase; this is used > to target a minimum Windows version we want to support. See also for more > detailled information : > https://docs.microsoft.com/en-us/windows/win32/winprog/using-the-windows-headers?redirectedfrom=MSDN#setting-winver-or-_win32_winnt > Macros for Conditional Declarations > "Certain functions that depend on a particular version of Windows are > declared using conditional code. This enables you to use the compiler to > detect whether your application uses functions that are not supported on its > target version(s) of Windows." > > However currently we have quite a lot of differing settings of _WIN32_WINNT > in the codebase ; setting _WIN32_WINNT to 0x0601 (Windows 7) where possible > would make sense because we have this setting already at java_props_md.c > (so targeting older Windows versions at other places is most likely not > useful). This is OK by me, but I wonder if the build folks might want to think about this and whether it should be centralised somehow since it seems odd to mandate different versions of windows in different places. - PR: https://git.openjdk.java.net/jdk/pull/8428
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman wrote: >> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which >> JDK version to target. >> >> We will refresh this PR periodically to pick up changes and fixes from the >> loom repo. >> >> Most of the new mechanisms in the HotSpot VM are disabled by default and >> require running with `--enable-preview` to enable. >> >> The patch has support for x64 and aarch64 on the usual operating systems >> (Linux, macOS, and Windows). There are stubs (calling Unimplemented) for >> zero and some of the other ports. Additional ports can be contributed via >> PRs against the fibers branch in the loom repo. >> >> There are changes in many areas. To reduce notifications/mails, the labels >> have been trimmed down for now to hotspot, serviceability and core-libs. >> We'll add the complete set of labels when the PR is further along. >> >> The changes include a refresh of java.util.concurrent and ForkJoinPool from >> Doug Lea's CVS. These changes will probably be proposed and integrated in >> advance of this PR. >> >> The changes include some non-exposed and low-level infrastructure to support >> the (in draft) JEPs for Structured Concurrency and Scope Locals. This is to >> make life a bit easier and avoid having to separate VM changes and juggle >> branches at this time. > > Alan Bateman has updated the pull request incrementally with one additional > commit since the last revision: > > Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e src/java.base/share/classes/jdk/internal/misc/UnsafeConstants.java line 122: > 120: */ > 121: > 122: public static final int SCOPED_CACHE_SHIFT; I can't find this constant being used. If added for future, maybe keep `UnsafeConstants` class and this static field package-private for now. - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8285485: Fix typos in corelibs
On Wed, 27 Apr 2022 17:02:40 GMT, Pavel Rappo wrote: >> I ran `codespell` on modules owned by core-libs, and accepted those changes >> where it indeed discovered real typos. >> >> I will update copyright years using a script before pushing (otherwise like >> every second change would be a copyright update, making reviewing much >> harder). >> >> The long term goal here is to make tooling support for running `codespell`. >> The trouble with automating this is of course all false positives. But >> before even trying to solve that issue, all true positives must be fixed. >> Hence this PR. > > src/java.sql.rowset/share/classes/com/sun/rowset/providers/RIXMLProvider.java > line 239: > >> 237: >> 238: /** >> 239: * Returns the vendor name of the Reference Implementation >> Optimistic > > L240: an optimistic _what_ provider? :-) Yep typo there. The original authors needed to run spell check ;-) - PR: https://git.openjdk.java.net/jdk/pull/8364
Re: RFR: 8285485: Fix typos in corelibs
On Fri, 22 Apr 2022 15:08:51 GMT, Magnus Ihse Bursie wrote: > I ran `codespell` on modules owned by core-libs, and accepted those changes > where it indeed discovered real typos. > > I will update copyright years using a script before pushing (otherwise like > every second change would be a copyright update, making reviewing much > harder). > > The long term goal here is to make tooling support for running `codespell`. > The trouble with automating this is of course all false positives. But before > even trying to solve that issue, all true positives must be fixed. Hence this > PR. Changes look OK with the exception of the comment pointed out below - Marked as reviewed by lancea (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8364
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman wrote: >> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which >> JDK version to target. >> >> We will refresh this PR periodically to pick up changes and fixes from the >> loom repo. >> >> Most of the new mechanisms in the HotSpot VM are disabled by default and >> require running with `--enable-preview` to enable. >> >> The patch has support for x64 and aarch64 on the usual operating systems >> (Linux, macOS, and Windows). There are stubs (calling Unimplemented) for >> zero and some of the other ports. Additional ports can be contributed via >> PRs against the fibers branch in the loom repo. >> >> There are changes in many areas. To reduce notifications/mails, the labels >> have been trimmed down for now to hotspot, serviceability and core-libs. >> We'll add the complete set of labels when the PR is further along. >> >> The changes include a refresh of java.util.concurrent and ForkJoinPool from >> Doug Lea's CVS. These changes will probably be proposed and integrated in >> advance of this PR. >> >> The changes include some non-exposed and low-level infrastructure to support >> the (in draft) JEPs for Structured Concurrency and Scope Locals. This is to >> make life a bit easier and avoid having to separate VM changes and juggle >> branches at this time. > > Alan Bateman has updated the pull request incrementally with one additional > commit since the last revision: > > Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e Reviewed debugger-related code (JDI/JDWP/JDB/tests) and partially JVMTI. - Marked as reviewed by amenkov (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8285485: Fix typos in corelibs
On Fri, 22 Apr 2022 15:08:51 GMT, Magnus Ihse Bursie wrote: > I ran `codespell` on modules owned by core-libs, and accepted those changes > where it indeed discovered real typos. > > I will update copyright years using a script before pushing (otherwise like > every second change would be a copyright update, making reviewing much > harder). > > The long term goal here is to make tooling support for running `codespell`. > The trouble with automating this is of course all false positives. But before > even trying to solve that issue, all true positives must be fixed. Hence this > PR. Thanks for doing this so carefully. If reviewers decide that parts of this PR need to be addressed upstream, we should probably consider contributing those parts to respective projects. src/java.sql.rowset/share/classes/com/sun/rowset/CachedRowSetImpl.java line 970: > 968: * and sends a rowSetChanged event to all registered > 969: * listeners. > 970: * @throws SQLException if an error is occurs rolling back the RowSet L976, same as above: "is occurs". src/java.sql.rowset/share/classes/com/sun/rowset/JoinRowSetImpl.java line 636: > 634: // to be INNER JOIN'ED to form a new rowset > 635: // Compare table1.MatchColumn1.value1 == { > table2.MatchColumn2.value1 > 636: // ... up to > table2.MatchColumn2.valueN } Curious: it is not some established string representation, is it? src/java.sql.rowset/share/classes/com/sun/rowset/internal/SyncResolverImpl.java line 664: > 662: * and sends a {@code rowSetChanged} event to all registered > 663: * listeners. > 664: * @throws SQLException if an error is occurs rolling back the RowSet L664: delete "is" in "is occurs". src/java.sql.rowset/share/classes/com/sun/rowset/providers/RIXMLProvider.java line 239: > 237: > 238: /** > 239: * Returns the vendor name of the Reference Implementation Optimistic L240: an optimistic _what_ provider? :-) src/java.xml/share/classes/com/sun/xml/internal/stream/writers/XMLDOMWriterImpl.java line 238: > 236: > 237: /** > 238: * Creates a DOM Attribute @see org.w3c.dom.Node and associates it > with the current DOM element @see org.w3c.dom.Node. Not that it matters in this PR, but I think I should mention that `@see` tags do not work like that. (No action needed from you.) src/java.xml/share/classes/javax/xml/transform/Transformer.java line 127: > 125: * namespace URI in curly braces ({}). > 126: * @param value The value object. This can be any valid Java > object. It is > 127: * up to the processor to provide the proper object coersion or to > simply That made me pause: some systems have the notion of _type coercion_; but your change looks right. - PR: https://git.openjdk.java.net/jdk/pull/8364
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman wrote: >> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which >> JDK version to target. >> >> We will refresh this PR periodically to pick up changes and fixes from the >> loom repo. >> >> Most of the new mechanisms in the HotSpot VM are disabled by default and >> require running with `--enable-preview` to enable. >> >> The patch has support for x64 and aarch64 on the usual operating systems >> (Linux, macOS, and Windows). There are stubs (calling Unimplemented) for >> zero and some of the other ports. Additional ports can be contributed via >> PRs against the fibers branch in the loom repo. >> >> There are changes in many areas. To reduce notifications/mails, the labels >> have been trimmed down for now to hotspot, serviceability and core-libs. >> We'll add the complete set of labels when the PR is further along. >> >> The changes include a refresh of java.util.concurrent and ForkJoinPool from >> Doug Lea's CVS. These changes will probably be proposed and integrated in >> advance of this PR. >> >> The changes include some non-exposed and low-level infrastructure to support >> the (in draft) JEPs for Structured Concurrency and Scope Locals. This is to >> make life a bit easier and avoid having to separate VM changes and juggle >> branches at this time. > > Alan Bateman has updated the pull request incrementally with one additional > commit since the last revision: > > Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e src/java.base/share/classes/java/lang/PinnedThreadPrinter.java line 58: > 56: > 57: // maps a class to the hashes of stack traces pinned by that code in > that class > 58: private static final Map, Hashes> classToHashes = new > WeakHashMap<>(); Can you use `ClassValue` in this case? - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284992: Fix misleading Vector API doc for LSHR operator [v3]
On Wed, 27 Apr 2022 09:06:12 GMT, Jie Fu wrote: >> Hi all, >> >> The Current Vector API doc for `LSHR` is >> >> Produce a>>>(n&(ESIZE*8-1)). Integral only. >> >> >> This is misleading which may lead to bugs for Java developers. >> This is because for negative byte/short elements, the results computed by >> `LSHR` will be different from that of `>>>`. >> For more details, please see >> https://github.com/openjdk/jdk/pull/8276#issue-1206391831 . >> >> After the patch, the doc for `LSHR` is >> >> Produce zero-extended right shift of a by (n&(ESIZE*8-1)) bits. Integral >> only. >> >> >> Thanks. >> Best regards, >> Jie > > Jie Fu 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: > > - Address review comments > - Merge branch 'master' into JDK-8284992 > - Merge branch 'master' into JDK-8284992 > - Address review comments > - Merge branch 'master' into JDK-8284992 > - 8284992: Fix misleading Vector API doc for LSHR operator Thanks, looks good, we will need to create a CSR. Have you done that before? - PR: https://git.openjdk.java.net/jdk/pull/8291
Integrated: 8285736: JDK-8236128 causes validate-source failures
On Wed, 27 Apr 2022 16:53:44 GMT, Daniel D. Daugherty wrote: > A trivial fix for JDK-8236128 causes validate-source failures. This pull request has now been integrated. Changeset: 5b42747b Author:Daniel D. Daugherty URL: https://git.openjdk.java.net/jdk/commit/5b42747ba1606b34b05449518fa601d2451c5c66 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod 8285736: JDK-8236128 causes validate-source failures Reviewed-by: mikael, asemenyuk - PR: https://git.openjdk.java.net/jdk/pull/8429
Re: Integrated: 8285736: JDK-8236128 causes validate-source failures
On Wed, 27 Apr 2022 16:53:44 GMT, Daniel D. Daugherty wrote: > A trivial fix for JDK-8236128 causes validate-source failures. Marked as reviewed by asemenyuk (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8429
Re: Integrated: 8285736: JDK-8236128 causes validate-source failures
On Wed, 27 Apr 2022 16:53:44 GMT, Daniel D. Daugherty wrote: > A trivial fix for JDK-8236128 causes validate-source failures. The bot is being very, very, slow to integrate this... - PR: https://git.openjdk.java.net/jdk/pull/8429
Re: Integrated: 8285736: JDK-8236128 causes validate-source failures
On Wed, 27 Apr 2022 16:55:11 GMT, Mikael Vidstedt wrote: >> A trivial fix for JDK-8236128 causes validate-source failures. > > Marked as reviewed by mikael (Reviewer). @vidmik and @alexeysemenyukoracle - Thanks for the fast reviews. - PR: https://git.openjdk.java.net/jdk/pull/8429
Re: Integrated: 8285736: JDK-8236128 causes validate-source failures
On Wed, 27 Apr 2022 16:53:44 GMT, Daniel D. Daugherty wrote: > A trivial fix for JDK-8236128 causes validate-source failures. Marked as reviewed by mikael (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8429
Integrated: 8285736: JDK-8236128 causes validate-source failures
A trivial fix for JDK-8236128 causes validate-source failures. - Commit messages: - 8285736: JDK-8236128 causes validate-source failures Changes: https://git.openjdk.java.net/jdk/pull/8429/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8429=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8285736 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/8429.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8429/head:pull/8429 PR: https://git.openjdk.java.net/jdk/pull/8429
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman wrote: >> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which >> JDK version to target. >> >> We will refresh this PR periodically to pick up changes and fixes from the >> loom repo. >> >> Most of the new mechanisms in the HotSpot VM are disabled by default and >> require running with `--enable-preview` to enable. >> >> The patch has support for x64 and aarch64 on the usual operating systems >> (Linux, macOS, and Windows). There are stubs (calling Unimplemented) for >> zero and some of the other ports. Additional ports can be contributed via >> PRs against the fibers branch in the loom repo. >> >> There are changes in many areas. To reduce notifications/mails, the labels >> have been trimmed down for now to hotspot, serviceability and core-libs. >> We'll add the complete set of labels when the PR is further along. >> >> The changes include a refresh of java.util.concurrent and ForkJoinPool from >> Doug Lea's CVS. These changes will probably be proposed and integrated in >> advance of this PR. >> >> The changes include some non-exposed and low-level infrastructure to support >> the (in draft) JEPs for Structured Concurrency and Scope Locals. This is to >> make life a bit easier and avoid having to separate VM changes and juggle >> branches at this time. > > Alan Bateman has updated the pull request incrementally with one additional > commit since the last revision: > > Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e I've reviewed the JVM/TI, JDWP and JDI changes and I'm good with those. I haven't reviewed the JDB changes (forgot about those) and I have not (yet) reviewed the Runtime changes. - Marked as reviewed by dcubed (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman wrote: >> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which >> JDK version to target. >> >> We will refresh this PR periodically to pick up changes and fixes from the >> loom repo. >> >> Most of the new mechanisms in the HotSpot VM are disabled by default and >> require running with `--enable-preview` to enable. >> >> The patch has support for x64 and aarch64 on the usual operating systems >> (Linux, macOS, and Windows). There are stubs (calling Unimplemented) for >> zero and some of the other ports. Additional ports can be contributed via >> PRs against the fibers branch in the loom repo. >> >> There are changes in many areas. To reduce notifications/mails, the labels >> have been trimmed down for now to hotspot, serviceability and core-libs. >> We'll add the complete set of labels when the PR is further along. >> >> The changes include a refresh of java.util.concurrent and ForkJoinPool from >> Doug Lea's CVS. These changes will probably be proposed and integrated in >> advance of this PR. >> >> The changes include some non-exposed and low-level infrastructure to support >> the (in draft) JEPs for Structured Concurrency and Scope Locals. This is to >> make life a bit easier and avoid having to separate VM changes and juggle >> branches at this time. > > Alan Bateman has updated the pull request incrementally with one additional > commit since the last revision: > > Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e Been through the JDI changes and jdb/example code. Built and manually tested some operations with jdb observing virtual threads. Been through jdk.management/share/classes/com/sun/management and also manually tested jconsole attaching (e.g. dumpThreads operation in both TEXT_PLAIN and JSON). - Marked as reviewed by kevinw (Committer). PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8285485: Fix typos in corelibs
On Fri, 22 Apr 2022 15:08:51 GMT, Magnus Ihse Bursie wrote: > I ran `codespell` on modules owned by core-libs, and accepted those changes > where it indeed discovered real typos. > > I will update copyright years using a script before pushing (otherwise like > every second change would be a copyright update, making reviewing much > harder). > > The long term goal here is to make tooling support for running `codespell`. > The trouble with automating this is of course all false positives. But before > even trying to solve that issue, all true positives must be fixed. Hence this > PR. src/java.se/share/data/jdwp/jdwp.spec line 47: > 45: "Returns reference types for all the classes loaded by the target > VM " > 46: "which match the given signature. " > 47: "Multiple reference types will be returned if two or more class " This file's name scares me. The fact that there's no "serviceability" label on this PR adds to this feeling. I would ask around if I were you. - PR: https://git.openjdk.java.net/jdk/pull/8364
Re: RFR: 8238373: Punctuation should be same in jlink help usage on Japanese language
On Wed, 27 Apr 2022 08:59:20 GMT, KIRIYAMA Takuya wrote: > When showing help for the jlink command in a Japanese locale, delimiters of > option's aliases are a mixture of "," and \u3001. Delimiter should be unified > to ",". > As the same, there is a inconsistency of delimiters in the jar command help > in a Japanese locale, and "," should be used. > Similarly, the javap command help uses space as delimiters other than the > module option in all locales. This inconsistency should also be corrected. > > Would you please review this fix? Looks fine to me. Nit: please modify the copyright years for `javap` properties files, as you modified the base (`javap.properties`) file. - Marked as reviewed by naoto (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8417
Integrated: JDK-8236128: Allow jpackage create installers for services
On Fri, 11 Mar 2022 23:37:06 GMT, Alexey Semenyuk wrote: > Implementation of [JDK-8275062: "Allow jpackage create installers for > services"](https://bugs.openjdk.java.net/browse/JDK-8275062) > CSR This pull request has now been integrated. Changeset: b675c597 Author:Alexey Semenyuk URL: https://git.openjdk.java.net/jdk/commit/b675c597e3f22af9e75992dab27001b9875af32e Stats: 3116 lines in 64 files changed: 2848 ins; 121 del; 147 mod 8236128: Allow jpackage create installers for services Reviewed-by: almatvee - PR: https://git.openjdk.java.net/jdk/pull/7793
Re: RFR: 8285485: Fix typos in corelibs
On Wed, 27 Apr 2022 06:39:30 GMT, Athijegannathan Sundararajan wrote: >> I ran `codespell` on modules owned by core-libs, and accepted those changes >> where it indeed discovered real typos. >> >> I will update copyright years using a script before pushing (otherwise like >> every second change would be a copyright update, making reviewing much >> harder). >> >> The long term goal here is to make tooling support for running `codespell`. >> The trouble with automating this is of course all false positives. But >> before even trying to solve that issue, all true positives must be fixed. >> Hence this PR. > > src/jdk.jlink/share/classes/jdk/tools/jlink/resources/plugins_zh_CN.properties > line 188: > >> 186: main.plugin.module=\u63D2\u4EF6\u6A21\u5757 >> 187: >> 188: main.plugin.category=\u7C7B\u522B > > what's this? Removing the trailing space? A similar one is in the `ja` resource bundle. - PR: https://git.openjdk.java.net/jdk/pull/8364
Re: RFR: 8285485: Fix typos in corelibs
On Fri, 22 Apr 2022 15:08:51 GMT, Magnus Ihse Bursie wrote: > I ran `codespell` on modules owned by core-libs, and accepted those changes > where it indeed discovered real typos. > > I will update copyright years using a script before pushing (otherwise like > every second change would be a copyright update, making reviewing much > harder). > > The long term goal here is to make tooling support for running `codespell`. > The trouble with automating this is of course all false positives. But before > even trying to solve that issue, all true positives must be fixed. Hence this > PR. Marked as reviewed by naoto (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8364
Re: RFR: 8285690: CloneableReference subtest should not throw CloneNotSupportedException [v2]
On Wed, 27 Apr 2022 10:11:15 GMT, Kim Barrett wrote: >> Please review this fix to test/jdk/java/lang/ref/ReferenceClone.java. It was >> passing if CloneableReference::clone were to throw >> CloneNotSupportedException. >> That should be a failure. >> >> Testing: >> Locally (linux-x64) verified test still passes. Temporarily changed >> CloneableReference::clone to throw and verified the expected failure gets >> reported, unlike before. > > Kim Barrett has updated the pull request incrementally with one additional > commit since the last revision: > > update copyright, @bug list Marked as reviewed by mchung (Reviewer). test/jdk/java/lang/ref/ReferenceClone.java line 52: > 50: } catch (CloneNotSupportedException e) { > 51: throw new RuntimeException("CloneableReference::clone should > not throw CloneNotSupportedException"); > 52: } Alternatively, it could simply let CNSE propagate. CloneableReference ref = new CloneableReference(o); ref.clone(); `test()` and `main` will need to declare this checked exception. - PR: https://git.openjdk.java.net/jdk/pull/8418
Re: RFR: 8285658: Fix two typos in the spec of j.u.random.RandomGenerator [v3]
On Wed, 27 Apr 2022 07:34:29 GMT, Raffaello Giulietti wrote: >> The spec of the interface `java.util.random.RandomGenerator` is slightly >> incorrect when it discusses `float` and `double` random values. > > Raffaello Giulietti has updated the pull request incrementally with one > additional commit since the last revision: > > 8285658: Fix two typos in the spec of j.u.random.RandomGenerator Marked as reviewed by bpb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8404
Re: RFR: 8285485: Fix typos in corelibs
On Fri, 22 Apr 2022 15:08:51 GMT, Magnus Ihse Bursie wrote: > I ran `codespell` on modules owned by core-libs, and accepted those changes > where it indeed discovered real typos. > > I will update copyright years using a script before pushing (otherwise like > every second change would be a copyright update, making reviewing much > harder). > > The long term goal here is to make tooling support for running `codespell`. > The trouble with automating this is of course all false positives. But before > even trying to solve that issue, all true positives must be fixed. Hence this > PR. Since you've changed some code; you need to run tests for tiers 1-3. (Note that even for trivial changes, hundreds of OpenJDK developers are notified and distracted; be considerate of other developers). src/java.xml/share/classes/com/sun/xml/internal/stream/XMLEventReaderImpl.java line 140: > 138: } else if(type == XMLEvent.START_ELEMENT) { > 139: throw new XMLStreamException( > 140: "elementGetText() function expects text only element but > START_ELEMENT was encountered.", event.getLocation()); Should we be fixing typos in third party software; it can easily get wiped out in an update. src/java.xml/share/classes/com/sun/xml/internal/stream/writers/WriterUtility.java line 97: > 95: } > 96: else{ > 97: //attempt to retrieve default fEncoderoder "fEncoderoder" looks out of place, please recheck. - PR: https://git.openjdk.java.net/jdk/pull/8364
Re: RFR: 8285690: CloneableReference subtest should not throw CloneNotSupportedException [v2]
On Wed, 27 Apr 2022 10:11:15 GMT, Kim Barrett wrote: >> Please review this fix to test/jdk/java/lang/ref/ReferenceClone.java. It was >> passing if CloneableReference::clone were to throw >> CloneNotSupportedException. >> That should be a failure. >> >> Testing: >> Locally (linux-x64) verified test still passes. Temporarily changed >> CloneableReference::clone to throw and verified the expected failure gets >> reported, unlike before. > > Kim Barrett has updated the pull request incrementally with one additional > commit since the last revision: > > update copyright, @bug list LGTM - Marked as reviewed by rriggs (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8418
Re: RFR: JDK-8285730: unify _WIN32_WINNT settings
On Wed, 27 Apr 2022 14:57:41 GMT, Matthias Baesken wrote: > Currently we set _WIN32_WINNT at various places in the codebase; this is used > to target a minimum Windows version we want to support. See also for more > detailled information : > https://docs.microsoft.com/en-us/windows/win32/winprog/using-the-windows-headers?redirectedfrom=MSDN#setting-winver-or-_win32_winnt > Macros for Conditional Declarations > "Certain functions that depend on a particular version of Windows are > declared using conditional code. This enables you to use the compiler to > detect whether your application uses functions that are not supported on its > target version(s) of Windows." > > However currently we have quite a lot of differing settings of _WIN32_WINNT > in the codebase ; setting _WIN32_WINNT to 0x0601 (Windows 7) where possible > would make sense because we have this setting already at java_props_md.c > (so targeting older Windows versions at other places is most likely not > useful). src/java.base/windows/native/libnio/ch/wepoll.c line 159: > 157: > 158: #undef _WIN32_WINNT > 159: #define _WIN32_WINNT 0x0601 This is 3rd party code and would prefer not change it if possible. - PR: https://git.openjdk.java.net/jdk/pull/8428
RFR: JDK-8285730: unify _WIN32_WINNT settings
Currently we set _WIN32_WINNT at various places in the codebase; this is used to target a minimum Windows version we want to support. See also for more detailled information : https://docs.microsoft.com/en-us/windows/win32/winprog/using-the-windows-headers?redirectedfrom=MSDN#setting-winver-or-_win32_winnt Macros for Conditional Declarations "Certain functions that depend on a particular version of Windows are declared using conditional code. This enables you to use the compiler to detect whether your application uses functions that are not supported on its target version(s) of Windows." However currently we have quite a lot of differing settings of _WIN32_WINNT in the codebase ; setting _WIN32_WINNT to 0x0601 (Windows 7) where possible would make sense because we have this setting already at java_props_md.c (so targeting older Windows versions at other places is most likely not useful). - Commit messages: - JDK-8285730 Changes: https://git.openjdk.java.net/jdk/pull/8428/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8428=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8285730 Stats: 14 lines in 7 files changed: 1 ins; 0 del; 13 mod Patch: https://git.openjdk.java.net/jdk/pull/8428.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8428/head:pull/8428 PR: https://git.openjdk.java.net/jdk/pull/8428
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman wrote: >> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which >> JDK version to target. >> >> We will refresh this PR periodically to pick up changes and fixes from the >> loom repo. >> >> Most of the new mechanisms in the HotSpot VM are disabled by default and >> require running with `--enable-preview` to enable. >> >> The patch has support for x64 and aarch64 on the usual operating systems >> (Linux, macOS, and Windows). There are stubs (calling Unimplemented) for >> zero and some of the other ports. Additional ports can be contributed via >> PRs against the fibers branch in the loom repo. >> >> There are changes in many areas. To reduce notifications/mails, the labels >> have been trimmed down for now to hotspot, serviceability and core-libs. >> We'll add the complete set of labels when the PR is further along. >> >> The changes include a refresh of java.util.concurrent and ForkJoinPool from >> Doug Lea's CVS. These changes will probably be proposed and integrated in >> advance of this PR. >> >> The changes include some non-exposed and low-level infrastructure to support >> the (in draft) JEPs for Structured Concurrency and Scope Locals. This is to >> make life a bit easier and avoid having to separate VM changes and juggle >> branches at this time. > > Alan Bateman has updated the pull request incrementally with one additional > commit since the last revision: > > Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e Reviewed jvmti changes, hotspot tests (excluding tests modified by lmesnik), jdk svc tests changes. - Marked as reviewed by lmesnik (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284950: CgroupV1 detection code should consider memory.swappiness [v6]
On Wed, 27 Apr 2022 10:34:33 GMT, xpbob wrote: >> set memory.swappiness to 0,swap space will not be used >> determine the value of memory.swappiness >> https://www.kernel.org/doc/Documentation/cgroup-v1/memory.txt >> >> >> Memory Limit: 50.00M >> Memory Soft Limit: Unlimited >> Memory & Swap Limit: 100.00M >> Maximum Processes Limit: 4194305 >> >> => >> >> Memory Limit: 50.00M >> Memory Soft Limit: Unlimited >> Memory & Swap Limit: 50.00M >> Maximum Processes Limit: 4194305 > > xpbob has updated the pull request incrementally with one additional commit > since the last revision: > > revert file Changes requested by sgehwolf (Reviewer). test/hotspot/jtreg/containers/docker/TestMisc.java line 60: > 58: testPrintContainerInfo(); > 59: testPrintContainerInfoActiveProcessorCount(); > 60: testPrintContainerMemoryInfo("100M", "150M"); Again. This test runs unconditionally. `--memory-swappiness` is not supported in cgroups v2. Thus, the test will fail on a cgroups v2 system. You need to only run this test on a cgroups v1 system. Have a look at `test/lib/jdk/test/lib/containers/cgroup/MetricsTester.java` how could could detect this and only run on `cgroupv1` providers. - PR: https://git.openjdk.java.net/jdk/pull/8285
Re: RFR: 8284950: CgroupV1 detection code should consider memory.swappiness [v6]
On Wed, 27 Apr 2022 14:24:50 GMT, Severin Gehwolf wrote: >> xpbob has updated the pull request incrementally with one additional commit >> since the last revision: >> >> revert file > > test/hotspot/jtreg/containers/docker/TestMisc.java line 60: > >> 58: testPrintContainerInfo(); >> 59: testPrintContainerInfoActiveProcessorCount(); >> 60: testPrintContainerMemoryInfo("100M", "150M"); > > Again. This test runs unconditionally. `--memory-swappiness` is not supported > in cgroups v2. Thus, the test will fail on a cgroups v2 system. You need to > only run this test on a cgroups v1 system. Have a look at > `test/lib/jdk/test/lib/containers/cgroup/MetricsTester.java` how could could > detect this and only run on `cgroupv1` providers. On my cgroups v2 system (i.e. it's using 200m memory + 200m swap): $ sudo docker run --rm -ti --memory-swappiness=0 --memory=200m fedora:35 WARNING: Your kernel does not support memory swappiness capabilities or the cgroup is not mounted. Memory swappiness discarded. [root@fb3182c8e23c /]# cat /sys/fs/cgroup/memory.max 209715200 [root@fb3182c8e23c /]# cat /sys/fs/cgroup/memory.swap.max 209715200 - PR: https://git.openjdk.java.net/jdk/pull/8285
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman wrote: >> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which >> JDK version to target. >> >> We will refresh this PR periodically to pick up changes and fixes from the >> loom repo. >> >> Most of the new mechanisms in the HotSpot VM are disabled by default and >> require running with `--enable-preview` to enable. >> >> The patch has support for x64 and aarch64 on the usual operating systems >> (Linux, macOS, and Windows). There are stubs (calling Unimplemented) for >> zero and some of the other ports. Additional ports can be contributed via >> PRs against the fibers branch in the loom repo. >> >> There are changes in many areas. To reduce notifications/mails, the labels >> have been trimmed down for now to hotspot, serviceability and core-libs. >> We'll add the complete set of labels when the PR is further along. >> >> The changes include a refresh of java.util.concurrent and ForkJoinPool from >> Doug Lea's CVS. These changes will probably be proposed and integrated in >> advance of this PR. >> >> The changes include some non-exposed and low-level infrastructure to support >> the (in draft) JEPs for Structured Concurrency and Scope Locals. This is to >> make life a bit easier and avoid having to separate VM changes and juggle >> branches at this time. > > Alan Bateman has updated the pull request incrementally with one additional > commit since the last revision: > > Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e Thanks for the refresh Alan! Things look good to me now. I have gone over java.io, the networking parts of java.nio, java.net, and the JMX related changes. For what I have reviewed, I believe this is good to go. - Marked as reviewed by dfuchs (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which > JDK version to target. > > We will refresh this PR periodically to pick up changes and fixes from the > loom repo. > > Most of the new mechanisms in the HotSpot VM are disabled by default and > require running with `--enable-preview` to enable. > > The patch has support for x64 and aarch64 on the usual operating systems > (Linux, macOS, and Windows). There are stubs (calling Unimplemented) for zero > and some of the other ports. Additional ports can be contributed via PRs > against the fibers branch in the loom repo. > > There are changes in many areas. To reduce notifications/mails, the labels > have been trimmed down for now to hotspot, serviceability and core-libs. > We'll add the complete set of labels when the PR is further along. > > The changes include a refresh of java.util.concurrent and ForkJoinPool from > Doug Lea's CVS. These changes will probably be proposed and integrated in > advance of this PR. > > The changes include some non-exposed and low-level infrastructure to support > the (in draft) JEPs for Structured Concurrency and Scope Locals. This is to > make life a bit easier and avoid having to separate VM changes and juggle > branches at this time. Alan Bateman has updated the pull request incrementally with one additional commit since the last revision: Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e - Changes: - all: https://git.openjdk.java.net/jdk/pull/8166/files - new: https://git.openjdk.java.net/jdk/pull/8166/files/0864cb09..f2ed4f9c Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8166=07 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8166=06-07 Stats: 505 lines in 15 files changed: 139 ins; 189 del; 177 mod Patch: https://git.openjdk.java.net/jdk/pull/8166.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8166/head:pull/8166 PR: https://git.openjdk.java.net/jdk/pull/8166
Integrated: 8285440: Typo in Collections.addAll method javadoc
On Fri, 31 Dec 2021 18:58:43 GMT, Johnny Lim wrote: > This PR fixes a typo. This pull request has now been integrated. Changeset: 4919525d Author:Johnny Lim Committer: Jaikiran Pai URL: https://git.openjdk.java.net/jdk/commit/4919525ddb55ba52d199a37c3b0e14e4a0c7c738 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod 8285440: Typo in Collections.addAll method javadoc Reviewed-by: jpai, rriggs - PR: https://git.openjdk.java.net/jdk/pull/6942
Re: RFR: 8285452: Support new API to replace a file content using FileUtils.java [v3]
On Fri, 22 Apr 2022 14:35:14 GMT, Sibabrata Sahoo wrote: >> A new API to support replacing selective lines with desired content. > > Sibabrata Sahoo has updated the pull request incrementally with one > additional commit since the last revision: > > Update FileUtils.java I've updated the title of the bug. Siba can update the PR title. You're right that it's not easy to discover such APIs. We put it in `FileUtils` hoping people who does not want to write their own method will search from there first. As for the API itself, we've imagined something like FileUtils.patch(inputFile) .with(1, 10, List.of(lines), List.of(newLines)) .with(100, 100, linesAsAString, newLinesAsAString) .writeTo(outputFile) but the current form is the simplest case and could be kept even if we have a verbose one. - PR: https://git.openjdk.java.net/jdk/pull/8360
Re: RFR: 8285452: Support new API to replace a file content using FileUtils.java [v3]
On Fri, 22 Apr 2022 14:35:14 GMT, Sibabrata Sahoo wrote: >> A new API to support replacing selective lines with desired content. > > Sibabrata Sahoo has updated the pull request incrementally with one > additional commit since the last revision: > > Update FileUtils.java Please change the issue/pr title to indicate it is a new API in the **test** library. The problem with single purpose APIs with not enough forethought is that they are not discoverable in the library and fall into disuse or are not appropriate for someone else to find and use. - PR: https://git.openjdk.java.net/jdk/pull/8360
Re: RFR: 8285452: Support new API to replace a file content using FileUtils.java [v3]
On Fri, 22 Apr 2022 14:35:14 GMT, Sibabrata Sahoo wrote: >> A new API to support replacing selective lines with desired content. > > Sibabrata Sahoo has updated the pull request incrementally with one > additional commit since the last revision: > > Update FileUtils.java Currently it's just for one test, but it's the exact kind of method that should go into the test library, i.e. a general purpose file manipulation utility that can be useful by everyone. `patch` overwrites the input file (by default) too. We can always enhance it to support `-o`. - PR: https://git.openjdk.java.net/jdk/pull/8360
Re: RFR: 8285452: Support new API to replace a file content using FileUtils.java [v3]
On Fri, 22 Apr 2022 14:35:14 GMT, Sibabrata Sahoo wrote: >> A new API to support replacing selective lines with desired content. > > Sibabrata Sahoo has updated the pull request incrementally with one > additional commit since the last revision: > > Update FileUtils.java If it is just for one/few tests, it would fit better in the directory with the test or in the test itself. This doesn't seem like enough of a general purpose function to be in the test library. Overwriting the input file seems like a bad choice, it will be a trap for someone. It prevents the use of the function in a case where the output is written to a different file. - PR: https://git.openjdk.java.net/jdk/pull/8360
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v7]
On Tue, 26 Apr 2022 17:27:35 GMT, Alan Bateman wrote: >> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which >> JDK version to target. >> >> We will refresh this PR periodically to pick up changes and fixes from the >> loom repo. >> >> Most of the new mechanisms in the HotSpot VM are disabled by default and >> require running with `--enable-preview` to enable. >> >> The patch has support for x64 and aarch64 on the usual operating systems >> (Linux, macOS, and Windows). There are stubs (calling Unimplemented) for >> zero and some of the other ports. Additional ports can be contributed via >> PRs against the fibers branch in the loom repo. >> >> There are changes in many areas. To reduce notifications/mails, the labels >> have been trimmed down for now to hotspot, serviceability and core-libs. >> We'll add the complete set of labels when the PR is further along. >> >> The changes include a refresh of java.util.concurrent and ForkJoinPool from >> Doug Lea's CVS. These changes will probably be proposed and integrated in >> advance of this PR. >> >> The changes include some non-exposed and low-level infrastructure to support >> the (in draft) JEPs for Structured Concurrency and Scope Locals. This is to >> make life a bit easier and avoid having to separate VM changes and juggle >> branches at this time. > > Alan Bateman has updated the pull request incrementally with one additional > commit since the last revision: > > Refresh 8d8f0a2fd646e57fe6b4e8ab669f836dc46dda69 test/jdk/java/net/vthread/HttpALot.java line 76: > 74: var address = server.getAddress(); > 75: URL url = new URL("http://; + address.getHostName() + ":" + > address.getPort() + "/hello"); > 76: Nit: Ideally we should use the URIBuilder from the test library here, and the IP literal address to improve stability of the test on machines that may have strange /etc/hosts configuration. This can be taken care of after this PR has been integrated. test/jdk/java/net/vthread/InterruptHttp.java line 88: > 86: InetAddress lb = InetAddress.getLoopbackAddress(); > 87: listener = new ServerSocket(0, -1, lb); > 88: address = "http://; + lb.getHostAddress() + ":" + > listener.getLocalPort(); Same remark about using URIBuilder here. It would take care of properly formatting the address in case of IPv6 literals. This can be taken care of after this PR has been integrated. - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v7]
On Wed, 27 Apr 2022 11:34:51 GMT, Daniel Fuchs wrote: >> Alan Bateman has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Refresh 8d8f0a2fd646e57fe6b4e8ab669f836dc46dda69 > > src/jdk.management/share/classes/com/sun/management/HotSpotDiagnosticMXBean.java > line 149: > >> 147: * access to the file or {@link >> java.lang.management.ManagementPermission >> 148: * ManagementPermission("control")} is denied >> 149: * @since 19 > > Maybe there ought to be an `@throws UnsupportedOperationException` here since > that is what the default implementation of the method is supposed to do. Well spotted. The implSpec further up documents it but it should be in the throws list too. > test/jdk/java/net/vthread/HttpALot.java line 76: > >> 74: var address = server.getAddress(); >> 75: URL url = new URL("http://; + address.getHostName() + ":" + >> address.getPort() + "/hello"); >> 76: > > Nit: Ideally we should use the URIBuilder from the test library here, and the > IP literal address to improve stability of the test on machines that may have > strange /etc/hosts configuration. This can be taken care of after this PR has > been integrated. A few of these tests started out as standalone classes that could be run without test infrastructure. This one, and InterruptHttp, can be easily changed to use the URIBuilder infra library, so we can do that. - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8285452: Support new API to replace a file content using FileUtils.java [v3]
On Fri, 22 Apr 2022 14:35:14 GMT, Sibabrata Sahoo wrote: >> A new API to support replacing selective lines with desired content. > > Sibabrata Sahoo has updated the pull request incrementally with one > additional commit since the last revision: > > Update FileUtils.java Or we can provide an example in the method spec. - PR: https://git.openjdk.java.net/jdk/pull/8360
Re: RFR: 8282191: Implementation of Foreign Function & Memory API (Preview) [v33]
> This PR contains the API and implementation changes for JEP-424 [1]. A more > detailed description of such changes, to avoid repetitions during the review > process, is included as a separate comment. > > [1] - https://openjdk.java.net/jeps/424 Maurizio Cimadamore has updated the pull request incrementally with one additional commit since the last revision: Simplify libraryLookup impl - Changes: - all: https://git.openjdk.java.net/jdk/pull/7888/files - new: https://git.openjdk.java.net/jdk/pull/7888/files/6990183f..5866bbb5 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7888=32 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7888=31-32 Stats: 21 lines in 1 file changed: 6 ins; 11 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/7888.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7888/head:pull/7888 PR: https://git.openjdk.java.net/jdk/pull/7888
Re: RFR: 8282191: Implementation of Foreign Function & Memory API (Preview) [v32]
> This PR contains the API and implementation changes for JEP-424 [1]. A more > detailed description of such changes, to avoid repetitions during the review > process, is included as a separate comment. > > [1] - https://openjdk.java.net/jeps/424 Maurizio Cimadamore has updated the pull request incrementally with one additional commit since the last revision: Address CSR comments - Changes: - all: https://git.openjdk.java.net/jdk/pull/7888/files - new: https://git.openjdk.java.net/jdk/pull/7888/files/746de5ce..6990183f Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7888=31 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7888=30-31 Stats: 114 lines in 9 files changed: 36 ins; 0 del; 78 mod Patch: https://git.openjdk.java.net/jdk/pull/7888.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7888/head:pull/7888 PR: https://git.openjdk.java.net/jdk/pull/7888
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v7]
On Tue, 26 Apr 2022 17:27:35 GMT, Alan Bateman wrote: >> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which >> JDK version to target. >> >> We will refresh this PR periodically to pick up changes and fixes from the >> loom repo. >> >> Most of the new mechanisms in the HotSpot VM are disabled by default and >> require running with `--enable-preview` to enable. >> >> The patch has support for x64 and aarch64 on the usual operating systems >> (Linux, macOS, and Windows). There are stubs (calling Unimplemented) for >> zero and some of the other ports. Additional ports can be contributed via >> PRs against the fibers branch in the loom repo. >> >> There are changes in many areas. To reduce notifications/mails, the labels >> have been trimmed down for now to hotspot, serviceability and core-libs. >> We'll add the complete set of labels when the PR is further along. >> >> The changes include a refresh of java.util.concurrent and ForkJoinPool from >> Doug Lea's CVS. These changes will probably be proposed and integrated in >> advance of this PR. >> >> The changes include some non-exposed and low-level infrastructure to support >> the (in draft) JEPs for Structured Concurrency and Scope Locals. This is to >> make life a bit easier and avoid having to separate VM changes and juggle >> branches at this time. > > Alan Bateman has updated the pull request incrementally with one additional > commit since the last revision: > > Refresh 8d8f0a2fd646e57fe6b4e8ab669f836dc46dda69 src/jdk.management/share/classes/com/sun/management/HotSpotDiagnosticMXBean.java line 149: > 147: * access to the file or {@link > java.lang.management.ManagementPermission > 148: * ManagementPermission("control")} is denied > 149: * @since 19 Maybe there ought to be an `@throws UnsupportedOperationException` here since that is what the default implementation of the method is supposed to do. - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284950: CgroupV1 detection code should consider memory.swappiness [v6]
On Wed, 27 Apr 2022 10:34:33 GMT, xpbob wrote: >> set memory.swappiness to 0,swap space will not be used >> determine the value of memory.swappiness >> https://www.kernel.org/doc/Documentation/cgroup-v1/memory.txt >> >> >> Memory Limit: 50.00M >> Memory Soft Limit: Unlimited >> Memory & Swap Limit: 100.00M >> Maximum Processes Limit: 4194305 >> >> => >> >> Memory Limit: 50.00M >> Memory Soft Limit: Unlimited >> Memory & Swap Limit: 50.00M >> Maximum Processes Limit: 4194305 > > xpbob has updated the pull request incrementally with one additional commit > since the last revision: > > revert file I refer to the content of this test https://github.com/openjdk/jdk/blob/0f81d8fcc3fb703760b1cddb01861ea5031023fb/test/hotspot/jtreg/containers/docker/TestMemoryAwareness.java#L119 - PR: https://git.openjdk.java.net/jdk/pull/8285
Re: RFR: 8284950: CgroupV1 detection code should consider memory.swappiness [v6]
On Wed, 27 Apr 2022 10:34:33 GMT, xpbob wrote: >> set memory.swappiness to 0,swap space will not be used >> determine the value of memory.swappiness >> https://www.kernel.org/doc/Documentation/cgroup-v1/memory.txt >> >> >> Memory Limit: 50.00M >> Memory Soft Limit: Unlimited >> Memory & Swap Limit: 100.00M >> Maximum Processes Limit: 4194305 >> >> => >> >> Memory Limit: 50.00M >> Memory Soft Limit: Unlimited >> Memory & Swap Limit: 50.00M >> Maximum Processes Limit: 4194305 > > xpbob has updated the pull request incrementally with one additional commit > since the last revision: > > revert file Thanks for review. @iklam @jerboaa I added test to TestMisc with print_container_info and -XshowSettings:system - PR: https://git.openjdk.java.net/jdk/pull/8285
Re: RFR: JDK-8285676: Add missing @param tags for type parameters on classes and interfaces
On Tue, 26 Apr 2022 22:24:26 GMT, Joe Darcy wrote: > To enable more complete doclint checking (courtesy @jonathan-gibbons), please > review this PR to add type-level @param tags where they are missing. > > To the maintainers of java.util.concurrent, those changes could be separated > out in another bug if that would ease maintenance of that code. > > Making these library fixes is a blocker for correcting and expanding the > doclint checks (JDK-8285496). > > I'll update copyright years before pushing. I note that many years ago you filed JDK-6327933, which I'm currently looking at. If implemented, many of the issues from this PR will be addressed automatically, including those in `java.util.concurrent`. - PR: https://git.openjdk.java.net/jdk/pull/8410
Re: RFR: JDK-8285676: Add missing @param tags for type parameters on classes and interfaces
On Tue, 26 Apr 2022 22:24:26 GMT, Joe Darcy wrote: > To enable more complete doclint checking (courtesy @jonathan-gibbons), please > review this PR to add type-level @param tags where they are missing. > > To the maintainers of java.util.concurrent, those changes could be separated > out in another bug if that would ease maintenance of that code. > > Making these library fixes is a blocker for correcting and expanding the > doclint checks (JDK-8285496). > > I'll update copyright years before pushing. Hi Joe, just two suggestions about the javax.management changes. Otherwise looks good! src/java.management/share/classes/javax/management/openmbean/ArrayType.java line 96: > 94: * > 95: * @param the Java type that instances described by this type must > 96: * have. Instead of "instances" - would it be more correct to say "array elements"? src/java.management/share/classes/javax/management/openmbean/SimpleType.java line 60: > 58: * @param the Java type that instances described by this type must > 59: * have. > 60: * I would suggest to say "that values described by this type"... - PR: https://git.openjdk.java.net/jdk/pull/8410
RE: System.exit deadlock if called within a hook
Ok, Yes that would be hard to track down hooks "blocked" either directly or indirectly that way. Indirect blocking case are the hardest one, like a hook joining another thread that is blocked calling System.exit. This hook might really be blocked if joining again and again regardless of any join timeout/join interruption, and maybe just delayed because it joined with a timeout without retrying after a timeout: in both case, at some point, both logic (the blocked and not-so-blocked ones) have the same kind of stack and inter-thread join sequence. So scanning on join-chains is not reliable. So detecting blocked hooks is not either. I might have overlooked other solutions, but I think keeping the blocked threads alive and try to be smart at having the hook detected/handle them is a deadend. Having a thread calling System.exit while System.exit is already in progress being able to "exit" itself (like pthread_exit for posix threads for exemple) would also solve those hook joining blocked thread scenario. Having a thread being able to cease execution change what could be expected from the documented behavior. In term of execution flow, having the thread blocked indefinitely on a monitor enter or have it cease execution is the same : it won't run any other piece of code. Calling Thread.stop while on threads trying to monitor enter the synchronized block has no effect: they need to "enter" the monitor before being able to see the ThreadDeath error to throw. The thread running inside the synchronized block will never do a monitor exit since it calls halt within the synchronized boundaries. The difference between blocked versus cease is the garbage collector behavior: the blocked thread is still alive, still holds refs that cannot be GCed. the thread that cease execution allow objects to be GCed. Even if I don't think that would really be an issue, I still have to mention it. By the way, the hook calling System.exit might be tricky to see/avoid. My demo code is simple but I got this more or less in this kind of execution flow public static void main(String[] args) { Thread.setDefaultUncaughtExceptionHandler((th, trw)->{ /* if trw is considered a fatal throwable then ask for a graceful JVM shutdown */ System.exit(1); /* OOM errors often screw too many things, leaving not-daemon thread hanging around, preventing JVM standard shutdown without either a System.exit or a Runtime.halt, so I think OOM errors are good candidates for being fatal throwables */ }); Runtime.getRuntime().addShutdownHook(new Thread("hook") { public void run() { // the hook does not need to do a hard work to throw an OOM, if running inside a saturated JVM, just a new String to log some message will do throw new OutOfMemoryError("error in hook"); } }); } Regards, Remi Orange Restricted -Message d'origine- De : David Holmes Envoyé : mardi 26 avril 2022 01:24 À : CATHERINOT Rémi DTSI/PFS ; core-libs-dev@openjdk.java.net Objet : Re: System.exit deadlock if called within a hook On 25/04/2022 11:08 pm, remi.catheri...@orange.com wrote: > Hi, > > Exemple code to deadlock the JVM. Only Runtime.halt from within the JVM or a > SIGKILL from outside stops it. Normal kills and Ctrl-C (which is a SIGTERM) > fail to do so. > > public class ExitInHookDemo { > public static void main(String...args) { > Runtime.getRuntime().addShutdownHook(new > Thread("hook") { > public void run() { > System.exit(1); } }); > } > } This is a documented usage limitation of exit(). " All registered shutdown hooks, if any, are started in some unspecified order and allowed to run concurrently until they finish. Once this is done the virtual machine halts. If this method is invoked after all shutdown hooks have already been run and the status is nonzero then this method halts the virtual machine with the given status code. Otherwise, this method blocks indefinitely. " Here you have a hook that calls exit() before all hooks have been run, hence it blocks indefinitely. As the hook blocks indefinitely the running of hooks cannot finish as required and so the virtual machine does not halt. Conceptually you are asking for the indefinitely blocked hook to be treated as-if "finished", but there is no actual mechanism to implement that. You don't need to "destroy" the thread to force it to be "finished", but you do need a way for exit() to check if the current thread is a hook and to adapt the logic so that thread.join() is not used to wait for true termination. It would be possible to construct something I think, but I don't know if such an enhancement would be considered worthwhile versus don't call exit() from a hook. Cheers, David - > I've tried to more or less do a pure java patch in java.lang.Shutdown > but the only way to have a real patch would be to