Re: RFR: 8288414: Long::compress/expand samples are not correct
On Tue, 14 Jun 2022 16:28:37 GMT, Paul Sandoz wrote: > Update the code examples in the api notes of Long::compress/expand. Some > constants need to be explicitly long values. Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.org/jdk19/pull/14
Re: RFR: JDK-8288207: Enhance MalformedURLException in Uri.parseCompat [v4]
On Tue, 14 Jun 2022 12:18:52 GMT, Matthias Baesken wrote: >> When trying to construct an LdapURL object with a bad input string (in this >> example the _ in ad_jbs is causing issues), and not using >> the backward compatibility flag -Dcom.sun.jndi.ldapURLParsing="legacy" we >> run into the exception below : >> >> import com.sun.jndi.ldap.LdapURL; >> >> String url = "ldap://ad_jbs.ttt.net:389/xyz;; // bad input string containing >> _ >> LdapURL ldapUrl = new LdapURL(url); >> >> >> java --add-opens java.naming/com.sun.jndi.ldap=ALL-UNNAMED LdapParseUrlTest >> Exception in thread "main" javax.naming.NamingException: Cannot parse url: >> ldap://ad_jbs.ttt.net:389/xyz [Root exception is >> java.net.MalformedURLException: unsupported authority: ad_jbs.ttt.net:389] >> at java.naming/com.sun.jndi.ldap.LdapURL.(LdapURL.java:115) >> at LdapParseUrlTest.main(LdapParseUrlTest.java:9) >> Caused by: java.net.MalformedURLException: unsupported authority: >> ad_jbs.ttt.net:389 >> at java.naming/com.sun.jndi.toolkit.url.Uri.parseCompat(Uri.java:367) >> at java.naming/com.sun.jndi.toolkit.url.Uri.parse(Uri.java:230) >> at java.naming/com.sun.jndi.toolkit.url.Uri.init(Uri.java:174) >> at java.naming/com.sun.jndi.ldap.LdapURL.(LdapURL.java:105) >> >> I would like to add the host and port info to the exception (in the example >> it is host:port of URI:null:-1] ) so that it is directly visible that the >> input caused the construction of a URI >> with "special"/problematic host and port values. > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > avoid very long line Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.org/jdk/pull/9126
Re: Handling of USR2 signal in JVM on Linux
On 14/06/2022 10:44, Andrey Turbanov wrote: Hello. During investigation of signal handling in JVM (for https://github.com/openjdk/jdk/pull/9100#discussion_r894992558 ) I found out that sending USR2 crashes my JDK. (Linux fastdebug x64) kill -USR2 1346792 # assert(thread != __null) failed: Missing current thread in SR_handler # Internal Error (/home/turbanoff/Projects/official_jdk/src/hotspot/os/posix/signals_posix.cpp:1600), pid=1346792, tid=1346792 Full hs_err_pid1346792.log: https://gist.github.com/turbanoff/2099327ea13357a90df43a2d6b0e2e6a Is it known/expected behaviour? I found some description there https://docs.oracle.com/en/java/javase/11/troubleshoot/handle-signals-and-exceptions.html that USR2 is used for SUSPEND/RESUME. Is it supported by Hotspot? In general you have to be very careful when using signals. Yes, it can easily break things and probably notice it quickly with debug builds as asserts are compiled in to the builds (like the above). So I think you've found the right page to read up on this. In this case, you can set _JAVA_SR_SIGNUM to specify a different signal for S/R. -Alan
Re: RFR: JDK-8288207: Enhance MalformedURLException in Uri.parseCompat [v3]
On Tue, 14 Jun 2022 11:36:36 GMT, Matthias Baesken wrote: >> When trying to construct an LdapURL object with a bad input string (in this >> example the _ in ad_jbs is causing issues), and not using >> the backward compatibility flag -Dcom.sun.jndi.ldapURLParsing="legacy" we >> run into the exception below : >> >> import com.sun.jndi.ldap.LdapURL; >> >> String url = "ldap://ad_jbs.ttt.net:389/xyz;; // bad input string containing >> _ >> LdapURL ldapUrl = new LdapURL(url); >> >> >> java --add-opens java.naming/com.sun.jndi.ldap=ALL-UNNAMED LdapParseUrlTest >> Exception in thread "main" javax.naming.NamingException: Cannot parse url: >> ldap://ad_jbs.ttt.net:389/xyz [Root exception is >> java.net.MalformedURLException: unsupported authority: ad_jbs.ttt.net:389] >> at java.naming/com.sun.jndi.ldap.LdapURL.(LdapURL.java:115) >> at LdapParseUrlTest.main(LdapParseUrlTest.java:9) >> Caused by: java.net.MalformedURLException: unsupported authority: >> ad_jbs.ttt.net:389 >> at java.naming/com.sun.jndi.toolkit.url.Uri.parseCompat(Uri.java:367) >> at java.naming/com.sun.jndi.toolkit.url.Uri.parse(Uri.java:230) >> at java.naming/com.sun.jndi.toolkit.url.Uri.init(Uri.java:174) >> at java.naming/com.sun.jndi.ldap.LdapURL.(LdapURL.java:105) >> >> I would like to add the host and port info to the exception (in the example >> it is host:port of URI:null:-1] ) so that it is directly visible that the >> input caused the construction of a URI >> with "special"/problematic host and port values. > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > fix copy paste error src/java.naming/share/classes/com/sun/jndi/toolkit/url/Uri.java line 368: > 366: // throw if we have user info or regname > 367: throw new MalformedURLException("Authority > component is not server-based, or contains user info. Unsupported authority: > " + auth); > 368: } This looks okay but you may have to split up the line to avoid adding a 150+ char line (most of the file seems to keep the lines under 100 or so). - PR: https://git.openjdk.org/jdk/pull/9126
RFR: 8286176: Add JNI_VERSION_19 to jni.h and JNI spec
JNI is updated in Java 19 so we need to define JNI_VERSION_19 and change GetVersion to return this version. test/hotspot/jtreg/native_sanity/JniVersion.java is updated to check that JNI_VERSION_19 is returned. The native library in the JMX agent, and several tests, define JNI_OnLoad that return JNI_VERSION_10 as the JNI version needed. These are updated to return JNI_VERSION_19 although these updates aren't strictly needed. - Commit messages: - Initial commit Changes: https://git.openjdk.org/jdk19/pull/7/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk19=7=00 Issue: https://bugs.openjdk.org/browse/JDK-8286176 Stats: 16 lines in 10 files changed: 2 ins; 0 del; 14 mod Patch: https://git.openjdk.org/jdk19/pull/7.diff Fetch: git fetch https://git.openjdk.org/jdk19 pull/7/head:pull/7 PR: https://git.openjdk.org/jdk19/pull/7
Re: RFR: 8279047: Remove expired flags in JDK 20
On Fri, 10 Jun 2022 13:23:51 GMT, David Holmes wrote: > Expired Flags in 20: > > - FilterSpuriousWakeups > - MinInliningThreshold > - PrefetchFieldsAhead > > No remaining usages in code or tests. > > - UseHeavyMonitors (expired in PRODUCT ONLY) > > As this flag now only exists for debug builds it has to be a "develop" flag > rather than product. There are then changes to two tests that use this flag, > so that they only run on a debug VM. > > - test/jdk/java/lang/Thread/virtual/HoldsLock.java > > The second @run that uses UseHeavyMonitors is moved to a second test segment > that only applies to the debug VM. > > - test/jdk/java/util/concurrent/ConcurrentHashMap/MapLoops.java > > Change the test section that uses UseHeavyMonitors to only run on a debug VM > and remove the IgnoreUnrecognizedOptions flag. Note that the existing test > logic would run the same test twice on product builds. > > No documentation in the manpage exists for any of the newly expired flags. > > Obsoleted flags in 20: > > - ExtendedDTraceProbes > > Documented in manpage so moved from Deprecated section to Obsolete section. > There is special handling for messages about use of this flag so that won't > be updated until the flag is actually obsoleted (JDK-8279913). > > - UseContainerCpuShares > - PreferContainerQuotaForCPUCount > - AliasLevel > > Undocumented. > > Java manpage updates: > - Updates Java version to 20 > - Moved ExtendedDTraceProbe info as previously mentioned > - Applied changes from the .1 file that had not been applied to the markdown > source so that they were not lost (and note the .1 file was also missing > changes from the markdown file that had not been propagated). > - Removed an unrepresentable character (the section symbol) that was not > being generated into either the html or nroff file correctly Marked as reviewed by alanb (Reviewer). test/jdk/java/lang/Thread/virtual/HoldsLock.java line 39: > 37: * @modules java.base/java.lang:+open > 38: * @compile --enable-preview -source ${jdk.version} HoldsLock.java > 39: * @run testng/othervm --enable-preview -XX:+UseHeavyMonitors HoldsLock The test updates look okay, probably don't need to duplicate the summary tag when it's unchanged but what you have is okay. - PR: https://git.openjdk.org/jdk/pull/9127
Re: RFR: 8287760: --do-not-resolve-by-default gets overwritten if --warn-if-resolved flags is used [v8]
On Fri, 10 Jun 2022 18:19:42 GMT, Thiago Henrique Hüpner wrote: >> test/jdk/tools/jar/modularJar/Basic.java line 44: >> >>> 42: >>> 43: import jdk.internal.module.ModuleReferenceImpl; >>> 44: import jdk.internal.module.ModuleResolution; >> >> At some point we need to put in test infrastructure to avoid this and a few >> other tests from depending on JDK internals. > > Do you think it is better to parse the output of `javap` instead of using the > internals to detect if it worked? Parsing the output of javap would be fragile. Instead, I think we'll just add some test infrastructure at some point, maybe when this class file attribute is documented somewhere. - PR: https://git.openjdk.org/jdk/pull/9049
Re: RFR: 8287760: --do-not-resolve-by-default gets overwritten if --warn-if-resolved flags is used [v9]
On Fri, 10 Jun 2022 18:25:09 GMT, Thiago Henrique Hüpner wrote: >> 8287760: --do-not-resolve-by-default gets overwritten if --warn-if-resolved >> flags is used > > Thiago Henrique Hüpner has updated the pull request incrementally with one > additional commit since the last revision: > > Rename dataprovider Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.org/jdk/pull/9049
Re: RFR: 8287760: --do-not-resolve-by-default gets overwritten if --warn-if-resolved flags is used [v8]
On Fri, 10 Jun 2022 16:30:57 GMT, Thiago Henrique Hüpner wrote: >> 8287760: --do-not-resolve-by-default gets overwritten if --warn-if-resolved >> flags is used > > Thiago Henrique Hüpner has updated the pull request incrementally with one > additional commit since the last revision: > > Fix copyright year test/jdk/tools/jar/modularJar/Basic.java line 44: > 42: > 43: import jdk.internal.module.ModuleReferenceImpl; > 44: import jdk.internal.module.ModuleResolution; At some point we need to put in test infrastructure to avoid this and a few other tests from depending on JDK internals. test/jdk/tools/jar/modularJar/Basic.java line 951: > 949: > 950: @DataProvider(name = "resolutionNames") > 951: public Object[][] resolutionNames() { This is a data provider for resolution warnings, maybe it should be renamed. - PR: https://git.openjdk.org/jdk/pull/9049
Re: RFR: JDK-8288227: Refactor annotation implementation to use pattern matching for instanceof
On Fri, 10 Jun 2022 16:15:36 GMT, Joe Darcy wrote: > There are many instanceof checks in the sun.reflection.annotation code; these > would be improved by using pattern matching for instanceof. Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.org/jdk/pull/9129
Re: RFR: JDK-8288207: Enhance MalformedURLException in Uri.parseCompat
On Fri, 10 Jun 2022 13:41:48 GMT, Matthias Baesken wrote: > Hi Alan , sure we could use something like the already existing hostInfo of > property jdk.includeInException private static final boolean > enhancedExceptionText = SecurityProperties.includedInExceptions("hostInfo"); > and make the enhancement optional/switchable this way. On the other hand we > already print the url (_**Cannot parse url: ldap://ad_jbs.ttt.net:389/xyz**_ > ) in the existing exception text so I wonder what additional problem the > added info would bring? That's why I did not use the property so far. But if > you think there could be special cases were it would be problematic to have > the enhancement, I'll add the usage of the property. This is a security sensitive area and not possible to discuss all issues in JBS or in this PR. If this code is changed then it will require someone from security-dev to review. - PR: https://git.openjdk.org/jdk/pull/9126
Re: RFR: JDK-8288207: Enhance MalformedURLException in Uri.parseCompat
On Fri, 10 Jun 2022 12:16:17 GMT, Matthias Baesken wrote: > When trying to construct an LdapURL object with a bad input string (in this > example the _ in ad_jbs is causing issues), and not using > the backward compatibility flag -Dcom.sun.jndi.ldapURLParsing="legacy" we run > into the exception below : > > import com.sun.jndi.ldap.LdapURL; > > String url = "ldap://ad_jbs.ttt.net:389/xyz;; // bad input string containing _ > LdapURL ldapUrl = new LdapURL(url); > > > java --add-opens java.naming/com.sun.jndi.ldap=ALL-UNNAMED LdapParseUrlTest > Exception in thread "main" javax.naming.NamingException: Cannot parse url: > ldap://ad_jbs.ttt.net:389/xyz [Root exception is > java.net.MalformedURLException: unsupported authority: ad_jbs.ttt.net:389] > at java.naming/com.sun.jndi.ldap.LdapURL.(LdapURL.java:115) > at LdapParseUrlTest.main(LdapParseUrlTest.java:9) > Caused by: java.net.MalformedURLException: unsupported authority: > ad_jbs.ttt.net:389 > at java.naming/com.sun.jndi.toolkit.url.Uri.parseCompat(Uri.java:367) > at java.naming/com.sun.jndi.toolkit.url.Uri.parse(Uri.java:230) > at java.naming/com.sun.jndi.toolkit.url.Uri.init(Uri.java:174) > at java.naming/com.sun.jndi.ldap.LdapURL.(LdapURL.java:105) > > I would like to add the host and port info to the exception (in the example > it is host:port of URI:null:-1] ) so that it is directly visible that the > input caused the construction of a URI > with "special"/problematic host and port values. We have to be cautious about leaking security sensitive configuration in exception messages. Can you look at the security property jdk.includeInException (conf/security/java.security) and usages in the JDK for ideas on how this might be implemented as opt-in? - PR: https://git.openjdk.org/jdk/pull/9126
Re: RFR: 8287186: JDK modules participating in preview [v2]
On Wed, 8 Jun 2022 18:24:35 GMT, Paul Sandoz wrote: >> Allow JDK modules that use preview features (preview language features or >> preview API features from dependent modules) to participate without the need >> to compile with `--enable-preview`. >> >> It's difficult to enable participation using an annotation due to the nature >> in which symbols are encountered when processing source as there is no >> guaranteed order to the processing of certain symbols. >> >> Instead a JDK module participates if the `java.base` package >> `jdk.internal.javac` is exported to that module (@lahodaj clever idea!). An >> internal annotation `jdk.internal.javac.ParticipatesInPreview` can be >> declared on the module. Such a declaration cannot be enforced but does by >> its use require the `jdk.internal.javac`'s export list to be updated. >> >> The modules `jdk.incubator.vector` and `jdk.incubator.concurrent` have been >> updated accordingly, both of which participate in preview APIs (APIs in >> `java.lang.foreign` and `Thread.ofVirtual`, respectively). > > Paul Sandoz has updated the pull request incrementally with one additional > commit since the last revision: > > Let java.management participate in preview features. The updates to java.base, java.management, and jdk.incubator.* looks fine, it's good to have the reflection code go away. - Marked as reviewed by alanb (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/9087
Re: RFR: 8287186: JDK modules participating in preview
On Wed, 8 Jun 2022 15:46:24 GMT, Paul Sandoz wrote: > Allow JDK modules that use preview features (preview language features or > preview API features from dependent modules) to participate without the need > to compile with `--enable-preview`. > > It's difficult to enable participation using an annotation due to the nature > in which symbols are encountered when processing source as there is no > guaranteed order to the processing of certain symbols. > > Instead a JDK module participates if the `java.base` package > `jdk.internal.javac` is exported to that module (@lahodaj clever idea!). An > internal annotation `jdk.internal.javac.ParticipatesInPreview` can be > declared on the module. Such a declaration cannot be enforced but does by its > use require the `jdk.internal.javac`'s export list to be updated. > > The modules `jdk.incubator.vector` and `jdk.incubator.concurrent` have been > updated accordingly, both of which participate in preview APIs (APIs in > `java.lang.foreign` and `Thread.ofVirtual`, respectively). Can java.management participate too? It would allow sun.management.Util.isVirtual(Thread) to go away (lots of methods in sun.management.ThreadImpl need to test if a thread is virtual). - PR: https://git.openjdk.java.net/jdk/pull/9087
Re: RFR: 8287760: --do-not-resolve-by-default gets overwritten if --warn-if-resolved flags is used [v5]
On Wed, 8 Jun 2022 14:51:59 GMT, Thiago Henrique Hüpner wrote: >> 8287760: --do-not-resolve-by-default gets overwritten if --warn-if-resolved >> flags is used > > Thiago Henrique Hüpner has updated the pull request incrementally with one > additional commit since the last revision: > > Fix test message test/jdk/tools/jar/modularJar/Basic.java line 1050: > 1048: */ > 1049: @Test(dataProvider = "resolutionNames") > 1050: public void > updateFooModuleResolutionDoNotResolveByDefaultAndWarnIfResolved(String > resolutionName, Predicate hasWarning) throws IOException { Update the existing tests is good but overly long lines make it too hard to read (it will make future side-by-side view impossible). Can you trim all these lines down to something closer to the existing tests in this file? - PR: https://git.openjdk.java.net/jdk/pull/9049
Re: CFV: new Core Libraries Group member: Roger Riggs
Vote: yes
Re: RFR: 8287760: --do-not-resolve-by-default gets overwritten if --warn-if-resolved flags is used
On Tue, 7 Jun 2022 00:45:17 GMT, Thiago Henrique Hüpner wrote: > 8287760: --do-not-resolve-by-default gets overwritten if --warn-if-resolved > flags is used The change looks okay but I think we should add a test. You'll have to check the test tree to see if there are existing tests for incubator modules packages as JAR files, there may not be as it's an unusual setup. - PR: https://git.openjdk.java.net/jdk/pull/9049
Re: CVF: new Core Libraries Group member: Naoto Sato
Vote: yes
Re: RFR: JDK-8287838: Update Float and Double to use snippets [v2]
On Mon, 6 Jun 2022 16:54:29 GMT, Joe Darcy wrote: >> src/java.base/share/classes/java/lang/Double.java line 683: >> >>> 681: * "[\\x00-\\x20]*");// Optional trailing "whitespace" >>> 682: * >>> 683: * if (Pattern.matches(fpRegex, myString)) // @link >>> substring="Pattern.matches" target ="java.util.regex.Pattern#matches" >> >> If you want to avoid the annoyingly long line then you can put the "// @link >> ..." on the previous line if you want. > > It can be done with a snippet region like this: > > > * // @link region substring="Pattern.matches" target > ="java.util.regex.Pattern#matches" > * if (Pattern.matches(fpRegex, myString)) > * Double.valueOf(myString); // Will not throw NumberFormatException > * // @end Yes, that works. The alternative is to terminate the comment line with a colon (:) and that applies markup tag to the line that follows. In this case, it would be: ``` // @link substring="Pattern.matches" target ="java.util.regex.Pattern#matches" : - PR: https://git.openjdk.java.net/jdk/pull/9034
Re: RFR: JDK-8287838: Update Float and Double to use snippets [v2]
On Mon, 6 Jun 2022 20:37:07 GMT, Joe Darcy wrote: >> Various code blocks in Float and Double would be better as snippets. > > Joe Darcy has updated the pull request incrementally with two additional > commits since the last revision: > > - Use idiom for shorter lines > - Respond to review feedback. src/java.base/share/classes/java/lang/Double.java line 683: > 681: * "[\\x00-\\x20]*");// Optional trailing "whitespace" > 682: * > 683: * if (Pattern.matches(fpRegex, myString)) // @link > substring="Pattern.matches" target ="java.util.regex.Pattern#matches" If you want to avoid the annoyingly long line then you can put the "// @link ..." on the previous line if you want. - PR: https://git.openjdk.java.net/jdk/pull/9034
Re: RFR: 8279283 - BufferedInputStream should override transferTo [v5]
On Mon, 27 Dec 2021 13:43:12 GMT, Markus KARG wrote: >> Implementation of JDK-8279283 > > Markus KARG has updated the pull request incrementally with one additional > commit since the last revision: > > fixed missing BufferedInputStream Can this PR be closed or returned to daft? - PR: https://git.openjdk.java.net/jdk/pull/6935
Re: RFR: JDK-8287838: Update Float and Double to use snippets
On Sun, 5 Jun 2022 21:19:37 GMT, Joe Darcy wrote: > Various code blocks in Float and Double would be better as snippets. One other thing you could do is link Pattern.matches in the snippet to the matches method. - Marked as reviewed by alanb (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/9034
Integrated: 8284199: Implementation of Structured Concurrency (Incubator)
On Thu, 19 May 2022 13:05:54 GMT, Alan Bateman wrote: > This is the implementation of JEP 428: Structured Concurrency (Incubator). > > This is a non-final API that provides a gentle on-ramp to structure a task as > a family of concurrent subtasks, and to coordinate the subtasks as a unit. This pull request has now been integrated. Changeset: e4e1e8f6 Author:Alan Bateman URL: https://git.openjdk.java.net/jdk/commit/e4e1e8f66c9b0321cdb1aaf3b1c5d9b67224b210 Stats: 2816 lines in 11 files changed: 2815 ins; 0 del; 1 mod 8284199: Implementation of Structured Concurrency (Incubator) Co-authored-by: Ron Pressler Co-authored-by: Alan Bateman Co-authored-by: Brian Goetz Co-authored-by: Paul Sandoz Reviewed-by: psandoz, mcimadamore, darcy - PR: https://git.openjdk.java.net/jdk/pull/8787
Re: RFR: 8287541: Files.writeString fails to throw IOException for charset "windows-1252"
On Fri, 3 Jun 2022 16:48:46 GMT, Naoto Sato wrote: > The code path calls `String.getBytesNoRepl()`, but it blindly replaces > unmappable characters with replacements if the encoder is an `ArrayEncoder`. > Changed only to do so if `doReplace` is `true` in > `String.encodeWithEncoder()`. Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/9019
Re: RFR: 8287746: ProblemList jni/nullCaller/NullCallerTest.java
On Thu, 2 Jun 2022 18:57:36 GMT, Mandy Chung wrote: > 8287746: ProblemList jni/nullCaller/NullCallerTest.java Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/9002
Re: RFR: 8284199: Implementation of Structured Concurrency (Incubator) [v5]
> This is the implementation of JEP 428: Structured Concurrency (Incubator). > > This is a non-final API that provides a gentle on-ramp to structure a task as > a family of concurrent subtasks, and to coordinate the subtasks as a unit. Alan Bateman 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 12 additional commits since the last revision: - Add test directory to jdk_other so tests run in tier2 - Merge - Fix typo in javadoc - Merge - Javadoc updates, add to jdk_loom test group - Add statement to close about thread termination - Use {@code ...}, replace task->subtask, fix typos, add jls ref - Merge - @ignore StructuredThreadDumpTest until test infra in place - Refresh - ... and 2 more: https://git.openjdk.java.net/jdk/compare/d40ed463...7f48656e - Changes: - all: https://git.openjdk.java.net/jdk/pull/8787/files - new: https://git.openjdk.java.net/jdk/pull/8787/files/d11b24bc..7f48656e Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8787=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8787=03-04 Stats: 46596 lines in 447 files changed: 23349 ins; 17945 del; 5302 mod Patch: https://git.openjdk.java.net/jdk/pull/8787.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8787/head:pull/8787 PR: https://git.openjdk.java.net/jdk/pull/8787
Integrated: 8287496: Alternative virtual thread implementation that maps to OS thread
On Sun, 29 May 2022 14:46:39 GMT, Alan Bateman wrote: > This patch adds an alternative virtual thread implementation where each > virtual thread is backed by an OS thread. It doesn't scale but it can be used > by ports that don't have continuations support in the VM. Aside from > scalability, the lack of continuations support means: > > 1. JVM TI is not supported when running with --enable-preview (the JVM TI > spec allows for this) > 2. jshell --enable-preview can't be used (as jshell uses the debugger APIs > and so needs JVM TI) > > The VM option "VMContinuations" is added as an experimental option so it can > be used by tests. A number of tests are changed to re-run with > -XX:-VMContinuations. A new jtreg property is added so that tests that need > the underlying VM support to be present can use "@requires vm.continuations" > in the test description. A follow-up change would be to add "@requires > vm.continuations" to the ~70 serviceability/jvmti/vthread that run with > preview features enabled. This pull request has now been integrated. Changeset: 6ff2d89e Author:Alan Bateman URL: https://git.openjdk.java.net/jdk/commit/6ff2d89ea11934bb13c8a419e7bad4fd40f76759 Stats: 747 lines in 72 files changed: 574 ins; 53 del; 120 mod 8287496: Alternative virtual thread implementation that maps to OS thread Reviewed-by: rehn, mchung - PR: https://git.openjdk.java.net/jdk/pull/8939
Re: RFR: 8287496: Alternative virtual thread implementation that maps to OS thread [v3]
On Wed, 1 Jun 2022 06:26:23 GMT, David Holmes wrote: >> Alan Bateman has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains 11 commits: >> >> - Fixed another typo in comment >> - Merge >> - Fix typos in comments >> - Allowing linking without intrinsic stub, contributed-by: rehn >> - Continuation clinit should fail if no continuations support >> - Fix merge issue with test >> - Change VMContinuations to be experimental option, ensure JNI GetEnv >> returns EVERSION >> - Update >> - Expend test coverage >> - Merge >> - ... and 1 more: >> https://git.openjdk.java.net/jdk/compare/3deb58a8...a5c06cb6 > > src/java.base/share/classes/java/lang/Thread.java line 742: > >> 740: * @param name thread name, can be null >> 741: * @param characteristics thread characteristics >> 742: * @param bound true when bound to an OS thread > > Nit: s/OS/VM/ ? I think it would be confusing to use "VM" here. The intention is that "true" means the virtual thread will be bound to an OS thread once it has started. Yes, technically it's whatever the VM implements but I think that is too much to try to explain in the parameter description. - PR: https://git.openjdk.java.net/jdk/pull/8939
Re: RFR: 4511638: Double.toString(double) sometimes produces incorrect results [v14]
On Wed, 1 Jun 2022 08:56:38 GMT, Raffaello Giulietti wrote: >> src/java.base/share/classes/jdk/internal/math/MathUtils.java line 38: >> >>> 36: * >>> 37: * Giulietti, "The Schubfach way to render doubles", >>> 38: * >>> https://drive.google.com/file/d/1gp5xv4CAa78SVgCeWfGqqI4FfYYYuNFb >> >> Even though not public, should the reference use the `` tag and >> perhaps be in a `@see` annotation? >> >> @see > href=“https://drive.google.com/file/d/1gp5xv4CAa78SVgCeWfGqqI4FfYYYuNFb”> >> The Schubfach way to render doubles > > These references are inside a normal multi-line comment, so I'm not sure that > Javadoc or html tags or have a well-defined semantics there. I'm not sure about linking to a document on Google Drive, even from JDK internal classes. Is the paper uploaded to anywhere else that could be linked to instead? - PR: https://git.openjdk.java.net/jdk/pull/3402
Re: RFR: 8287496: Alternative virtual thread implementation that maps to OS thread [v3]
> This patch adds an alternative virtual thread implementation where each > virtual thread is backed by an OS thread. It doesn't scale but it can be used > by ports that don't have continuations support in the VM. Aside from > scalability, the lack of continuations support means: > > 1. JVM TI is not supported when running with --enable-preview (the JVM TI > spec allows for this) > 2. jshell --enable-preview can't be used (as jshell uses the debugger APIs > and so needs JVM TI) > > The VM option "VMContinuations" is added as an experimental option so it can > be used by tests. A number of tests are changed to re-run with > -XX:-VMContinuations. A new jtreg property is added so that tests that need > the underlying VM support to be present can use "@requires vm.continuations" > in the test description. A follow-up change would be to add "@requires > vm.continuations" to the ~70 serviceability/jvmti/vthread that run with > preview features enabled. Alan Bateman has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 11 commits: - Fixed another typo in comment - Merge - Fix typos in comments - Allowing linking without intrinsic stub, contributed-by: rehn - Continuation clinit should fail if no continuations support - Fix merge issue with test - Change VMContinuations to be experimental option, ensure JNI GetEnv returns EVERSION - Update - Expend test coverage - Merge - ... and 1 more: https://git.openjdk.java.net/jdk/compare/3deb58a8...a5c06cb6 - Changes: https://git.openjdk.java.net/jdk/pull/8939/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8939=02 Stats: 747 lines in 72 files changed: 574 ins; 53 del; 120 mod Patch: https://git.openjdk.java.net/jdk/pull/8939.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8939/head:pull/8939 PR: https://git.openjdk.java.net/jdk/pull/8939
Re: RFR: 8287496: Alternative virtual thread implementation that maps to OS thread [v2]
On Tue, 31 May 2022 17:03:54 GMT, Aleksey Shipilev wrote: > I expected this change to fix the broken ARM32 port, but it doesn't work. There is work required to get the arm32 port working again, currently tracked as JDK-828636 but there may be further issues beyond that. - PR: https://git.openjdk.java.net/jdk/pull/8939
Re: RFR: 8287496: Alternative virtual thread implementation that maps to OS thread [v2]
> This patch adds an alternative virtual thread implementation where each > virtual thread is backed by an OS thread. It doesn't scale but it can be used > by ports that don't have continuations support in the VM. Aside from > scalability, the lack of continuations support means: > > 1. JVM TI is not supported when running with --enable-preview (the JVM TI > spec allows for this) > 2. jshell --enable-preview can't be used (as jshell uses the debugger APIs > and so needs JVM TI) > > The VM option "VMContinuations" is added as an experimental option so it can > be used by tests. A number of tests are changed to re-run with > -XX:-VMContinuations. A new jtreg property is added so that tests that need > the underlying VM support to be present can use "@requires vm.continuations" > in the test description. A follow-up change would be to add "@requires > vm.continuations" to the ~70 serviceability/jvmti/vthread that run with > preview features enabled. Alan Bateman has updated the pull request incrementally with one additional commit since the last revision: Allowing linking without intrinsic stub, contributed-by: rehn - Changes: - all: https://git.openjdk.java.net/jdk/pull/8939/files - new: https://git.openjdk.java.net/jdk/pull/8939/files/7989708b..126e38b6 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8939=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8939=00-01 Stats: 5 lines in 1 file changed: 4 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/8939.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8939/head:pull/8939 PR: https://git.openjdk.java.net/jdk/pull/8939
Re: RFR: 8287520: Shrink x86_32 problemlists after JDK-8287437
On Mon, 30 May 2022 13:20:17 GMT, Aleksey Shipilev wrote: > [JDK-8287137](https://bugs.openjdk.java.net/browse/JDK-8287137) added a bunch > of tests into problemlist. Those lists basically exclude every test that runs > with --enable-preview. > [JDK-8287437](https://bugs.openjdk.java.net/browse/JDK-8287437) makes it > better: it only disables Loom parts, even with --enable-preview. This allows > testing x86_32 on other preview features and avoids accidental bug slippage > in non-Loom code paths. We can and should shrink the problem lists now. > > Additional testing: > - [x] Removed tests with Linux x86_32 fastdebug; mostly pass, some non-Loom > failures > - [x] Linux x86_32 fastdebug `tier1` > - [ ] GHA run This looks okay. If we get PR8939 in then it should be possible to do more clear out of the exclude lists. - Marked as reviewed by alanb (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8946
Re: RFR: 8287496: Alternative virtual thread implementation that maps to OS thread
On Sun, 29 May 2022 14:46:39 GMT, Alan Bateman wrote: > This patch adds an alternative virtual thread implementation where each > virtual thread is backed by an OS thread. It doesn't scale but it can be used > by ports that don't have continuations support in the VM. Aside from > scalability, the lack of continuations support means: > > 1. JVM TI is not supported when running with --enable-preview (the JVM TI > spec allows for this) > 2. jshell --enable-preview can't be used (as jshell uses the debugger APIs > and so needs JVM TI) > > The VM option "VMContinuations" is added as an experimental option so it can > be used by tests. A number of tests are changed to re-run with > -XX:-VMContinuations. A new jtreg property is added so that tests that need > the underlying VM support to be present can use "@requires vm.continuations" > in the test description. A follow-up change would be to add "@requires > vm.continuations" to the ~70 serviceability/jvmti/vthread that run with > preview features enabled. > Since the package `jdk.internal.access` is > exported[1](#user-content-fn-1-97aea7d7960164849e591e42b91fb5c4) to the > `java.management` module, this can use `MethodHandle`s obtained using the > trusted lookup. That export is for another reason, and probably should be re-examined so that java.base doesn't need to export this package to java.management. In any case, we expect there will be compiler support soon to allow java.management be compiled with code that makes use of preview APIs in java.base. That will at least us reference Thread::isVirtual from code. - PR: https://git.openjdk.java.net/jdk/pull/8939
RFR: 8287496: Alternative virtual thread implementation that maps to OS thread
This patch adds an alternative virtual thread implementation where each virtual thread is backed by an OS thread. It doesn't scale but it can be used by ports that don't have continuations support in the VM. Aside from scalability, the lack of continuations support means: 1. JVM TI is not supported when running with --enable-preview (the JVM TI spec allows for this) 2. jshell --enable-preview can't be used (as jshell uses the debugger APIs and so needs JVM TI) The VM option "VMContinuations" is added as an experimental option so it can be used by tests. A number of tests are changed to re-run with -XX:-VMContinuations. A new jtreg property is added so that tests that need the underlying VM support to be present can use "@requires vm.continuations" in the test description. A follow-up change would be to add "@requires vm.continuations" to the ~70 serviceability/jvmti/vthread that run with preview features enabled. - Commit messages: - Continuation clinit should fail if no continuations support - Fix merge issue with test - Change VMContinuations to be experimental option, ensure JNI GetEnv returns EVERSION - Update - Expend test coverage - Merge - Initial commit Changes: https://git.openjdk.java.net/jdk/pull/8939/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8939=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8287496 Stats: 742 lines in 71 files changed: 570 ins; 53 del; 119 mod Patch: https://git.openjdk.java.net/jdk/pull/8939.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8939/head:pull/8939 PR: https://git.openjdk.java.net/jdk/pull/8939
Re: RFR: 8284199: Implementation of Structured Concurrency (Incubator) [v4]
> This is the implementation of JEP 428: Structured Concurrency (Incubator). > > This is a non-final API that provides a gentle on-ramp to structure a task as > a family of concurrent subtasks, and to coordinate the subtasks as a unit. Alan Bateman 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 10 additional commits since the last revision: - Fix typo in javadoc - Merge - Javadoc updates, add to jdk_loom test group - Add statement to close about thread termination - Use {@code ...}, replace task->subtask, fix typos, add jls ref - Merge - @ignore StructuredThreadDumpTest until test infra in place - Refresh - Merge - Initial commit - Changes: - all: https://git.openjdk.java.net/jdk/pull/8787/files - new: https://git.openjdk.java.net/jdk/pull/8787/files/4fc454a9..d11b24bc Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8787=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8787=02-03 Stats: 33428 lines in 507 files changed: 6180 ins; 25559 del; 1689 mod Patch: https://git.openjdk.java.net/jdk/pull/8787.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8787/head:pull/8787 PR: https://git.openjdk.java.net/jdk/pull/8787
Re: RFR: 8284638: store skip buffers in InputStream Object [v15]
On Mon, 30 May 2022 05:29:01 GMT, Alan Bateman wrote: >> More practically. >> This PR has a noticeable negative effect - it increases the size of >> InputStream objects. Moreover, it increases the size of InputStream >> subclasses which has own skip() implementation and don't need this >> superclass modification. >> >> Let's look into InputStream subclasses. >> I've checked 51 InputStream from classlib. 30 of them either have their own >> skip() implementation or use super.skip() other than from InputStream. This >> PR is definitely harmful to these 30 classes. These 30 classes can be >> divided into the following categories: >> >> 1) Clean non-allocating skip() implementation (22 classes): >> - BufferedInputStream (java.io) >> - ByteArrayInputStream (java.io) >> - FileInputStream (java.io) >> - FilterInputStream (java.io) >> - PeekInputStream in ObjectInputStream (java.io) >> - BlockDataInputStream in ObjectInputStream (java.io) >> - PushbackInputStream (java.io) >> - FastInputStream in Manifest (java.util.jar) >> - ZipFileInputStream in ZipFile (java.util.zip) >> - CipherInputStream (javax.crypto) >> - MeteredStream (sun.net.www) >> - Anonymous in nullInputStream() in InputStream (java.io) >> - DataInputStream (java.io) >> - Anonymous in readTrailer() in GZIPInputStream (java.util.zip) >> - FtpInputStream in FtpURLConnection (sun.net.www.protocol.ftp) >> - JarURLInputStream in JarURLConnection (sun.net.www.protocol.jar) >> - PlainTextInputStream (sun.net.www.content.text) >> - PushbackInputStream (java.io) >> - TelnetInputStream (sun.net) >> - UnclosableInputStream in FileInputStreamPool (sun.security.provider) >> - MeteredStream (sun.net.www) >> - KeepAliveStream (sun.net.www.http) >> >> 2) Partially clean skip() implementation (1 class): >> - ChannelInputStream (sun.nio.ch) >>Note: clean skip impl when using SeekableByteChannel (most usages) >> otherwise super.skip() is used, and it may be easily rewritten using the >> existing internal buffer. >> >> 3) Unavoidable skip buffers (7 classes): >> - PipeInputStream in Process (java.lang) // per skip() invocation buffer >> byte[2048] >> - CheckedInputStream (java.util.zip) // per skip() invocation buffer >> byte[512] >> - InflaterInputStream (java.util.zip)// per skip() invocation buffer >> byte[512] >> - AppInputStream in SSLSocketImpl (sun.security.ssl) // per skip() >> invocation buffer byte[256] >> - DeflaterInputStream (java.util.zip) // cached skip buffer, byte[512], >> allocated on demand >> - ZipInputStream (java.util.zip) // preallocated skip buffer >> byte[512] >> - HttpInputStream in HttpURLConnection (sun.net.www.protocol.http) // >> cached skip buffer, byte[8096], allocated on demand >> >> It would be better to clean all skip implementations for the latter category >> and make it consistent. Now it's totally a mess. All possible strategies >> were implemented. >> >> Now let's consider classes which uses InputStream.skip() implementation (21 >> classes): >> >> 4) skip() is not implemented, when trivial implementation is possible (4 >> classes): >> - EmptyInputStream (sun.net.www.protocol.http) >> - NullInputStream in ProcessBuilder (java.lang) >> - ObjectInputStream (java.io) >> - extObjectInputStream (javax.crypto) >> >> 5) skip() is not implemented, when not-so-trivial implementation is possible >> (9 classes): >> - Anonymous in getInputStream() in NioSocketImpl (sun.nio.ch) >>Notes: temp direct buffer is used for reading, it's possible to implement >> skip over the direct buffer, save allocation + no copying from direct buffer >> to byte[] >> - Anonymous in newInputStream() in Channels (java.nio.channels) >>Notes: temp direct buffer is used for reading, it's possible to implement >> skip over the direct buffer, save allocation + no copying from direct buffer >> to byte[] >> - ChunkedInputStream (sun.net.www.http) >>Notes: skip() maybe implemented with the existing internal buffer >> - DecInputStream in Base64 (java.util) >> - ErrorStream in HttpURLConnection (sun.net.www.protocol.http) >>Notes: skip (but easily can be implemented with internal buffer + >> delegation to tail stream) >> - PipedInputStream (java.io) >>Notes: skip() may be implemented with the existing internal buffer >> - SequenceInputStream (java.io) >>Notes: skip() maybe implemented with delegat
Re: RFR: 8287171: Refactor null caller tests to a single directory [v3]
On Mon, 30 May 2022 00:10:50 GMT, Tim Prinzing wrote: >> Created a test at test/jdk/jdk/nullCaller called NullCallerTest that creates >> a test module with some resources in it for the actual tests that occur at >> the native level. The native part was switched to c++ instead of c to make >> it easier to create helper objects that reduce the redundant code performing >> error checking. The two helper classes InstanceCall and StaticCall were >> placed in an include file called CallHelper.hpp. The main part of the tests >> are in exeNullCallerTest.cpp, and there is a separate function for each of >> the bug reports. > > Tim Prinzing has updated the pull request incrementally with one additional > commit since the last revision: > > problem with iostream on Windows, use printf I don't think jdk/nullCaller is right location for this. Maybe jni/nullCaller could work. You'll probably need to add the location to an appropriate group/tier in test/jdk/TEST.groups, otherwise it won't be run. - PR: https://git.openjdk.java.net/jdk/pull/8934
Re: RFR: 8287363: null pointer should use NULL instead of 0
On Sat, 28 May 2022 03:31:30 GMT, Yasumasa Suenaga wrote: > We found using `0` as `NULL` in java_md_common.c . `0` is not a pointer, so > we should use `NULL` where we want to handle it. > > https://github.com/openjdk/jdk/pull/8646#discussion_r882294076 > > Also I found using `0` as NUL char in java_md_common.c , so I replaced it to > `\0` in this PR. Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8931
Re: RFR: 8284638: store skip buffers in InputStream Object [v15]
On Sun, 29 May 2022 18:15:52 GMT, Sergey Kuksenko wrote: > 5. skip() is not implemented, when not-so-trivial implementation is possible > (9 classes): For the low-level streams (e.g. connected to socket) then it would be common to see them wrapped by buffered streams. So it might not be worth doing anything there. However, I think your suggestion to change the no-arg read/write be non-abstract is interesting as it's always a pain to have to implement that. - PR: https://git.openjdk.java.net/jdk/pull/5872
Re: RFR: 8287285: Avoid redundant HashMap.containsKey call in java.util.zip.ZipFile.Source.get
On Sat, 30 Apr 2022 08:56:23 GMT, Andrey Turbanov wrote: > The method `java.util.zip.ZipFile.Source#get` could be improved by usage of > `Map.putIfAbsent` instead of separate `containsKey`/`get`/`put` calls. We > known that HashMap `java.util.zip.ZipFile.Source#files` can contain only > non-null values. And to check if putIfAbsent was successful or not we can > just compare result with `null`. > It makes code a bit cleaner and faster. Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8481
Re: RFR: 8284199: Implementation of Structured Concurrency (Incubator) [v3]
> This is the implementation of JEP 428: Structured Concurrency (Incubator). > > This is a non-final API that provides a gentle on-ramp to structure a task as > a family of concurrent subtasks, and to coordinate the subtasks as a unit. Alan Bateman has updated the pull request incrementally with one additional commit since the last revision: Add statement to close about thread termination - Changes: - all: https://git.openjdk.java.net/jdk/pull/8787/files - new: https://git.openjdk.java.net/jdk/pull/8787/files/c72f0330..4fc454a9 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8787=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8787=01-02 Stats: 12 lines in 3 files changed: 5 ins; 0 del; 7 mod Patch: https://git.openjdk.java.net/jdk/pull/8787.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8787/head:pull/8787 PR: https://git.openjdk.java.net/jdk/pull/8787
Re: RFR: 8284199: Implementation of Structured Concurrency (Incubator) [v2]
On Tue, 24 May 2022 10:52:07 GMT, Maurizio Cimadamore wrote: >> Alan Bateman 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: >> >> - Use {@code ...}, replace task->subtask, fix typos, add jls ref >> - Merge >> - @ignore StructuredThreadDumpTest until test infra in place >> - Refresh >> - Merge >> - Initial commit > > src/jdk.incubator.concurrent/share/classes/jdk/incubator/concurrent/StructuredTaskScope.java > line 214: > >> 212: * Tree structure >> 213: * >> 214: * StructuredTaskScopes form a tree where parent-child relations are >> established > > Should we mention what happens in the owner thread completes its execution > and the scope's `close` method has not been called? I think that, as > discussed offline, the fact that the thread will attempt to close any nested > scopes when terminating is an important aspect of this API. The close method might be the right place for this. It has to specify that an attempt to close out of order will close the nested scopes and terminating without close is much the same. I'll see what I can do, thanks for this suggestion. - PR: https://git.openjdk.java.net/jdk/pull/8787
Re: RFR: 8284199: Implementation of Structured Concurrency (Incubator) [v2]
On Tue, 24 May 2022 10:48:02 GMT, Maurizio Cimadamore wrote: >> More generally, I see that you used `{@code ... }` in a lot of places where >> `{@link ... }` could also be used. In some of those places (like this one) >> where there is a clear cross-reference, I think `@link` could be >> preferrable. The only case where `@code` is fine is when referring to the >> name of the class itself (e.g. `{@code StructuredTaskScope}`). But of course >> this is subjective. > > Also, note the typo `the join is invoked`. Either `the` is dropped, or > `method` is added. I've seen more than one occurrence of this. It should be "before join is invoked". It doesn't use a link here because it already links to join and joinUntil in the previous sentence. - PR: https://git.openjdk.java.net/jdk/pull/8787
Re: RFR: 8284199: Implementation of Structured Concurrency (Incubator) [v2]
> This is the implementation of JEP 428: Structured Concurrency (Incubator). > > This is a non-final API that provides a gentle on-ramp to structure a task as > a family of concurrent subtasks, and to coordinate the subtasks as a unit. Alan Bateman 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: - Use {@code ...}, replace task->subtask, fix typos, add jls ref - Merge - @ignore StructuredThreadDumpTest until test infra in place - Refresh - Merge - Initial commit - Changes: - all: https://git.openjdk.java.net/jdk/pull/8787/files - new: https://git.openjdk.java.net/jdk/pull/8787/files/6a9553b9..c72f0330 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8787=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8787=00-01 Stats: 6142 lines in 192 files changed: 3679 ins; 1909 del; 554 mod Patch: https://git.openjdk.java.net/jdk/pull/8787.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8787/head:pull/8787 PR: https://git.openjdk.java.net/jdk/pull/8787
Re: RFR: 8284199: Implementation of Structured Concurrency (Incubator)
On Tue, 24 May 2022 04:18:44 GMT, Joe Darcy wrote: >> This is the implementation of JEP 428: Structured Concurrency (Incubator). >> >> This is a non-final API that provides a gentle on-ramp to structure a task >> as a family of concurrent subtasks, and to coordinate the subtasks as a unit. > > src/jdk.incubator.concurrent/share/classes/jdk/incubator/concurrent/StructuredTaskScope.java > line 237: > >> 235: * the task result is retrieved via its {@code Future}, or >> happen-before any actions >> 236: * taken in a thread after {@linkplain #join() joining} of the task >> scope. >> 237: * > > Would a @-jls reference to the appropriate section of the memory model > chapter help here? Yes, it can reference JLS 17.4.5. - PR: https://git.openjdk.java.net/jdk/pull/8787
Re: RFR: 8284199: Implementation of Structured Concurrency (Incubator)
On Mon, 23 May 2022 21:09:24 GMT, Maurizio Cimadamore wrote: >> This is the implementation of JEP 428: Structured Concurrency (Incubator). >> >> This is a non-final API that provides a gentle on-ramp to structure a task >> as a family of concurrent subtasks, and to coordinate the subtasks as a unit. > > src/jdk.incubator.concurrent/share/classes/jdk/incubator/concurrent/StructuredTaskScope.java > line 88: > >> 86: * {@code join} method after forking. >> 87: * >> 88: * StructuredTaskScope defines the {@link #shutdown() shutdown} >> method to shut down a > > This sentence, because of the place where it appears, is a bit confusing. So > far we only know about the fact that a scope has an owner thread. So it seems > odd that shutdown could be called _while_ the owner thread is waiting on a > `join`. Of course, then you read what's next, and you discover that: (a) > shutdown might be called by a custom scope subclass and that (b) shutdown is > confined to the threads contained in this task scope - but this definition is > only given much later. I see your point. The intention is to introduce all the public methods before introducing the subclasses or policies. I think I can adjust this sentence to make it clear that a subtask may call shutdown while the owner is waiting in join. > src/jdk.incubator.concurrent/share/classes/jdk/incubator/concurrent/StructuredTaskScope.java > line 353: > >> 351: * >> 352: * The {@code handleComplete} method should be thread safe. It >> may be >> 353: * invoked by several threads at around the same. > > Something is missing? E.g. "at around the same TIME" ? (I'd suggest just > using "concurrently") Thanks, it was supposed to say "around the same time" but saying "concurrently" would be better. > src/jdk.incubator.concurrent/share/classes/jdk/incubator/concurrent/StructuredTaskScope.java > line 376: > >> 374: * >> 375: * If this task scope is {@linkplain #shutdown() shutdown} (or >> in the process >> 376: * of shutting down) then {@code fork} returns a Future >> representing a {@link > > Future in plaintext? Yes, Daniel also pointed this point that there are a few uses of "StructuredTaskScope" that should also use {@code ...}. - PR: https://git.openjdk.java.net/jdk/pull/8787
Re: RFR: 8284199: Implementation of Structured Concurrency (Incubator)
On Mon, 23 May 2022 13:11:29 GMT, Daniel Fuchs wrote: >> This is the implementation of JEP 428: Structured Concurrency (Incubator). >> >> This is a non-final API that provides a gentle on-ramp to structure a task >> as a family of concurrent subtasks, and to coordinate the subtasks as a unit. > > src/jdk.incubator.concurrent/share/classes/jdk/incubator/concurrent/StructuredTaskScope.java > line 54: > >> 52: >> 53: /** >> 54: * A basic API for structured concurrency. StructuredTaskScope >> supports cases > > Should StructuredTaskScope in this class-level API doc comment be surrounded > by `{@code }` to appear in code font? Okay, there are quite a few of these so I may have to adjust some of the lines to avoid too many inconsistent line lengths. - PR: https://git.openjdk.java.net/jdk/pull/8787
Re: RFR: 8287137: Problemlist failing x86_32 tests after Loom integration [v3]
On Tue, 24 May 2022 07:01:52 GMT, Aleksey Shipilev wrote: > Please approve, @AlanBateman, @mcimadamore and others? Okay with me. - PR: https://git.openjdk.java.net/jdk/pull/8843
Re: RFR: 8287206: Use WrongThreadException for confinement errors
On Tue, 24 May 2022 09:26:44 GMT, Maurizio Cimadamore wrote: > This patch tweaks the foreign API to use the newly added > `WrongThreadException` instead of `IllegalStateException` to report > confinement errors. Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8865
Re: RFR: 8287162: (zipfs) Performance regression related to support for POSIX file permissions
On Mon, 23 May 2022 19:47:33 GMT, Lance Andersen wrote: > Hi all, > > This PR addresses the performance issue that is described in JDK-8287162. > > With this fix, the ZipFileSystem methods: initOwner, initGroup, and > initPermissions will not be invoked unless enablePosixFileAttributes=true. > > Mach5 tiers1-3 are currently running and have not encountered any issues. Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8854
Re: RFR: 8287137: Problemlist failing x86_32 tests after Loom integration
On Mon, 23 May 2022 15:18:10 GMT, Aleksey Shipilev wrote: >> test/jdk/ProblemList.txt line 894: >> >>> 892: >>> java/util/stream/test/org/openjdk/tests/java/util/stream/CollectionAndMapModifyStreamTest.java >>> 8286642 generic-i586 >>> 893: >>> java/util/stream/test/org/openjdk/tests/java/util/stream/CollectorExample.java >>> 8286642 generic-i586 >>> 894: >>> java/util/stream/test/org/openjdk/tests/java/util/stream/CollectorToUnmodListTest.java >>> 8286642 generic-i586 >> >> I'm surprised that the stream tests are included as they don't run with >> --enable-preview. > > Those tests *do* run with preview enabled: > https://github.com/openjdk/jdk/blob/master/test/jdk/java/util/stream/test/TEST.properties#L10-L11 > -- and I know that because I basically copy-pasted all jtreg failures that > lead to `Unimplemented()` in x86_32 code. Okay, so the issue is SpliteratorTest uses preview APIs and having the configuration in TEST.properties means that all the streams tests are run with this option. - PR: https://git.openjdk.java.net/jdk/pull/8843
Re: RFR: 8287137: Problemlist failing x86_32 tests after Loom integration
On Mon, 23 May 2022 12:28:30 GMT, Aleksey Shipilev wrote: > [JDK-8284161](https://bugs.openjdk.java.net/browse/JDK-8284161) broke a lot > of x86_32 code. The x86_32 porting is done under > [JDK-8286642](https://bugs.openjdk.java.net/browse/JDK-8286642). Meanwhile, > we can problemlist the failing tests to get cleaner runs for other patches. > This should also make GHA runs cleaner. > > Additional testing: > - [x] Linux x86_32 fastdebug `tier1` (some unrelated failures in hotspot) > - [x] Linux x86_32 fastdebug `tier2` (some unrelated failures in hotspot) > - [x] Linux x86_32 fastdebug `tier3` (clean now) > - [ ] GHA run test/jdk/ProblemList.txt line 894: > 892: > java/util/stream/test/org/openjdk/tests/java/util/stream/CollectionAndMapModifyStreamTest.java > 8286642 generic-i586 > 893: > java/util/stream/test/org/openjdk/tests/java/util/stream/CollectorExample.java > 8286642 generic-i586 > 894: > java/util/stream/test/org/openjdk/tests/java/util/stream/CollectorToUnmodListTest.java > 8286642 generic-i586 I'm surprised that the stream tests are included as they don't run with --enable-preview. - PR: https://git.openjdk.java.net/jdk/pull/8843
Re: RFR: 8284199: Implementation of Structured Concurrency (Incubator)
On Sat, 21 May 2022 14:09:59 GMT, ExE Boss wrote: >> This is the implementation of JEP 428: Structured Concurrency (Incubator). >> >> This is a non-final API that provides a gentle on-ramp to structure a task >> as a family of concurrent subtasks, and to coordinate the subtasks as a unit. > > src/jdk.incubator.concurrent/share/classes/jdk/incubator/concurrent/StructuredTaskScope.java > line 1172: > >> 1170: } >> 1171: }; >> 1172: return AccessController.doPrivileged(pa); > > It might be better to use `MethodHandle`s obtained using > [jdk.internal.access.SharedSecrets].getJavaLangInvokeAccess() > and [JavaLangInvokeAccess].findStatic(…) and > [JavaLangInvokeAccess].findVirtual(…) for this, which > would avoid going through `AccessController.doPrivilaged(…)`. > > [jdk.internal.access.SharedSecrets]: > https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/jdk/internal/access/SharedSecrets.java > [JavaLangInvokeAccess]: > https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/jdk/internal/access/JavaLangInvokeAccess.java I'd prefer not export jdk.internal.access to this module, if possible. In general, it's a lot easier to reason about the security and integrity of the core platform when java.base doesn't export any of its internal packages. In any case, I expect this issue will resolve itself once there is a way for code in "core" modules to use preview APIs without needing to be compiled with --enable-preview. The java.management and jdk.incubator.vector modules have the same issue. - PR: https://git.openjdk.java.net/jdk/pull/8787
RFR: 8284199: Implementation of Structured Concurrency (Incubator)
This is the implementation of JEP 428: Structured Concurrency (Incubator). This is a non-final API that provides a gentle on-ramp to structure a task as a family of concurrent subtasks, and to coordinate the subtasks as a unit. - Commit messages: - @ignore StructuredThreadDumpTest until test infra in place - Refresh - Merge - Initial commit Changes: https://git.openjdk.java.net/jdk/pull/8787/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8787=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8284199 Stats: 2801 lines in 10 files changed: 2800 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/8787.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8787/head:pull/8787 PR: https://git.openjdk.java.net/jdk/pull/8787
Re: RFR: 8287121: Fix typo in jlink's description resource key lookup
On Sun, 22 May 2022 05:58:25 GMT, Christian Stein wrote: > Commit > https://github.com/openjdk/jdk/commit/655500a4f5e3abcff176599604deceefb6ca6640 > for issue [JDK-8286654](https://bugs.openjdk.java.net/browse/JDK-8286654) > added an optional description accessor on the `ToolProvider` interface. It > included a typo in` jlink`'s description resource key lookup: > `"jlink.desciption"` > > This follow-up commit fixes the typo by adding the missing `r` character: > `"jlink.description"`. > > This commit also also adds an automated check that ensures all current and > future tool provider implementations within the JDK don't throw an exception > when invoking their name and description accessor methods. Specific tool > names and descriptions are not expected by this general test. Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8825
Re: RFR: 8286571: java source launcher from a minimal jdk image containing jdk.compiler fails with --enable-preview option [v3]
On Mon, 23 May 2022 07:29:56 GMT, Adam Sotona wrote: > Sorry, I wrongly assumed it was `java.compiler` (the library module), but > this is about `jdk.compiler` (the tool)... Nevertheless, the same question > applies: Does `jdk.compiler` have any meaningful functionality even without > `jdk.zipfs` ? This came up in JDK 9 and the decision at the time was not to require jdk.zipfs as it's a service provider module. The addition of the source launcher, the addition of --enable-preview, and more use of --release N means more ct.sym usage. It probably isn't too interesting to have a small runtime image that contains jdk.compiler and not jdk.zipfs so Adam's proposal mightn't be too bad, it's just a bit weird and not something that we'd want other libraries in the eco system to do. - PR: https://git.openjdk.java.net/jdk/pull/8747
Re: RFR: 8287121: Fix typo in jlink's description resource key lookup
On Sun, 22 May 2022 05:58:25 GMT, Christian Stein wrote: > Commit > https://github.com/openjdk/jdk/commit/655500a4f5e3abcff176599604deceefb6ca6640 > for issue [JDK-8286654](https://bugs.openjdk.java.net/browse/JDK-8286654) > added an optional description accessor on the `ToolProvider` interface. It > included a typo in` jlink`'s description resource key lookup: > `"jlink.desciption"` > > This follow-up commit fixes the typo by adding the missing `r` character: > `"jlink.description"`. > > This commit also also adds an automated check that ensures all current and > future tool provider implementations within the JDK don't throw an exception > when invoking their name and description accessor methods. Specific tool > names and descriptions are not expected by this general test. test/jdk/java/util/spi/ToolProviderDescriptionTest.java line 46: > 44: > 45: static List listToolDescriptions() { > 46: return > StreamSupport.stream(ServiceLoader.load(ToolProvider.class).spliterator(), > false) it doesn't matter for this test but SL does have a stream method that could be used instead: ServiceLoader.load(ToolProvider.class) .stream() .map(ServiceLoader.Provider::get) .sorted(Comparator.comparing(ToolProvider::name)) ... - PR: https://git.openjdk.java.net/jdk/pull/8825
Re: RFR: 8286956: Loom: Define test groups for development/porting use
On Wed, 18 May 2022 11:10:58 GMT, Aleksey Shipilev wrote: > It would be beneficial to bring over the Loom-specific test groups from the > loom repo to aid development/porting work. > > https://github.com/openjdk/loom/blob/fibers/test/jdk/TEST.groups#L97-L108 > https://github.com/openjdk/loom/blob/6f42923b3342e41d95b262733205283068802b40/test/hotspot/jtreg/TEST.groups#L111-L125 > > I had to drop `jdk/incubator/concurrent` from JDK definition, because there > are no tests in that folder and tests break. > > Additional testing: > - [x] Linux x86_64 fastdebug `hotspot_loom jdk_loom` > - [x] Linux AArch64 fastdebug `hotspot_loom jdk_loom` Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8765
Integrated: 8286788: Test java/lang/Thread/virtual/ThreadAPI.testGetStackTrace3 fails
On Tue, 17 May 2022 11:15:19 GMT, Alan Bateman wrote: > This is a test fix. ThreadAPI.testGetStackTrace3 tests Thread::getStackTrace > on a thread doing a selection operation. The test is not reliable as it > expects to see the "select" method in the stack trace after waiting 200ms. > The test is changed to poll the stack trace so that it's no longer dependent > on sleep. > > The update includes a drive-by change to the test description to use > `@enablePreview`. This pull request has now been integrated. Changeset: 8535d51d Author:Alan Bateman URL: https://git.openjdk.java.net/jdk/commit/8535d51db7e1c33218c4254e774de4ca4ca60023 Stats: 10 lines in 1 file changed: 3 ins; 2 del; 5 mod 8286788: Test java/lang/Thread/virtual/ThreadAPI.testGetStackTrace3 fails Reviewed-by: darcy, jpai - PR: https://git.openjdk.java.net/jdk/pull/8743
RFR: 8286788: Test java/lang/Thread/virtual/ThreadAPI.testGetStackTrace3 fails
This is a test fix. ThreadAPI.testGetStackTrace3 tests Thread::getStackTrace on a thread doing a selection operation. The test is not reliable as it expects to see the "select" method in the stack trace after waiting 200ms. The test is changed to poll the stack trace so that it's no longer dependent on sleep. The update includes a drive-by change to the test description to use `@enablePreview`. - Commit messages: - Initial commit Changes: https://git.openjdk.java.net/jdk/pull/8743/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8743=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8286788 Stats: 10 lines in 1 file changed: 3 ins; 2 del; 5 mod Patch: https://git.openjdk.java.net/jdk/pull/8743.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8743/head:pull/8743 PR: https://git.openjdk.java.net/jdk/pull/8743
Re: RFR: JDK-8286783: Expand use of @inheritDoc in InputStream and OutputStream subclasses [v4]
On Mon, 16 May 2022 17:29:16 GMT, Joe Darcy wrote: >> Make the javadoc in the InputStream and OutputStream subclasses in core libs >> DRY-er by use of inheritDoc. (Any analagous changes to AudioInputStream in >> client libs will be done another a separate bug.) When the time comes, will >> do any necessary merging for the changes in[JDK-8286200. >> >> Please also review the CSR to cover the introduction of implspec and >> implNote tags: https://bugs.openjdk.java.net/browse/JDK-8286784 > > Joe Darcy has updated the pull request incrementally with one additional > commit since the last revision: > > Add @Override's requested in review feedback. thanks for the updates. - Marked as reviewed by alanb (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8717
Re: RFR: JDK-8286783: Expand use of @inheritDoc in InputStream and OutputStream subclasses [v3]
On Mon, 16 May 2022 16:56:00 GMT, Joe Darcy wrote: > I added the `@Override` annotations to methods I reviewed and/or updated; it > was not necessarily an exhaustive process. Yeah, there are inconsistencies as a result and I think we should go the extra mile and add to the methods such as readAllBbytes, readNBytes that weren't changed. - PR: https://git.openjdk.java.net/jdk/pull/8717
Re: RFR: JDK-8286783: Expand use of @inheritDoc in InputStream and OutputStream subclasses
On Sun, 15 May 2022 18:36:07 GMT, Joe Darcy wrote: > Make the javadoc in the InputStream and OutputStream subclasses in core libs > DRY-er by use of inheritDoc. (Any analagous changes to AudioInputStream in > client libs will be done another a separate bug.) When the time comes, will > do any necessary merging for the changes in[JDK-8286200. > > Please also review the CSR to cover the introduction of implspec and implNote > tags: https://bugs.openjdk.java.net/browse/JDK-8286784 Picking FileInputStream at random, I wonder if the `@inheritDoc` on transferTo is needed. Also, I think you've attempted to add `@Override` to all overridden methods but some may have been missed (e.g. readAllBytes and readNBytes). - PR: https://git.openjdk.java.net/jdk/pull/8717
Re: RFR: 8286559: Re-examine synchronization of mark and reset methods on InflaterInputStream [v2]
On Fri, 13 May 2022 07:14:30 GMT, Jaikiran Pai wrote: >> Can I please get a review of this change that addresses >> https://bugs.openjdk.java.net/browse/JDK-8286559? >> >> The commit here removes the `synchronized` on `mark` and `reset` methods of >> `InflaterInputStream`. The `mark` method is a no-op method and the `reset` >> method only always throws a `IOException`. So `synchronized` isn't adding >> any value here. >> >> Additionally, the commit does a minor change to the javadoc of these methods >> to use `@implNote` to describe what the implementation does. Please let me >> know if the `@implNote` is unnecessary, in which case, I'll revert that part. >> >> This change is similar to what was recently done for `FilterInputStream` >> https://github.com/openjdk/jdk/pull/8309 and `PushbackInputStream` >> https://github.com/openjdk/jdk/pull/8433 >> >> tier1, tier2 and tier3 tests were run and no related failures were noticed. > > Jaikiran Pai has updated the pull request incrementally with one additional > commit since the last revision: > > Incorporate review comment made on CSR by Joe - Change @implNote to > @implSpec Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8649
Re: RFR: JDK-8286604: Update InputStream and OutputStream to use @implSpec [v2]
On Thu, 12 May 2022 20:04:33 GMT, Joe Darcy wrote: >> While doing a CSR review of another issue, I noticed some cases in >> InputStream and OutputStream what would benefit from being upgraded to >> implSpec and related javadoc tags. >> >> The "A subclass must provide an implementation of this method." statements >> on several abstract methods don't add much value, but I chose to leave them >> in for this request. >> >> Please also review the corresponding CSR: >> https://bugs.openjdk.java.net/browse/JDK-8286605 > > Joe Darcy has updated the pull request incrementally with one additional > commit since the last revision: > > Respond to review feedback. Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8663
Re: RFR: 8286681: ShenandoahControlThread::request_gc misses the case of GCCause::_codecache_GC_threshold
On Fri, 13 May 2022 06:47:20 GMT, Jie Fu wrote: >> test/jdk/java/foreign/TestIntrinsics.java line 48: >> >>> 46: * -XX:+UseShenandoahGC >>> 47: * TestIntrinsics >>> 48: */ >> >> Is this needed? The parameters looks the same as the first test description >> so if you are testing with +ShenandoahGC then it will run already. > > Without `-XX:+UseShenandoahGC`, this bug wouldn't be exposed. > > What do you mean by `if you are testing with +ShenandoahGC then it will run > already`? I assume you are running the tests with: make run-tests TEST_OPTS_JAVA_OPTIONS="-XX:+UseShenandoahGC" in which case, all of the tests you select to run will be run with that GC. What you have is not wrong but wouldn't be a better to add a new test to test/hotspot/jtreg/gc rather than relying on test in test/jdk/java/foreign? - PR: https://git.openjdk.java.net/jdk/pull/8691
Re: RFR: 8286681: ShenandoahControlThread::request_gc misses the case of GCCause::_codecache_GC_threshold
On Fri, 13 May 2022 02:43:55 GMT, Jie Fu wrote: > Hi all, > > Some tests fail with Shenandoah GC after JDK-8282191. > The reason is that the assert in `ShenandoahControlThread::request_gc` misses > the case of `GCCause::_codecache_GC_threshold`. > It would be better to fix it. > > Thanks. > Best regards, > Jie test/jdk/java/foreign/TestIntrinsics.java line 48: > 46: * -XX:+UseShenandoahGC > 47: * TestIntrinsics > 48: */ Is this needed? The parameters looks the same as the first test description so if you are testing with +ShenandoahGC then it will run already. - PR: https://git.openjdk.java.net/jdk/pull/8691
Re: RFR: 8286378: Address possibly lossy conversions in java.base [v3]
On Fri, 13 May 2022 04:41:03 GMT, ExE Boss wrote: >> Roger Riggs has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Updated copyrights >> Fixed cast style to add a space after cast, (where consistent with file >> style) >> Improved code per review comments in PollSelectors. > > src/java.base/linux/classes/sun/nio/ch/EPollSelectorImpl.java line 128: > >> 126: // timed poll interrupted so need to adjust timeout >> 127: long adjust = System.nanoTime() - startTime; >> 128: to =- (int) TimeUnit.NANOSECONDS.toMillis(adjust); > > This will now always assign a negative number to `to`. > > > > `=-` is not a compound assignment, it’s negation followed by a normal > assignment. Well spotted, I don't think that change was intentionally. - PR: https://git.openjdk.java.net/jdk/pull/8642
Re: RFR: 8286560: Remove user parameter from jdk.internal.perf.Perf.attach()
On Wed, 11 May 2022 23:08:32 GMT, Ioi Lam wrote: > The API `jdk.internal.perf.Perf.::attach(String user, int lvmid)` is never > used. It should be removed, and all the handling of a specified user name > should be removed. Marked as reviewed by alanb (Reviewer). src/java.base/share/classes/jdk/internal/perf/Perf.java line 246: > 244: */ > 245: private native ByteBuffer attach0(int lvmid) > 246:throws IllegalArgumentException, IOException; You can drop the "throws IllegalArgumentException" here if you want, it's not needed as it's a runtime exception. - PR: https://git.openjdk.java.net/jdk/pull/8669
Re: RFR: JDK-8286604: Update InputStream and OutputStream to use @implSpec
On Wed, 11 May 2022 20:40:30 GMT, Joe Darcy wrote: > While doing a CSR review of another issue, I noticed some cases in > InputStream and OutputStream what would benefit from being upgraded to > implSpec and related javadoc tags. > > The "A subclass must provide an implementation of this method." statements on > several abstract methods don't add much value, but I chose to leave them in > for this request. > > Please also review the corresponding CSR: > https://bugs.openjdk.java.net/browse/JDK-8286605 src/java.base/share/classes/java/io/InputStream.java line 177: > 175: * > 176: * @apiNote > 177: * A subclass must provide an implementation of this method. Is this sentence useful to keep? The method is abstract so a concrete implementation has to implement it. On the other other hand, an abstract subclass does not need to implement it. src/java.base/share/classes/java/io/InputStream.java line 688: > 686: * @implSpec > 687: * The {@code mark} method of {@code InputStream} does > 688: * nothing. Minor nit but the line break can be removed so that "nothing" is on the same line. - PR: https://git.openjdk.java.net/jdk/pull/8663
Re: RFR: 8286378: Address possibly lossy conversions in java.base [v3]
On Wed, 11 May 2022 16:30:41 GMT, Roger Riggs wrote: >> PR#8599 8244681: proposes to add compiler warnings for possible lossy >> conversions >> From the CSR: >> >> "If the type of the right-hand operand of a compound assignment is not >> assignment compatible with the type of the variable, a cast is implied and >> possible lossy conversion may silently occur. While similar situations are >> resolved as compilation errors for primitive assignments, there are no >> similar rules defined for compound assignments." >> >> This PR anticipates the warnings and adds explicit casts to replace the >> implicit casts. >> In most cases, the cast matches the type of the right-hand side to type of >> the compound assignment. >> Since these casts are truncating the value, there might be a different >> refactoring that avoids the cast >> but a direct replacement was chosen here. >> >> (Advise and suggestions will inform similar updates to other OpenJDK >> modules). > > Roger Riggs has updated the pull request incrementally with one additional > commit since the last revision: > > Updated copyrights > Fixed cast style to add a space after cast, (where consistent with file > style) > Improved code per review comments in PollSelectors. Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8642
Re: RFR: 8286441: Remove mode parameter from jdk.internal.perf.Perf.attach() [v2]
On Wed, 11 May 2022 02:43:21 GMT, Ioi Lam wrote: >> The `mode` parameter for ` jdk.internal.perf.Perf.attach()` has never >> supported the value `"rw"` since the source code was imported to the openjdk >> repo more than 15 years ago. In fact HotSpot throws >> `IllegalArgumentException` when such a mode is specified. >> >> It's unlikely such a mode will be required for future enhancements. Support >> for `"rw"` is removed. The "mode" parameter is also removed, since now `"r"` >> is the only supported mode. >> >> I also cleaned up related code in the JDK and HotSpot. >> >> Testing: >> - Passed tiers 1 and 2 >> - Tiers 3, 4, 5 are in progress > > Ioi Lam has updated the pull request incrementally with one additional commit > since the last revision: > > review comments Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8622
Re: RFR: 8286582: Build fails on macos aarch64 when using --with-zlib=bundled
On Wed, 11 May 2022 12:47:08 GMT, Jaikiran Pai wrote: >> src/java.base/share/native/libzip/zlib/gzwrite.c line 452: >> >>> 450: len = strlen(next); >>> 451: # else >>> 452: # ifdef __APPLE__ // ignore format-nonliteral warning on macOS >> >> Instead of patching 3rd party code to fix a compilation warning, you should >> disable that warning instead. >> >> In `make/modules/java.base/lib/CoreLibraries.gmk`, add >> >> DISABLED_WARNINGS_clang := format-nonliteral, \ >> >> as line 138. > > Thank you for these useful inputs Magnus. I did these changes locally but for > some reason this format-nonliteral is not getting picked up while building > that library. I will investigate and see what's going on. Will update the PR > once I figure it out. I agree with Magnus and try to avoid changing the imported zlib code. - PR: https://git.openjdk.java.net/jdk/pull/8651
Re: RFR: 8286562: GCC 12 reports some compiler warnings [v2]
On Wed, 11 May 2022 08:40:21 GMT, Yasumasa Suenaga wrote: >> I saw some compiler warnings when I tried to build OpenJDK with GCC 12.0.1 >> on Fedora 36. >> As you can see, the warnings spreads several areas. Let me know if I should >> separate them by area. >> >> * -Wstringop-overflow >> * src/hotspot/share/oops/array.hpp >> * >> src/hotspot/share/jfr/recorder/checkpoint/types/traceid/jfrTraceIdBits.inline.hpp >> >> In member function 'void Array::at_put(int, const T&) [with T = unsigned >> char]', >> inlined from 'void ConstantPool::tag_at_put(int, jbyte)' at >> /home/ysuenaga/github-forked/jdk/src/hotspot/share/oops/constantPool.hpp:126:64, >> inlined from 'void ConstantPool::method_at_put(int, int, int)' at >> /home/ysuenaga/github-forked/jdk/src/hotspot/share/oops/constantPool.hpp:380:15, >> inlined from 'ConstantPool* >> BytecodeConstantPool::create_constant_pool(JavaThread*) const' at >> /home/ysuenaga/github-forked/jdk/src/hotspot/share/classfile/bytecodeAssembler.cpp:85:26: > > Yasumasa Suenaga has updated the pull request incrementally with one > additional commit since the last revision: > > Avoid pragma error in before GCC 12 src/java.base/unix/native/libjli/java_md_common.c line 135: > 133: if ((JLI_StrLen(indir) + JLI_StrLen(cmd) + 2) > sizeof(name)) return > 0; > 134: JLI_Snprintf(name, sizeof(name), "%s%c%s", indir, FILE_SEPARATOR, > cmd); > 135: #pragma GCC diagnostic pop Can we just replace this code rather than putting pragmas here? - PR: https://git.openjdk.java.net/jdk/pull/8646
Re: RFR: 8286444: javac errors after JDK-8251329 are not helpful enough to find root cause
On Tue, 10 May 2022 21:30:23 GMT, Sean Mullan wrote: > > It's probably ok, but the bug report is either incomplete or I am missing > > something. It says "This can be improved to something like: ..." but the > > same text as is emitted now is used. Can you fix this so I have a better > > example of what will be included in the message? > > The bug report also says "The error message does not give a clue which jar > file is causing the problem" but the error message includes the name > "invalid.jar" so I am also confused about that. There are two parts to it. In the case of initCEN method, the proposed change is to include the name of the rejected entry in the exception message. This is not the same thing as leaking a file path in the exception message so I don't think we have a concern here. - PR: https://git.openjdk.java.net/jdk/pull/8616
Re: RFR: 8286378: Address possibly lossy conversions in java.base [v2]
On Tue, 10 May 2022 23:01:33 GMT, Roger Riggs wrote: >> PR#8599 8244681: proposes to add compiler warnings for possible lossy >> conversions >> From the CSR: >> >> "If the type of the right-hand operand of a compound assignment is not >> assignment compatible with the type of the variable, a cast is implied and >> possible lossy conversion may silently occur. While similar situations are >> resolved as compilation errors for primitive assignments, there are no >> similar rules defined for compound assignments." >> >> This PR anticipates the warnings and adds explicit casts to replace the >> implicit casts. >> In most cases, the cast matches the type of the right-hand side to type of >> the compound assignment. >> Since these casts are truncating the value, there might be a different >> refactoring that avoids the cast >> but a direct replacement was chosen here. >> >> (Advise and suggestions will inform similar updates to other OpenJDK >> modules). > > Roger Riggs has updated the pull request incrementally with one additional > commit since the last revision: > > remove stray file src/java.base/linux/classes/sun/nio/ch/EPollSelectorImpl.java line 128: > 126: // timed poll interrupted so need to adjust timeout > 127: long adjust = System.nanoTime() - startTime; > 128: to -= (int)TimeUnit.MILLISECONDS.convert(adjust, > TimeUnit.NANOSECONDS); Can we change this `to =- (int) TimeUnit.NANOSECONDS.toMillis(adjust);` while we here? src/java.base/share/classes/jdk/internal/org/objectweb/asm/Frame.java line 615: > 613: if (outputStackTop >= elements) { > 614: outputStackTop -= (short)elements; > 615: } else { I think we usually try to avoid changing the ASM code if possible but maybe you have to do this because the compiler option is used for compiling java.base? src/java.base/unix/classes/sun/nio/ch/PollSelectorImpl.java line 123: > 121: // timed poll interrupted so need to adjust timeout > 122: long adjust = System.nanoTime() - startTime; > 123: to -= (int)TimeUnit.MILLISECONDS.convert(adjust, > TimeUnit.NANOSECONDS); This one can also be changed to: `to =- (int) TimeUnit.NANOSECONDS.toMillis(adjust);` - PR: https://git.openjdk.java.net/jdk/pull/8642
Re: RFR: 8286441: Remove mode parameter from jdk.internal.perf.Perf.attach()
On Tue, 10 May 2022 04:00:29 GMT, Ioi Lam wrote: > The `mode` parameter for ` jdk.internal.perf.Perf.attach()` has never > supported the value `"rw"` since the source code was imported to the openjdk > repo more than 15 years ago. In fact HotSpot throws > `IllegalArgumentException` when such a mode is specified. > > It's unlikely such a mode will be required for future enhancements. Support > for `"rw"` is removed. The "mode" parameter is also removed, since now `"r"` > is the only supported mode. > > I also cleaned up related code in the JDK and HotSpot. > > Testing: > - Passed tiers 1 and 2 > - Tiers 3, 4, 5 are in progress I skimmed through the changes and I think they look okay. In the distant past there were tools outside of the JDK that used the jvmstat API directly. It's possible that VisualVM still does but it would only compile/run if --add-exports is used to export the sun.jvmstat.* packages. So it might be that dropping the parameter from a method in RemoteHost is noticed and I think that is okay because this package is not exported and is not meant to be used by code outside of the JDK. - PR: https://git.openjdk.java.net/jdk/pull/8622
Re: RFR: 8286441: Remove mode parameter from jdk.internal.perf.Perf.attach()
On Tue, 10 May 2022 04:00:29 GMT, Ioi Lam wrote: > The `mode` parameter for ` jdk.internal.perf.Perf.attach()` has never > supported the value `"rw"` since the source code was imported to the openjdk > repo more than 15 years ago. In fact HotSpot throws > `IllegalArgumentException` when such a mode is specified. > > It's unlikely such a mode will be required for future enhancements. Support > for `"rw"` is removed. The "mode" parameter is also removed, since now `"r"` > is the only supported mode. > > I also cleaned up related code in the JDK and HotSpot. > > Testing: > - Passed tiers 1 and 2 > - Tiers 3, 4, 5 are in progress src/jdk.internal.jvmstat/share/classes/sun/jvmstat/perfdata/monitor/protocol/file/PerfDataBuffer.java line 60: > 58: FileChannel fc = new RandomAccessFile(f, "r").getChannel(); > 59: ByteBuffer bb = fc.map(FileChannel.MapMode.READ_ONLY, 0L, > (int)fc.size()); > 60: fc.close(); // doesn't need to remain open I think you can change this to: try (FileChannel fc = FileChannel.open(f.toPath())) { ByteBuffer bb = ... createPerfDataBuffer(bb, 0); } - PR: https://git.openjdk.java.net/jdk/pull/8622
Re: RFR: 8286444: javac errors after JDK-8251329 are not helpful enough to find root cause
On Tue, 10 May 2022 16:48:30 GMT, Christoph Langer wrote: > I think this would be OK, but would get to get someone from our security team > to bless it. It's print the entry name, I don't think it is leaking the file path to the zip file. - PR: https://git.openjdk.java.net/jdk/pull/8616
Re: RFR: 8286474: Drop --enable-preview from Sealed Classes related tests [v2]
On Tue, 10 May 2022 14:54:39 GMT, Aleksey Shipilev wrote: >> There are plenty of tests failing on many architectures due to >> `--enable-preview` VM code introduced by Loom. This improvement eliminates >> some of the redundant `--enable-preview` clauses from the Sealed Classes >> tests, since Sealed Classes have been graduated from preview in JDK 17. >> >> Additional testing: >> - [x] Linux x86_64 fastdebug, affected tests still pass >> - [x] Linux x86_32 fastdebug, affected tests start to pass > > Aleksey Shipilev has updated the pull request incrementally with one > additional commit since the last revision: > > Review comments Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8627
Re: RFR: 8286473: Drop --enable-preview from Record related tests [v2]
On Tue, 10 May 2022 14:51:52 GMT, Aleksey Shipilev wrote: >> There are plenty of tests failing on many architectures due to >> `--enable-preview` VM code introduced by Loom. This improvements eliminates >> some of the redundant `--enable-preview` clauses from the Record tests, >> since Records have been graduated from preview in JDK 16. >> >> Additional testing: >> - [x] Linux x86_64 fastdebug, affected tests still pass >> - [x] Linux x86_32 fastdebug, affected tests start to pass > > Aleksey Shipilev has updated the pull request incrementally with one > additional commit since the last revision: > > Review comments Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8626
Re: RFR: 8286473: Drop --enable-preview from Record related tests
On Tue, 10 May 2022 12:03:09 GMT, Aleksey Shipilev wrote: > There are plenty of tests failing on many architectures due to > `--enable-preview` VM code introduced by Loom. This improvements eliminates > some of the redundant `--enable-preview` clauses from the Record tests, since > Records have been graduated from preview in JDK 16. > > Additional testing: > - [x] Linux x86_64 fastdebug, affected tests still pass > - [x] Linux x86_32 fastdebug, affected tests start to pass test/jdk/java/nio/Buffer/BulkPutBuffer.java line 57: > 55: * @run testng/othervm BulkPutBuffer > 56: */ > 57: public class BulkPutBuffer { You can drop the -source option from these tests too. - PR: https://git.openjdk.java.net/jdk/pull/8626
Re: RFR: 8286444: javac errors after JDK-8251329 are not helpful enough to find root cause
On Mon, 9 May 2022 22:32:57 GMT, Christoph Langer wrote: > @LanceAndersen @AlanBateman do you think adding the entry name in the > exception in ZipFileSystem is ok? If so, should it maybe go into a different > patch? It should be okay as this is the name of an entry in the zip file. It might be a bit cleaner to add a method to IndexNode to return the name as String. Alternatively maybe its toString could be changed to drop the index (I would need to dig into the history to find out if there is really any use for that in the String representation). - PR: https://git.openjdk.java.net/jdk/pull/8616
Re: RFR: 8286474: Drop --enable-preview from Sealed Classes related tests
On Tue, 10 May 2022 12:07:31 GMT, Aleksey Shipilev wrote: > There are plenty of tests failing on many architectures due to > `--enable-preview` VM code introduced by Loom. This improvement eliminates > some of the redundant `--enable-preview` clauses from the Sealed Classes > tests, since Sealed Classes have been graduated from preview in JDK 17. > > Additional testing: > - [x] Linux x86_64 fastdebug, affected tests still pass > - [x] Linux x86_32 fastdebug, affected tests start to pass test/jdk/java/lang/reflect/sealed_classes/SealedClassesReflectionTest.java line 29: > 27: * @summary reflection test for sealed classes > 28: * @compile -source ${jdk.version} SealedClassesReflectionTest.java > 29: * @run testng/othervm SealedClassesReflectionTest You should be able to drop` -source ${jdk.version}` too. It was required when compiling with `--enable-preview`. - PR: https://git.openjdk.java.net/jdk/pull/8627
Re: RFR: JDK-8285730: unify _WIN32_WINNT settings [v4]
On Wed, 4 May 2022 08:00:08 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). > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > adjust API level to Windows 8 for security.cpp and do some cleanup Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8428
Re: RFR: 8286368: Cleanup problem lists after loom integration
On Mon, 9 May 2022 18:17:13 GMT, Leonid Mesnik wrote: > 8286368: Cleanup problem lists after loom integration Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8604
Re: RFR: 8286363: BigInteger.parallelMultiply missing @since 19
On Mon, 9 May 2022 15:26:20 GMT, Brian Burkhalter wrote: > Add missing `@since 19` tag. Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8598
Re: RFR: 8286294 : ForkJoinPool.commonPool().close() spins [v5]
On Sun, 8 May 2022 12:08:14 GMT, Doug Lea wrote: >> Changes ForkJoinPool.close spec and code to trap close as a no-op if called >> on common pool > > Doug Lea has updated the pull request incrementally with one additional > commit since the last revision: > > Test improvements Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8577
Re: RFR: 8284550: test failure_handler is not properly invoking jhsdb jstack, resulting in failure to produce a stack when a test times out
On Sun, 8 May 2022 21:57:20 GMT, Leonid Mesnik wrote: > …resulting in failure to produce a stack when a test times out Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8588
Re: RFR: 8286298: Remove unused methods in sun.invoke.util.VerifyType
On Fri, 6 May 2022 11:32:25 GMT, Claes Redestad wrote: > A few untested and unused methods in `VerifyType` which can be removed. > (Possibly used by native JSR 292 implementations in JDK 7). Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8570
Re: RFR: JDK-6725221 Standardize obtaining boolean properties with defaults
On Sun, 8 May 2022 02:22:08 GMT, Phil Race wrote: > I did wonder why it has security-libs as the sub-category and if the intent > was not what we see here. I suspect the JBS issue was initially created to to look at the usages of getProperty in the security code but it has been extended. The issue is that it changes the meaning of the empty value case in at least two places so each one will require careful attention. - PR: https://git.openjdk.java.net/jdk/pull/8559
Integrated: 8284161: Implementation of Virtual Threads (Preview)
On Fri, 8 Apr 2022 13:43:39 GMT, Alan Bateman wrote: > This is the implementation of JEP 425: Virtual Threads (Preview). > > 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 > can add additional labels, if required, as the PR progresses. > > 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 Extent Locals. This is to > make life a bit easier and avoid having to separate VM changes and juggle > branches at this time. This pull request has now been integrated. Changeset: 9583e365 Author:Alan Bateman URL: https://git.openjdk.java.net/jdk/commit/9583e3657e43cc1c6f2101a64534564db2a9bd84 Stats: 99468 lines in 1133 files changed: 91198 ins; 3598 del; 4672 mod 8284161: Implementation of Virtual Threads (Preview) Co-authored-by: Ron Pressler Co-authored-by: Alan Bateman Co-authored-by: Erik Österlund Co-authored-by: Andrew Haley Co-authored-by: Rickard Bäckman Co-authored-by: Markus Grönlund Co-authored-by: Leonid Mesnik Co-authored-by: Serguei Spitsyn Co-authored-by: Chris Plummer Co-authored-by: Coleen Phillimore Co-authored-by: Robbin Ehn Co-authored-by: Stefan Karlsson Co-authored-by: Thomas Schatzl Co-authored-by: Sergey Kuksenko Reviewed-by: lancea, eosterlund, rehn, sspitsyn, stefank, tschatzl, dfuchs, lmesnik, dcubed, kevinw, amenkov, dlong, mchung, psandoz, bpb, coleenp, smarks, egahlin, mseledtsov, coffeys, darcy - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v14]
> This is the implementation of JEP 425: Virtual Threads (Preview). > > 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 > can add additional labels, if required, as the PR progresses. > > 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 Extent 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 with a new target base due to a merge or a rebase. The pull request now contains 23 commits: - Refresh 4e99b5185eef9398c4cc4e90544de4a3153d61a9 - Merge with 5212535a276a92d96ca20bdcfccfbce956febdb1 - Refresh 6ace49bf42e5504c69fa11c564e8e865c0a95fb3 - Merge with 9425ab2b43b649bd591706bfc820e9c8795a6fdf - Refresh 12aa09ce7efd48425cc080d0b8761aca0f3e215f - Merge with 1bb4de2e2868a539846ec48dd43fd623c2ba69a5 - Refresh d77f7a49af75bcc5b20686805ff82a93a20dde05 - Merge with 4b2c82200fdc01de868cf414e40a4d891e753f89 - Refresh 6091080db743ece5f1b2111fcc35a5f2179a403a - Merge with cfcba1fccc8e3e6a68e1cb1826b70e076d5d83c4 - ... and 13 more: https://git.openjdk.java.net/jdk/compare/5212535a...df43e6fc - Changes: https://git.openjdk.java.net/jdk/pull/8166/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8166=13 Stats: 99468 lines in 1133 files changed: 91198 ins; 3598 del; 4672 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
Re: RFR: JDK-6725221 Standardize obtaining boolean properties with defaults
On Thu, 5 May 2022 16:49:07 GMT, Mark Powers wrote: > JDK-6725221 Standardize obtaining boolean properties with defaults src/java.base/share/classes/java/lang/reflect/AccessibleObject.java line 777: > 775: if (!printStackPropertiesSet && VM.initLevel() >= 1) { > 776: printStackWhenAccessFails = GetBooleanAction. > 777: > privilegedGetProperty("sun.reflect.debugModuleAccessChecks"); Running with `-Dsun.reflect.debugModuleAccessChecks` should set printStackWhenAccessFails to true, not false. - PR: https://git.openjdk.java.net/jdk/pull/8559
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v13]
On Fri, 6 May 2022 06:48:46 GMT, Alan Bateman wrote: >> This is the implementation of JEP 425: Virtual Threads (Preview). >> >> 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 >> can add additional labels, if required, as the PR progresses. >> >> 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 Extent 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 with a new target base due to a > merge or a rebase. The pull request now contains 21 commits: > > - Refresh 6ace49bf42e5504c69fa11c564e8e865c0a95fb3 > - Merge with 9425ab2b43b649bd591706bfc820e9c8795a6fdf > - Refresh 12aa09ce7efd48425cc080d0b8761aca0f3e215f > - Merge with 1bb4de2e2868a539846ec48dd43fd623c2ba69a5 > - Refresh d77f7a49af75bcc5b20686805ff82a93a20dde05 > - Merge with 4b2c82200fdc01de868cf414e40a4d891e753f89 > - Refresh 6091080db743ece5f1b2111fcc35a5f2179a403a > - Merge with cfcba1fccc8e3e6a68e1cb1826b70e076d5d83c4 > - Refresh ee9fa8ed05ec22de7a13383052d68aa8aa7832ec > - Merge with jdk-19+20 > - ... and 11 more: > https://git.openjdk.java.net/jdk/compare/9425ab2b...6d968671 > It is understandable to ship the preview feature not implemented on all > platforms. It is even understandable to ship the final product feature that > breaks some platforms in an clearly understandable way, prompting platform > maintainers to implement the agreed-upon, final Java feature. What I am > seeing, though, is that just running `java -version` (!) after integrating a > _preview feature_ is broken! Preview features need to be implemented by a port before it can be released. For JDK 19 this means that the maintainers of ports will have work to do for both JEP 424 and JEP 425 (ifdef is not an option). I think the issue that are concerned about is the interim period between the JEP 425 integration and the port/implementation of this feature to other architectures. I think the answer is that it will vary. It may be that some port maintainers decide to do something very short term so they can run without --enable-preview and buy time before they do the port/implementation for JDK 19. Good progress has been reported on at least ppc64le port and maybe that port be ready before others. So yes, some ports may crash until they receive attention, others (like zero) should be able to run tests that don't use --enable-preview. I've no doubt there will be a flurry of changes post integration. The motivation for Continuations::enabled() was to reduce risk and disable a lot of the new code by default. The motivation wasn't porting but it may be helpful during the interim period. That is probably a topic for loom-dev rather here. - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v13]
> This is the implementation of JEP 425: Virtual Threads (Preview). > > 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 > can add additional labels, if required, as the PR progresses. > > 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 Extent 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 with a new target base due to a merge or a rebase. The pull request now contains 21 commits: - Refresh 6ace49bf42e5504c69fa11c564e8e865c0a95fb3 - Merge with 9425ab2b43b649bd591706bfc820e9c8795a6fdf - Refresh 12aa09ce7efd48425cc080d0b8761aca0f3e215f - Merge with 1bb4de2e2868a539846ec48dd43fd623c2ba69a5 - Refresh d77f7a49af75bcc5b20686805ff82a93a20dde05 - Merge with 4b2c82200fdc01de868cf414e40a4d891e753f89 - Refresh 6091080db743ece5f1b2111fcc35a5f2179a403a - Merge with cfcba1fccc8e3e6a68e1cb1826b70e076d5d83c4 - Refresh ee9fa8ed05ec22de7a13383052d68aa8aa7832ec - Merge with jdk-19+20 - ... and 11 more: https://git.openjdk.java.net/jdk/compare/9425ab2b...6d968671 - Changes: https://git.openjdk.java.net/jdk/pull/8166/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8166=12 Stats: 99456 lines in 1130 files changed: 91188 ins; 3598 del; 4670 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
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v12]
On Thu, 5 May 2022 17:43:58 GMT, Aleksey Shipilev wrote: > I am sorry to be a buzzkill here, but this integration would break lots of > platforms even when Loom functionality is not enabled/used. For example, > running `java -version` on RISC-V runs into many issues: > `TemplateInterpreterGenerator::generate_Continuation_doYield_entry` runs into > `Unimplemented()`, `UnsafeCopyMemory` asserts about UCM table size, > `NativeDeoptInstruction::is_deopt` would run into `Unimplemented()` while > being called from signal handler. > > This effectively blocks development on those platforms, and it seems to be > too much breakage for a preview feature. Are we really breaking these > platforms? Do we have plans in place with maintainers of those platforms to > fix all this in JDK 19 timeframe? I'd suggest holding off the integration > until such a plan and agreement is in place. We mailed porters-dev in Sep 2021 to give a heads up that this feature would require porting work so it shouldn't be a surprise. We have been open to including other ports with the initial integration but it was never a goal to have all the ports done in advance of that. Most of the new code in the VM is only used when running with --enable-preview. You'll see several places that test Continuations::enabled(). It should be possible to get these port running without -enable-preview without much effort. The feature has to be implemented to pass all the tests of course. - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: JDK-8285987: executing shell scripts without #! fails on Alpine linux
On Wed, 4 May 2022 12:04:47 GMT, Matthias Baesken wrote: > A couple a tests like java/lang/ProcessBuilder/Basic.java#id0.Basic_id0 and > jdk/jshell/ExternalEditorTest.java.ExternalEditorTest try to start small > shell scripts without #! at the first line of the script. This fails with > error=8, Exec format error when running on Alpine 3.15 . > Looks like this is a known issue on musl / Alpine, see also > https://www.openwall.com/lists/musl/2018/03/09/2 > and > https://github.com/scala-steward-org/scala-steward/issues/1374 > (we see it on Alpine 3.15). test/lib/jdk/test/lib/Platform.java line 192: > 190: } > 191: > 192: public static boolean isMusl() { I think this will need test/lib/TestMutuallyExclusivePlatformPredicates.java to be updated too. - PR: https://git.openjdk.java.net/jdk/pull/8535
Re: RFR: 8274517: java/util/DoubleStreamSums/CompensatedSums.java fails with expected [true] but found [false]
On Tue, 19 Apr 2022 08:40:51 GMT, Raffaello Giulietti wrote: > Please review these small changes to address intermittent failures, as of > JDK-8274517. > > - Usage of jdk.test.lib.RandomFactory for reproducible random generation. > - Slightly less restrictive assertion about badParallelStreamError on L94 > (former L88). > - Verbatim copy of computeFinalSum() from j.u.s.Collectors 18. > > While these changes do not necessarily guarantee absence of intermittent > failures, the usage of jdk.test.lib.RandomFactory should at least help to pin > down specific double sequences that do not pass the test. > > There is still an inherent variability due to the use of parallel streams, > though. As double addition is not perfectly associative, even a fully known > sequence of doubles may lead to slightly different results with > parallelization. . - Marked as reviewed by alanb (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8290