Re: RFR: 8283667: [vectorapi] Vectorization for masked load with IOOBE with predicate feature [v3]
On Fri, 6 May 2022 04:22:30 GMT, Sandhya Viswanathan wrote: >> I'm afraid it's `argument(8)` for the load operation since the `argument(7)` >> is the mask input. It seems the argument number is not right begin from the >> mask input which is expected to be `6`. But the it's not. Actually I don't >> quite understand why. > > offset is long so uses two argument slots (5 and 6). > mask is argument (7). > offsetInRange is argument(8). Make sense! Thanks for the explanation! - PR: https://git.openjdk.java.net/jdk/pull/8035
Re: RFR: 8283667: [vectorapi] Vectorization for masked load with IOOBE with predicate feature [v3]
On Fri, 6 May 2022 03:47:47 GMT, Xiaohong Gong wrote:
>> src/hotspot/share/opto/vectorIntrinsics.cpp line 1238:
>>
>>> 1236: } else {
>>> 1237: // Masked vector load with IOOBE always uses the predicated
>>> load.
>>> 1238: const TypeInt* offset_in_range =
>>> gvn().type(argument(8))->isa_int();
>>
>> Should it be `argument(7)`? (and adjustments later to access the container).
>
> I'm afraid it's `argument(8)` for the load operation since the `argument(7)`
> is the mask input. It seems the argument number is not right begin from the
> mask input which is expected to be `6`. But the it's not. Actually I don't
> quite understand why.
offset is long so uses two argument slots (5 and 6).
mask is argument (7).
offsetInRange is argument(8).
-
PR: https://git.openjdk.java.net/jdk/pull/8035
Re: RFR: 8283667: [vectorapi] Vectorization for masked load with IOOBE with predicate feature [v3]
On Thu, 5 May 2022 19:27:47 GMT, Paul Sandoz wrote:
>> Xiaohong Gong has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Rename "use_predicate" to "needs_predicate"
>
> src/hotspot/share/opto/vectorIntrinsics.cpp line 1238:
>
>> 1236: } else {
>> 1237: // Masked vector load with IOOBE always uses the predicated load.
>> 1238: const TypeInt* offset_in_range =
>> gvn().type(argument(8))->isa_int();
>
> Should it be `argument(7)`? (and adjustments later to access the container).
I'm afraid it's `argument(8)` for the load operation since the `argument(7)` is
the mask input. It seems the argument number is not right begin from the mask
input which is expected to be `6`. But the it's not. Actually I don't quite
understand why.
-
PR: https://git.openjdk.java.net/jdk/pull/8035
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 bpb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8290
Re: RFR: 8285295: Need better testing for IdentityHashMap [v4]
On Wed, 4 May 2022 19:16:14 GMT, Stuart Marks wrote: >> Basic but fairly comprehensive set of tests for `IdentityHashMap`. The patch >> in the bug report that breaks `IdentityHashMap` now causes several cases in >> this new test to fail. There's more that could be done, but the new tests >> cover most of the core functions of `IdentityHashMap`. Unfortunately it >> seems difficult to merge this with the existing, comprehensive Collections >> tests (e.g., MOAT.java) because those tests implicity rely on >> `equals()`-based contract instead of the special-purpose `==`-based contract >> used by `IdentityHashMap`. > > Stuart Marks has updated the pull request incrementally with one additional > commit since the last revision: > > Add some assertions for entrySet.equals and keySet.equals > There should probably be something like > [test/jdk/java/util/Collections/Wrappers.java](https://github.com/openjdk/jdk/blob/master/test/jdk/java/util/Collections/Wrappers.java) Maybe. The intent of the test is fine, which is to ensure that a default method doesn't get added that breaks the invariants of the wrapper. One problem is that it tests only the default methods of `Collection` and not of the other collections interfaces. Another is that "override all default methods" isn't exactly the right criterion; instead, the criterion should be "override all default methods that would otherwise break this collection's invariants." It would be nice if such a test could be written, but as it stands I think that `Wrappers.java` test is too simplistic. - PR: https://git.openjdk.java.net/jdk/pull/8354
Re: RFR: 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available [v2]
On Thu, 28 Apr 2022 20:02:36 GMT, Brian Burkhalter wrote: >> Modify native multi-byte read-write code used by the `java.io` classes to >> limit the size of the allocated native buffer thereby decreasing off-heap >> memory footprint and increasing throughput. > > Brian Burkhalter has updated the pull request incrementally with one > additional commit since the last revision: > > 6478546: Decrease malloc'ed buffer maximum size to 64 kB Further performance testing was conducted for the case where the native read and write functions used a fixed, stack-allocated buffer of size 8192. The loops were moved up into the Java code of `FileInputStream`, `FileOutputStream` and `RandomAccessFile`. Note that there was code duplication because RAF needs both read and write methods as well. The performance of writing with this approach was approximately half what it had been, so for writing the approach was abandoned. Here are some updated performance measurements: https://user-images.githubusercontent.com/71468245/167041493-6d4c421c-c2ec-4a8a-8b32-09b2a902a77c.png;> https://user-images.githubusercontent.com/71468245/167041541-94e5806c-de86-4e62-a117-4cfafac82e87.png;> The performance measurements shown are for the following cases: 1. Master: unmodified code as it exists in the mainline 2. Java: fixed-size stack buffer in native read, read loops in Java, write as in the mainline but with malloc buffer size limit 3. Native: read loop in native read with malloc buffer size limit, write as in the mainline but with malloc buffer size limit The horizontal axis represents a variety of lengths from 8192 to 1GB; the vertical axis is throughput (ops/s) on a log 10 scale. The native lines in the charts are for the code proposed to be integrated. As can be seen, the performance of reading is quite similar up to larger lengths. The mainline version presumably starts to suffer the effect of large allocations. The native read loop performs the best throughout, being for lengths 10 MB and above from 50% to 3X faster than the mainline version. The native read loop is about 40% faster than the Java read loop for these larger lengths. Due to the log scale of the charts, the reading performance detail cannot be seen exactly and so is given here for the larger lengths: Throughput of read(byte[]) (ops/s) Length Master JavaNative 104857611341.39 6124.48211371.091 10485760 356.893 376.326 557.906 251503002 10.036 14.2719.869 5242880005.0056.8579.552 101.6753.5274.997 The performance of writing is about the same for the Java and Native versions, as it should be since the implementations are the same. Any difference is likely due to measurement noise. The mainline version again suffers for larger lengths. As the native write loop was already present in the mainline code, the principal complexity proposed to be added is the native read loop. Given the improved throughput and vastly reduced native memory allocation this seems to be justified. - PR: https://git.openjdk.java.net/jdk/pull/8235
Re: RFR: 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available [v3]
> Modify native multi-byte read-write code used by the `java.io` classes to > limit the size of the allocated native buffer thereby decreasing off-heap > memory footprint and increasing throughput. Brian Burkhalter has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains four additional commits since the last revision: - 6478546: Clean up io_util.c - Merge - 6478546: Decrease malloc'ed buffer maximum size to 64 kB - 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available - Changes: - all: https://git.openjdk.java.net/jdk/pull/8235/files - new: https://git.openjdk.java.net/jdk/pull/8235/files/40d46f8f..5c3a3446 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8235=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8235=01-02 Stats: 36827 lines in 1404 files changed: 26452 ins; 4333 del; 6042 mod Patch: https://git.openjdk.java.net/jdk/pull/8235.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8235/head:pull/8235 PR: https://git.openjdk.java.net/jdk/pull/8235
Re: RFR: 8285295: Need better testing for IdentityHashMap [v4]
On Wed, 4 May 2022 19:16:14 GMT, Stuart Marks wrote: >> Basic but fairly comprehensive set of tests for `IdentityHashMap`. The patch >> in the bug report that breaks `IdentityHashMap` now causes several cases in >> this new test to fail. There's more that could be done, but the new tests >> cover most of the core functions of `IdentityHashMap`. Unfortunately it >> seems difficult to merge this with the existing, comprehensive Collections >> tests (e.g., MOAT.java) because those tests implicity rely on >> `equals()`-based contract instead of the special-purpose `==`-based contract >> used by `IdentityHashMap`. > > Stuart Marks has updated the pull request incrementally with one additional > commit since the last revision: > > Add some assertions for entrySet.equals and keySet.equals Thanks Jaikiran. Could I have a Reviewer take a look at this please? - PR: https://git.openjdk.java.net/jdk/pull/8354
Integrated: 8286029: Add classpath exemption to globals_vectorApiSupport_***.S.inc
On Mon, 2 May 2022 20:05:36 GMT, Tyler Steele wrote: > Adds missing classpath exception to the header of two GPLv2 files. > > Requested > [here](https://mail.openjdk.java.net/pipermail/jdk-updates-dev/2022-April/013988.html). This pull request has now been integrated. Changeset: 6a1b145a Author:Tyler Steele Committer: Sandhya Viswanathan URL: https://git.openjdk.java.net/jdk/commit/6a1b145a0ab0057037f813f7dd6e71ad5b6f3de2 Stats: 8 lines in 2 files changed: 4 ins; 0 del; 4 mod 8286029: Add classpath exemption to globals_vectorApiSupport_***.S.inc Reviewed-by: sviswanathan - PR: https://git.openjdk.java.net/jdk/pull/8508
Integrated: JDK-8285497: Add system property for Java SE specification maintenance version
On Wed, 27 Apr 2022 22:27:34 GMT, Joe Darcy wrote: > Add a new system property, java.specification.maintenance.version, to return > the maintenance release number of the Java SE specification being > implemented. The property is unset, optional in the terminology of > System.getProperties, for an initial release of a specification. > > Please also review the CSR https://bugs.openjdk.java.net/browse/JDK-8285764 > > I'll update copyright years before an integration. This pull request has now been integrated. Changeset: 59ef76a3 Author:Joe Darcy URL: https://git.openjdk.java.net/jdk/commit/59ef76a365eafe40d8d68b4d5e028f0e731abd01 Stats: 23 lines in 4 files changed: 22 ins; 0 del; 1 mod 8285497: Add system property for Java SE specification maintenance version Reviewed-by: mullan, jpai, iris - PR: https://git.openjdk.java.net/jdk/pull/8437
Re: RFR: JDK-8285497: Add system property for Java SE specification maintenance version [v8]
> Add a new system property, java.specification.maintenance.version, to return > the maintenance release number of the Java SE specification being > implemented. The property is unset, optional in the terminology of > System.getProperties, for an initial release of a specification. > > Please also review the CSR https://bugs.openjdk.java.net/browse/JDK-8285764 > > I'll update copyright years before an integration. Joe Darcy has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 14 additional commits since the last revision: - Respond to review feedback and update copyrights. - Merge branch 'master' into JDK-8285497 - Respond to mbreinhold review feedback. - Merge branch 'master' into JDK-8285497 - Update wording to address review feedback. - Merge branch 'master' into JDK-8285497 - Change punctuation from review feedback. - Respond to review feedback; make sequence of values explicit. - Respond to review feedback. - Respond to review feedback. - ... and 4 more: https://git.openjdk.java.net/jdk/compare/796f256f...28ff456b - Changes: - all: https://git.openjdk.java.net/jdk/pull/8437/files - new: https://git.openjdk.java.net/jdk/pull/8437/files/7b7720cf..28ff456b Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8437=07 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8437=06-07 Stats: 1430 lines in 53 files changed: 600 ins; 493 del; 337 mod Patch: https://git.openjdk.java.net/jdk/pull/8437.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8437/head:pull/8437 PR: https://git.openjdk.java.net/jdk/pull/8437
Re: RFR: 8282191: Implementation of Foreign Function & Memory API (Preview) [v39]
On Thu, 5 May 2022 20:54:53 GMT, Maurizio Cimadamore wrote: >> This PR contains the API and implementation changes for JEP-424 [1]. A more >> detailed description of such changes, to avoid repetitions during the review >> process, is included as a separate comment. >> >> [1] - https://openjdk.java.net/jeps/424 > > Maurizio Cimadamore has updated the pull request incrementally with one > additional commit since the last revision: > > * Clarify what happens when SymbolLookup::loaderLookup is invoked from JNI > * Tweak restricted method check to work when there's no caller class src/java.base/share/classes/java/lang/foreign/SymbolLookup.java line 161: > 159: ClassLoader.getSystemClassLoader(); > 160: MemorySession loaderSession = (loader == null) ? > 161: MemorySession.global() : // boot loader never goes away The other built-in class loaders (`ClassLoaders::appClassLoader` and `ClassLoaders::platformClassLoader` are both instance of `BuiltinClassLoader`) will never be unloaded. Should they use the globel memory session? src/java.base/share/classes/jdk/internal/reflect/Reflection.java line 120: > 118: // if there is no caller class, act as if the call came from an > unnamed module > 119: Module module = currentClass != null ? > 120: currentClass.getModule() : Holder.FALLBACK_MODULE; This can be simplified to: Module module = currentClass != null ? currentClass.getModule() : ClassLoader::getSystemClassLoader().getUnnamedModule(); No need to have the Holder class. - PR: https://git.openjdk.java.net/jdk/pull/7888
Re: RFR: JDK-8285497: Add system property for Java SE specification maintenance version [v7]
On Wed, 4 May 2022 23:10:06 GMT, Joe Darcy wrote:
>> Add a new system property, java.specification.maintenance.version, to return
>> the maintenance release number of the Java SE specification being
>> implemented. The property is unset, optional in the terminology of
>> System.getProperties, for an initial release of a specification.
>>
>> Please also review the CSR https://bugs.openjdk.java.net/browse/JDK-8285764
>>
>> I'll update copyright years before an integration.
>
> Joe Darcy has updated the pull request with a new target base due to a merge
> or a rebase. The incremental webrev excludes the unrelated changes brought in
> by the merge/rebase. The pull request contains 12 additional commits since
> the last revision:
>
> - Respond to mbreinhold review feedback.
> - Merge branch 'master' into JDK-8285497
> - Update wording to address review feedback.
> - Merge branch 'master' into JDK-8285497
> - Change punctuation from review feedback.
> - Respond to review feedback; make sequence of values explicit.
> - Respond to review feedback.
> - Respond to review feedback.
> - Respond to CSR feedback.
> - Merge branch 'master' into JDK-8285497
> - ... and 2 more:
> https://git.openjdk.java.net/jdk/compare/f3cf898e...7b7720cf
Changes requested by mr (Lead).
src/java.base/share/classes/java/lang/System.java line 790:
> 788: * href="https://jcp.org/en/procedures/jcp2#3.6.4;>maintenance
> 789: * release. When defined, its value identifies that
> 790: * maintenance release. To indicate the first maintenance release
The final sentence can be shortened, and looking at it now the semicolon should
just be a comma:
* maintenance release. To indicate the first maintenance release
* this property will have the value {@code "1"}, to indicate the
* second maintenance release it will have the value {@code "2"},
* and so on.
Otherwise, this looks good to me.
-
PR: https://git.openjdk.java.net/jdk/pull/8437
Re: RFR: 8282191: Implementation of Foreign Function & Memory API (Preview) [v39]
> This PR contains the API and implementation changes for JEP-424 [1]. A more > detailed description of such changes, to avoid repetitions during the review > process, is included as a separate comment. > > [1] - https://openjdk.java.net/jeps/424 Maurizio Cimadamore has updated the pull request incrementally with one additional commit since the last revision: * Clarify what happens when SymbolLookup::loaderLookup is invoked from JNI * Tweak restricted method check to work when there's no caller class - Changes: - all: https://git.openjdk.java.net/jdk/pull/7888/files - new: https://git.openjdk.java.net/jdk/pull/7888/files/853f06b8..4d24ffa9 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7888=38 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7888=37-38 Stats: 22 lines in 2 files changed: 16 ins; 1 del; 5 mod Patch: https://git.openjdk.java.net/jdk/pull/7888.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7888/head:pull/7888 PR: https://git.openjdk.java.net/jdk/pull/7888
Re: RFR: 8282191: Implementation of Foreign Function & Memory API (Preview) [v37]
On Thu, 5 May 2022 16:39:08 GMT, Mandy Chung wrote: >> `BootLoader` is what you want in this case - it defines the static methods >> to access resources, packages etc defined to the boot loader (aka null >> `ClassLoader`). >> >> To find a symbol defined in the native libraries loaded by the boot loader, >> you can call `BootLoader.getNativeLibraries().find(name)`. > > When a caller-sensitive method is invoked from a thread attached via JNI, the > caller class returned from `Reflection::getCallerClass` is `null`.I would > recommend the default to be a class in the unnamed module defined to the > system class loader; hence it defaults to the system class loader. This is > consistent with JNI `FindClass` when invoked with no caller frame. > > It's an open issue [1] to revisit the default for `System::load` and > `System::loadLibrary` when invoked with null caller class. However, the > existing behavior may likely be unchanged because of the compatibility risk. > > [1] https://bugs.openjdk.java.net/browse/JDK-8281001 Thanks for the comments. I've pushed a new change which fixes a couple of thing: * first, for `SymbolLookup.loaderLookup`, the system class loader is used when no caller class exists (e.g. when method is called from JNI). If caller class exist but its loader is null (boot loader), we just call ClassLoader::findNative with a `null` loader which will do the right thing (thanks @mlchung for the tips!) * second, the restricted method check in `Reflection::ensureNativeAccess` has been improved to also work in case where caller class is `null`. In such cases, a dummy unnamed module module is used for the check. - PR: https://git.openjdk.java.net/jdk/pull/7888
Integrated: 8285616: [macos] Incorrect path for launcher-as-service.txt in .cfg file
On Thu, 5 May 2022 20:00:54 GMT, Alexey Semenyuk wrote:
> - Replace `System.getProperty("java.io.tmpdir")` call with hardcoded `/tmp`
> string to get root folder for test files.
> - Fix test cleanup - the test didn't delete test files upon completion
This pull request has now been integrated.
Changeset: 9644a314
Author:Alexey Semenyuk
URL:
https://git.openjdk.java.net/jdk/commit/9644a314cf1c80e43c48474f6f311fc98da597ac
Stats: 5 lines in 1 file changed: 1 ins; 0 del; 4 mod
8285616: [macos] Incorrect path for launcher-as-service.txt in .cfg file
Reviewed-by: almatvee
-
PR: https://git.openjdk.java.net/jdk/pull/8563
Re: RFR: 8285616: [macos] Incorrect path for launcher-as-service.txt in .cfg file
On Thu, 5 May 2022 20:00:54 GMT, Alexey Semenyuk wrote:
> - Replace `System.getProperty("java.io.tmpdir")` call with hardcoded `/tmp`
> string to get root folder for test files.
> - Fix test cleanup - the test didn't delete test files upon completion
Marked as reviewed by almatvee (Reviewer).
-
PR: https://git.openjdk.java.net/jdk/pull/8563
RFR: 8285616: [macos] Incorrect path for launcher-as-service.txt in .cfg file
- Replace `System.getProperty("java.io.tmpdir")` call with hardcoded `/tmp`
string to get root folder for test files.
- Fix test cleanup - the test didn't delete test files upon completion
-
Commit messages:
- 8285616: [macos] Incorrect path for launcher-as-service.txt in .cfg file
Changes: https://git.openjdk.java.net/jdk/pull/8563/files
Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8563=00
Issue: https://bugs.openjdk.java.net/browse/JDK-8285616
Stats: 5 lines in 1 file changed: 1 ins; 0 del; 4 mod
Patch: https://git.openjdk.java.net/jdk/pull/8563.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/8563/head:pull/8563
PR: https://git.openjdk.java.net/jdk/pull/8563
Integrated: 8286199: ProblemList jdk/jshell/ExternalEditorTest.java
On Thu, 5 May 2022 15:30:15 GMT, Roger Riggs wrote: > Put jdk/jshell/ExternalEditorTest.java on the problem list due to 8286191. This pull request has now been integrated. Changeset: 2f995c8d Author:Roger Riggs URL: https://git.openjdk.java.net/jdk/commit/2f995c8d2b8650e45e6a360f3c958bfaf46b2ef3 Stats: 2 lines in 1 file changed: 1 ins; 0 del; 1 mod 8286199: ProblemList jdk/jshell/ExternalEditorTest.java Reviewed-by: dcubed - PR: https://git.openjdk.java.net/jdk/pull/8557
Re: RFR: 8286199: ProblemList jdk/jshell/ExternalEditorTest.java [v2]
On Thu, 5 May 2022 19:56:36 GMT, Roger Riggs wrote: >> Put jdk/jshell/ExternalEditorTest.java on the problem list due to 8286191. > > Roger Riggs has updated the pull request with a new target base due to a > merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains three additional > commits since the last revision: > > - Merge branch 'master' into 8286199-problem-jshell > - 8286199: ProblemList jdk/jshell/ExternalEditorTest.java > - 8286195: ProblemList > test/lib-test/jdk/test/lib/TestMutuallyExclusivePlatformPredicates.java Thumbs up. This is a trivial fix. - Marked as reviewed by dcubed (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8557
Re: RFR: 8286199: ProblemList jdk/jshell/ExternalEditorTest.java [v2]
On Thu, 5 May 2022 19:56:36 GMT, Roger Riggs wrote: >> Put jdk/jshell/ExternalEditorTest.java on the problem list due to 8286191. > > Roger Riggs has updated the pull request with a new target base due to a > merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains three additional > commits since the last revision: > > - Merge branch 'master' into 8286199-problem-jshell > - 8286199: ProblemList jdk/jshell/ExternalEditorTest.java > - 8286195: ProblemList > test/lib-test/jdk/test/lib/TestMutuallyExclusivePlatformPredicates.java ProblemListing the test to clean up the CI. #8556 seems to have been delayed - PR: https://git.openjdk.java.net/jdk/pull/8557
Integrated: 8272352: Java launcher can not parse Chinese character when system locale is set to UTF-8
On Wed, 27 Apr 2022 20:23:32 GMT, Naoto Sato wrote: > Java runtime has been detecting the Windows system locale encoding using > `GetLocaleInfo(GetSystemDefaultLCID(), LOCALE_IDEFAULTANSICODEPAGE, ...)`, > but it returns the *legacy* ANSI code page value, e.g, 1252 for US-English. > In order to detect whether the user has selected `UTF-8` as the default, the > code page has to be queried with `GetACP()`. > Also, the case if the call to `GetLocaleInfo` fails changed to fall back to > `UTF-8` instead of `Cp1252`. This pull request has now been integrated. Changeset: 22934485 Author:Naoto Sato URL: https://git.openjdk.java.net/jdk/commit/229344853126692d38ff7cb164dd5d17c5bf7fbb Stats: 15 lines in 1 file changed: 6 ins; 4 del; 5 mod 8272352: Java launcher can not parse Chinese character when system locale is set to UTF-8 Reviewed-by: rriggs - PR: https://git.openjdk.java.net/jdk/pull/8434
Re: RFR: 8272352: Java launcher can not parse Chinese character when system locale is set to UTF-8 [v2]
On Tue, 3 May 2022 16:17:00 GMT, Roger Riggs wrote: >> Naoto Sato has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Default to UTF-8 if malloc fails > > src/java.base/windows/native/libjava/java_props_md.c line 695: > >> 693:_encoding); >> 694: >> 695: sprops.sun_jnu_encoding = getEncodingInternal(0); > > How should NULL from `getEncodingInternal` be handled? (only if malloc > fails). Ignore the repeated comment. The code looks fine. - PR: https://git.openjdk.java.net/jdk/pull/8434
Re: RFR: 8286199: ProblemList jdk/jshell/ExternalEditorTest.java [v2]
On Thu, 5 May 2022 19:23:53 GMT, Daniel D. Daugherty wrote: >> Roger Riggs has updated the pull request with a new target base due to a >> merge or a rebase. The incremental webrev excludes the unrelated changes >> brought in by the merge/rebase. The pull request contains three additional >> commits since the last revision: >> >> - Merge branch 'master' into 8286199-problem-jshell >> - 8286199: ProblemList jdk/jshell/ExternalEditorTest.java >> - 8286195: ProblemList >> test/lib-test/jdk/test/lib/TestMutuallyExclusivePlatformPredicates.java > > test/lib-test/ProblemList.txt line 40: > >> 38: # >> 39: >> # >> 40: jdk/test/lib/TestMutuallyExclusivePlatformPredicates.java 8286191 >> generic-all > > This change shouldn't be here. I think you need to update > your repo to sync with your earlier fix. Updated, after the merge only the langtools ProblemList.txt is modified - PR: https://git.openjdk.java.net/jdk/pull/8557
Re: RFR: 8286199: ProblemList jdk/jshell/ExternalEditorTest.java [v2]
> Put jdk/jshell/ExternalEditorTest.java on the problem list due to 8286191. Roger Riggs has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains three additional commits since the last revision: - Merge branch 'master' into 8286199-problem-jshell - 8286199: ProblemList jdk/jshell/ExternalEditorTest.java - 8286195: ProblemList test/lib-test/jdk/test/lib/TestMutuallyExclusivePlatformPredicates.java - Changes: - all: https://git.openjdk.java.net/jdk/pull/8557/files - new: https://git.openjdk.java.net/jdk/pull/8557/files/1962634e..cd0d157c Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8557=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8557=00-01 Stats: 656 lines in 10 files changed: 269 ins; 209 del; 178 mod Patch: https://git.openjdk.java.net/jdk/pull/8557.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8557/head:pull/8557 PR: https://git.openjdk.java.net/jdk/pull/8557
Re: RFR: 8272352: Java launcher can not parse Chinese character when system locale is set to UTF-8 [v2]
On Tue, 3 May 2022 18:55:52 GMT, Naoto Sato wrote: >> Java runtime has been detecting the Windows system locale encoding using >> `GetLocaleInfo(GetSystemDefaultLCID(), LOCALE_IDEFAULTANSICODEPAGE, ...)`, >> but it returns the *legacy* ANSI code page value, e.g, 1252 for US-English. >> In order to detect whether the user has selected `UTF-8` as the default, the >> code page has to be queried with `GetACP()`. >> Also, the case if the call to `GetLocaleInfo` fails changed to fall back to >> `UTF-8` instead of `Cp1252`. > > Naoto Sato has updated the pull request incrementally with one additional > commit since the last revision: > > Default to UTF-8 if malloc fails Looks good. src/java.base/windows/native/libjava/java_props_md.c line 695: > 693:_encoding); > 694: > 695: sprops.sun_jnu_encoding = getEncodingInternal(0); How should NULL from `getEncodingInternal` be handled? (only if malloc fails). - Marked as reviewed by rriggs (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8434
Re: RFR: 8283667: [vectorapi] Vectorization for masked load with IOOBE with predicate feature [v3]
On Thu, 5 May 2022 08:56:07 GMT, Xiaohong Gong wrote:
>> Currently the vector load with mask when the given index happens out of the
>> array boundary is implemented with pure java scalar code to avoid the IOOBE
>> (IndexOutOfBoundaryException). This is necessary for architectures that do
>> not support the predicate feature. Because the masked load is implemented
>> with a full vector load and a vector blend applied on it. And a full vector
>> load will definitely cause the IOOBE which is not valid. However, for
>> architectures that support the predicate feature like SVE/AVX-512/RVV, it
>> can be vectorized with the predicated load instruction as long as the
>> indexes of the masked lanes are within the bounds of the array. For these
>> architectures, loading with unmasked lanes does not raise exception.
>>
>> This patch adds the vectorization support for the masked load with IOOBE
>> part. Please see the original java implementation (FIXME: optimize):
>>
>>
>> @ForceInline
>> public static
>> ByteVector fromArray(VectorSpecies species,
>>byte[] a, int offset,
>>VectorMask m) {
>> ByteSpecies vsp = (ByteSpecies) species;
>> if (offset >= 0 && offset <= (a.length - species.length())) {
>> return vsp.dummyVector().fromArray0(a, offset, m);
>> }
>>
>> // FIXME: optimize
>> checkMaskFromIndexSize(offset, vsp, m, 1, a.length);
>> return vsp.vOp(m, i -> a[offset + i]);
>> }
>>
>> Since it can only be vectorized with the predicate load, the hotspot must
>> check whether the current backend supports it and falls back to the java
>> scalar version if not. This is different from the normal masked vector load
>> that the compiler will generate a full vector load and a vector blend if the
>> predicate load is not supported. So to let the compiler make the expected
>> action, an additional flag (i.e. `usePred`) is added to the existing
>> "loadMasked" intrinsic, with the value "true" for the IOOBE part while
>> "false" for the normal load. And the compiler will fail to intrinsify if the
>> flag is "true" and the predicate load is not supported by the backend, which
>> means that normal java path will be executed.
>>
>> Also adds the same vectorization support for masked:
>> - fromByteArray/fromByteBuffer
>> - fromBooleanArray
>> - fromCharArray
>>
>> The performance for the new added benchmarks improve about `1.88x ~ 30.26x`
>> on the x86 AVX-512 system:
>>
>> Benchmark before After Units
>> LoadMaskedIOOBEBenchmark.byteLoadArrayMaskIOOBE 737.542 1387.069 ops/ms
>> LoadMaskedIOOBEBenchmark.doubleLoadArrayMaskIOOBE 118.366 330.776 ops/ms
>> LoadMaskedIOOBEBenchmark.floatLoadArrayMaskIOOBE 233.832 6125.026 ops/ms
>> LoadMaskedIOOBEBenchmark.intLoadArrayMaskIOOBE233.816 7075.923 ops/ms
>> LoadMaskedIOOBEBenchmark.longLoadArrayMaskIOOBE 119.771 330.587 ops/ms
>> LoadMaskedIOOBEBenchmark.shortLoadArrayMaskIOOBE 431.961 939.301 ops/ms
>>
>> Similar performance gain can also be observed on 512-bit SVE system.
>
> Xiaohong Gong has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Rename "use_predicate" to "needs_predicate"
src/hotspot/share/opto/vectorIntrinsics.cpp line 1238:
> 1236: } else {
> 1237: // Masked vector load with IOOBE always uses the predicated load.
> 1238: const TypeInt* offset_in_range =
> gvn().type(argument(8))->isa_int();
Should it be `argument(7)`? (and adjustments later to access the container).
-
PR: https://git.openjdk.java.net/jdk/pull/8035
Re: RFR: 8286199: ProblemList jdk/jshell/ExternalEditorTest.java
On Thu, 5 May 2022 15:30:15 GMT, Roger Riggs wrote: > Put jdk/jshell/ExternalEditorTest.java on the problem list due to 8286191. Thumbs up (after resync). This is a trivial fix. test/lib-test/ProblemList.txt line 40: > 38: # > 39: > # > 40: jdk/test/lib/TestMutuallyExclusivePlatformPredicates.java 8286191 > generic-all This change shouldn't be here. I think you need to update your repo to sync with your earlier fix. - PR: https://git.openjdk.java.net/jdk/pull/8557
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: 8284161: Implementation of Virtual Threads (Preview) [v12]
On Thu, 5 May 2022 09:35:42 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 19 commits: > > - Refresh 12aa09ce7efd48425cc080d0b8761aca0f3e215f > - Merge with 1bb4de2e2868a539846ec48dd43fd623c2ba69a5 > - Refresh d77f7a49af75bcc5b20686805ff82a93a20dde05 > - Merge with 4b2c82200fdc01de868cf414e40a4d891e753f89 > - Refresh 6091080db743ece5f1b2111fcc35a5f2179a403a > - Merge with cfcba1fccc8e3e6a68e1cb1826b70e076d5d83c4 > - Refresh ee9fa8ed05ec22de7a13383052d68aa8aa7832ec > - Merge with jdk-19+20 > - Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e > - Refresh 8d8f0a2fd646e57fe6b4e8ab669f836dc46dda69 > - ... and 9 more: > https://git.openjdk.java.net/jdk/compare/1bb4de2e...86bea8ec 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. - Changes requested by shade (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8262889: Compiler implementation for Record Patterns
On Thu, 5 May 2022 15:21:49 GMT, Maurizio Cimadamore
wrote:
>> 8262889: Compiler implementation for Record Patterns
>>
>> A first version of a patch that introduces record patterns into javac as a
>> preview feature. For the specification, please see:
>> http://cr.openjdk.java.net/~gbierman/jep427+405/jep427+405-20220426/specs/patterns-switch-record-patterns-jls.html
>>
>> There are two notable tricky parts:
>> -in the parser, it was necessary to improve the `analyzePattern` method to
>> handle nested/record patterns, while still keeping error recovery reasonable
>> -in the `TransPatterns`, the desugaring of the record patterns is very
>> straightforward - effectivelly the record patterns are desugared into
>> guards/conditions. This will likely be improved in some future
>> version/preview
>>
>> `MatchException` has been extended to cover additional cases related to
>> record patterns.
>
> src/java.base/share/classes/jdk/internal/javac/PreviewFeature.java line 64:
>
>> 62: public enum Feature {
>> 63: SWITCH_PATTERN_MATCHING,
>> 64: DECONSTRUCTION_PATTERNS,
>
> The spec and JEP talks about record patterns - I think we should follow the
> correct name here
yup, I agree
-
PR: https://git.openjdk.java.net/jdk/pull/8516
Re: RFR: JDK-8285497: Add system property for Java SE specification maintenance version [v7]
On Wed, 4 May 2022 23:10:06 GMT, Joe Darcy wrote: >> Add a new system property, java.specification.maintenance.version, to return >> the maintenance release number of the Java SE specification being >> implemented. The property is unset, optional in the terminology of >> System.getProperties, for an initial release of a specification. >> >> Please also review the CSR https://bugs.openjdk.java.net/browse/JDK-8285764 >> >> I'll update copyright years before an integration. > > Joe Darcy has updated the pull request with a new target base due to a merge > or a rebase. The incremental webrev excludes the unrelated changes brought in > by the merge/rebase. The pull request contains 12 additional commits since > the last revision: > > - Respond to mbreinhold review feedback. > - Merge branch 'master' into JDK-8285497 > - Update wording to address review feedback. > - Merge branch 'master' into JDK-8285497 > - Change punctuation from review feedback. > - Respond to review feedback; make sequence of values explicit. > - Respond to review feedback. > - Respond to review feedback. > - Respond to CSR feedback. > - Merge branch 'master' into JDK-8285497 > - ... and 2 more: > https://git.openjdk.java.net/jdk/compare/460bf5de...7b7720cf Associated CSR also reviewed. - Marked as reviewed by iris (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8437
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v12]
On Thu, 5 May 2022 09:35:42 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 19 commits: > > - Refresh 12aa09ce7efd48425cc080d0b8761aca0f3e215f > - Merge with 1bb4de2e2868a539846ec48dd43fd623c2ba69a5 > - Refresh d77f7a49af75bcc5b20686805ff82a93a20dde05 > - Merge with 4b2c82200fdc01de868cf414e40a4d891e753f89 > - Refresh 6091080db743ece5f1b2111fcc35a5f2179a403a > - Merge with cfcba1fccc8e3e6a68e1cb1826b70e076d5d83c4 > - Refresh ee9fa8ed05ec22de7a13383052d68aa8aa7832ec > - Merge with jdk-19+20 > - Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e > - Refresh 8d8f0a2fd646e57fe6b4e8ab669f836dc46dda69 > - ... and 9 more: > https://git.openjdk.java.net/jdk/compare/1bb4de2e...86bea8ec Marked as reviewed by darcy (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8286154: Fix 3rd party notices in test files
On Thu, 5 May 2022 16:13:59 GMT, Naoto Sato wrote: > Trivial fix to 3rd party copyright notices. Marked as reviewed by iris (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8558
Re: RFR: 8282191: Implementation of Foreign Function & Memory API (Preview) [v37]
On Thu, 5 May 2022 16:22:41 GMT, Mandy Chung wrote: >> Looking deeper, `System::loadLibrary` seems to search the boot loader >> libraries when invoked with a `null` caller class, so replicating that >> behavior should be safe. > > `BootLoader` is what you want in this case - it defines the static methods to > access resources, packages etc defined to the boot loader (aka null > `ClassLoader`). > > To find a symbol defined in the native libraries loaded by the boot loader, > you can call `BootLoader.getNativeLibraries().find(name)`. When a caller-sensitive method is invoked from a thread attached via JNI, the caller class returned from `Reflection::getCallerClass` is `null`.I would recommend the default to be a class in the unnamed module defined to the system class loader; hence it defaults to the system class loader. This is consistent with JNI `FindClass` when invoked with no caller frame. It's an open issue [1] to revisit the default for `System::load` and `System::loadLibrary` when invoked with null caller class. However, the existing behavior may likely be unchanged because of the compatibility risk. [1] https://bugs.openjdk.java.net/browse/JDK-8281001 - PR: https://git.openjdk.java.net/jdk/pull/7888
Re: RFR: JDK-8286191: misc tests fail due to JDK-8286191
On Thu, 5 May 2022 15:21:23 GMT, Matthias Baesken wrote: > The isMusl method had to be handled in > test/lib-test/jdk/test/lib/TestMutuallyExclusivePlatformPredicates.java . > Additionally, the vm.musl predicate seem not to be available in the langtools > tests. @MBaesken - I corrected a typo in my synopsis update for JDK-8286191. Please use "/issue JDK-8286191" to update the PR's synopsis. - PR: https://git.openjdk.java.net/jdk/pull/8556
Re: RFR: 8286154: Fix 3rd party notices in test files
On Thu, 5 May 2022 16:13:59 GMT, Naoto Sato wrote: > Trivial fix to 3rd party copyright notices. Marked as reviewed by joehw (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8558
Re: RFR: 8282191: Implementation of Foreign Function & Memory API (Preview) [v37]
On Wed, 4 May 2022 23:44:08 GMT, Maurizio Cimadamore wrote: >> Another option would be to treat calls to `ensureNativeAccess` with `null` >> caller class as coming from unnamed module. > > Looking deeper, `System::loadLibrary` seems to search the boot loader > libraries when invoked with a `null` caller class, so replicating that > behavior should be safe. `BootLoader` is what you want in this case - it defines the static methods to access resources, packages etc defined to the boot loader (aka null `ClassLoader`). To find a symbol defined in the native libraries loaded by the boot loader, you can call `BootLoader.getNativeLibraries().find(name)`. - PR: https://git.openjdk.java.net/jdk/pull/7888
Re: RFR: 8286154: Fix 3rd party notices in test files
On Thu, 5 May 2022 16:13:59 GMT, Naoto Sato wrote: > Trivial fix to 3rd party copyright notices. Marked as reviewed by darcy (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8558
RFR: 8286154: Fix 3rd party notices in test files
Trivial fix to 3rd party copyright notices. - Commit messages: - 8286154: Fix 3rd party notices in test files Changes: https://git.openjdk.java.net/jdk/pull/8558/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8558=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8286154 Stats: 100 lines in 21 files changed: 64 ins; 0 del; 36 mod Patch: https://git.openjdk.java.net/jdk/pull/8558.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8558/head:pull/8558 PR: https://git.openjdk.java.net/jdk/pull/8558
Re: RFR: JDK-8286191: misc tests fail due to JDK-8286191
On Thu, 5 May 2022 15:21:23 GMT, Matthias Baesken wrote: > The isMusl method had to be handled in > test/lib-test/jdk/test/lib/TestMutuallyExclusivePlatformPredicates.java . > Additionally, the vm.musl predicate seem not to be available in the langtools > tests. Changes requested by dcubed (Reviewer). test/langtools/jdk/jshell/ExternalEditorTest.java line 29: > 27: * @bug 8143955 8080843 8163816 8143006 8169828 8171130 8162989 8210808 > 28: * @comment musl/Alpine has problems executing some shell scripts, see > 8285987 > 29: * @requires !vm.musl So this change backs out an "@requires" that was added by: JDK-8285987 executing shell scripts without #! fails on Alpine linux https://bugs.openjdk.java.net/browse/JDK-8285987 Presumably this "@requires" was added for some reason so what's going to happen if this test is run on Alpine Linux? Also, the fix in JDK-8285987 updated the copyright year. Do you plan on restoring it to the original "2017" value? - PR: https://git.openjdk.java.net/jdk/pull/8556
Re: RFR: 8284960: Integration of JEP 426: Vector API (Fourth Incubator) [v2]
On Thu, 5 May 2022 05:47:47 GMT, Jatin Bhateja wrote: >> Hi All, >> >> Patch adds the planned support for new vector operations and APIs targeted >> for [JEP 426: Vector API (Fourth >> Incubator).](https://bugs.openjdk.java.net/browse/JDK-8280173) >> >> Following is the brief summary of changes:- >> >> 1) Extends the scope of existing lanewise API for following new vector >> operations. >>- VectorOperations.BIT_COUNT: counts the number of one-bits >>- VectorOperations.LEADING_ZEROS_COUNT: counts the number of leading zero >> bits >>- VectorOperations.TRAILING_ZEROS_COUNT: counts the number of trailing >> zero bits >>- VectorOperations.REVERSE: reversing the order of bits >>- VectorOperations.REVERSE_BYTES: reversing the order of bytes >>- compress and expand bits: Semantics are based on Hacker's Delight >> section 7-4 Compress, or Generalized Extract. >> >> 2) Adds following new APIs to perform cross lane vector compress and >> expansion operations under the influence of a mask. >>- Vector.compress >>- Vector.expand >>- VectorMask.compress >> >> 3) Adds predicated and non-predicated versions of following new APIs to load >> and store the contents of vector from foreign MemorySegments. >> - Vector.fromMemorySegment >> - Vector.intoMemorySegment >> >> 4) C2 Compiler IR enhancements and optimized X86 and AARCH64 backend support >> for each newly added operation. >> >> >> Patch has been regressed over AARCH64 and X86 targets different AVX levels. >> >> Kindly review and share your feedback. >> >> Best Regards, >> Jatin > > Jatin Bhateja has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains 10 commits: > > - 8284960: Correcting a typo. > - 8284960: Integrating changes from panama-vector (Add @since 19 tags). > - Merge branch 'master' of http://github.com/openjdk/jdk into JDK-8284960 > - Merge branch 'master' of http://github.com/openjdk/jdk into JDK-8284960 > - Merge branch 'master' of http://github.com/openjdk/jdk into JDK-8284960 > - 8284960: AARCH64 backend changes. > - Merge branch 'master' of http://github.com/openjdk/jdk into JDK-8284960 > - Merge branch 'master' of http://github.com/openjdk/jdk into JDK-8284960 > - Merge branch 'master' of http://github.com/openjdk/jdk into JDK-8284960 > - 8284960: Integration of JEP 426: Vector API (Fourth Incubator) Hi @vnkozlov , It will be helpful if you can kindly review the changes. - PR: https://git.openjdk.java.net/jdk/pull/8425
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v12]
On Thu, 5 May 2022 09:35:42 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 19 commits: > > - Refresh 12aa09ce7efd48425cc080d0b8761aca0f3e215f > - Merge with 1bb4de2e2868a539846ec48dd43fd623c2ba69a5 > - Refresh d77f7a49af75bcc5b20686805ff82a93a20dde05 > - Merge with 4b2c82200fdc01de868cf414e40a4d891e753f89 > - Refresh 6091080db743ece5f1b2111fcc35a5f2179a403a > - Merge with cfcba1fccc8e3e6a68e1cb1826b70e076d5d83c4 > - Refresh ee9fa8ed05ec22de7a13383052d68aa8aa7832ec > - Merge with jdk-19+20 > - Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e > - Refresh 8d8f0a2fd646e57fe6b4e8ab669f836dc46dda69 > - ... and 9 more: > https://git.openjdk.java.net/jdk/compare/1bb4de2e...86bea8ec Studied the java.io package edits, the new JFR events and the new jcmd dump_to_file functionality. Looks good! - Marked as reviewed by coffeys (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8166
Withdrawn: 8286199: ProblemList jdk/jshell/ExternalEditorTest.java
On Thu, 5 May 2022 15:30:15 GMT, Roger Riggs wrote: > Put jdk/jshell/ExternalEditorTest.java on the problem list due to 8286191. This pull request has been closed without being integrated. - PR: https://git.openjdk.java.net/jdk/pull/8557
Re: RFR: 8286199: ProblemList jdk/jshell/ExternalEditorTest.java
On Thu, 5 May 2022 15:30:15 GMT, Roger Riggs wrote: > Put jdk/jshell/ExternalEditorTest.java on the problem list due to 8286191. The PR is not needed, the issue will be fixed by PR#8556. - PR: https://git.openjdk.java.net/jdk/pull/8557
Re: RFR: JDK-8286191: misc tests fail due to JDK-8286191
On Thu, 5 May 2022 15:21:23 GMT, Matthias Baesken wrote: > The isMusl method had to be handled in > test/lib-test/jdk/test/lib/TestMutuallyExclusivePlatformPredicates.java . > Additionally, the vm.musl predicate seem not to be available in the langtools > tests. LGTM - Marked as reviewed by rriggs (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8556
Re: RFR: 8285517: System.getenv() returns unexpected value if environment variable has non ASCII character [v4]
On Thu, 5 May 2022 15:41:44 GMT, Naoto Sato wrote:
>> Hello @naotoj .
>> If I just use `System.getProperty("sun.jnu.encoding")`, following testcases
>> were failed.
>>
>> java/lang/ProcessBuilder/SecurityManagerClinit.java
>> java/lang/ProcessHandle/PermissionTest.java
>>
>> Please give me your suggestion again.
>
> Ah, OK. Never mind.
It might be worthwhile to cache the `sun.jnu.encoding` property value in
jdk/internal/util/StaticProperty.
And perhaps even cache the Charset (not just the string).
-
PR: https://git.openjdk.java.net/jdk/pull/8378
Re: RFR: 8262889: Compiler implementation for Record Patterns
On Tue, 3 May 2022 12:07:50 GMT, Jan Lahoda wrote:
> 8262889: Compiler implementation for Record Patterns
>
> A first version of a patch that introduces record patterns into javac as a
> preview feature. For the specification, please see:
> http://cr.openjdk.java.net/~gbierman/jep427+405/jep427+405-20220426/specs/patterns-switch-record-patterns-jls.html
>
> There are two notable tricky parts:
> -in the parser, it was necessary to improve the `analyzePattern` method to
> handle nested/record patterns, while still keeping error recovery reasonable
> -in the `TransPatterns`, the desugaring of the record patterns is very
> straightforward - effectivelly the record patterns are desugared into
> guards/conditions. This will likely be improved in some future version/preview
>
> `MatchException` has been extended to cover additional cases related to
> record patterns.
I've added some renaming suggestions for the code in Flow, after some
discussions with @biboudis.
More generally, that code should contain comments, with example of what it's
trying to do - e.g. how it's partitioning the set of patterns, etc.
src/java.base/share/classes/jdk/internal/javac/PreviewFeature.java line 64:
> 62: public enum Feature {
> 63: SWITCH_PATTERN_MATCHING,
> 64: DECONSTRUCTION_PATTERNS,
The spec and JEP talks about record patterns - I think we should follow the
correct name here
src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Flow.java line 751:
> 749:Iterable JCCaseLabel> labels) {
> 750: Set constants = new HashSet<>();
> 751: Map>
> categorizedDeconstructionPatterns = new HashMap<>();
maybe rename to `deconstructionPatternsBySymbol` ?
src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Flow.java line 790:
> 788: }
> 789:
> 790: private boolean
> coversDeconstructionStartingFromComponent(DiagnosticPosition pos,
maybe rename as `coverDeconstructionFromComponent`/`coverDeconstructionAt`
src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Flow.java line 792:
> 790: private boolean
> coversDeconstructionStartingFromComponent(DiagnosticPosition pos,
> 791: Type
> targetType,
> 792:
> List patterns,
patterns -> "deconstructionPatterns"
src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Flow.java line 800:
> 798: }
> 799:
> 800: Type parameterizedComponentType =
> types.memberType(targetType, components.get(component));
maybe `instantiatedComponentType`
src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Flow.java line 802:
> 800: Type parameterizedComponentType =
> types.memberType(targetType, components.get(component));
> 801: List nestedComponentPatterns = patterns.map(d ->
> d.nested.get(component));
> 802: Set nestedCovered = coveredSymbols(pos,
> parameterizedComponentType,
maybe `coveredComponents`
src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Flow.java line 807:
> 805: Set covered = new HashSet<>();
> 806:
> 807: for (JCDeconstructionPattern subTypeCandidate : patterns) {
`subTypeCandidate` -> `deconstructionPattern`
src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Flow.java line 809:
> 807: for (JCDeconstructionPattern subTypeCandidate : patterns) {
> 808: JCPattern nestedPattern =
> subTypeCandidate.nested.get(component);
> 809: Symbol currentPatternType;
`currentPatternType` -> `componentPatternType`
src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Flow.java line 823:
> 821: }
> 822: }
> 823: for (Symbol currentType : nestedCovered) {
`currentType` -> `coveredComponent`
src/jdk.compiler/share/classes/com/sun/tools/javac/tree/JCTree.java line 246:
> 244: PARENTHESIZEDPATTERN,
> 245:
> 246: DECONSTRUCTIONPATTERN,
might want to rename to RECORDPATTERN (but this is impl dependent, so less
important to fix)
src/jdk.compiler/share/classes/com/sun/tools/javac/tree/JCTree.java line 2373:
> 2371: }
> 2372:
> 2373: public static class JCDeconstructionPattern extends JCPattern
JCRecordPattern (although this is impl-only, so less important to fix)
-
PR: https://git.openjdk.java.net/jdk/pull/8516
Re: RFR: 8262889: Compiler implementation for Record Patterns
On Thu, 5 May 2022 12:28:42 GMT, Aggelos Biboudis wrote: >>> I think this is i) from the domination relation: >>> >>> > A record pattern with type R and record component pattern list L >>> > dominates another record pattern with type S and record component pattern >>> > list M if (i) the erasure of S is a subtype of the erasure of R, and (ii) >>> > every pattern, if any, in L dominates the corresponding pattern in M. >> >> But this subtyping test seems to happen at the level of the component >> pattern list, not at the R/S level, right? > > You are right. It is the ii) which iteratively checks the component pattern > list L. I now believe that the check is needed to properly classify patterns based on the type of the i-th component. That said, not sure this should be a subtyping check, or a type equality - PR: https://git.openjdk.java.net/jdk/pull/8516
Re: RFR: 8285517: System.getenv() returns unexpected value if environment variable has non ASCII character [v4]
On Thu, 5 May 2022 08:08:29 GMT, Ichiroh Takiguchi
wrote:
>> src/java.base/unix/classes/java/lang/ProcessEnvironment.java line 73:
>>
>>> 71: @SuppressWarnings("removal")
>>> 72: String jnuEncoding =
>>> AccessController.doPrivileged((PrivilegedAction) ()
>>> 73: -> System.getProperty("sun.jnu.encoding"));
>>
>> No need to issue `doPrivileged()`, which is deprecated
>
> Hello @naotoj .
> If I just use `System.getProperty("sun.jnu.encoding")`, following testcases
> were failed.
>
> java/lang/ProcessBuilder/SecurityManagerClinit.java
> java/lang/ProcessHandle/PermissionTest.java
>
> Please give me your suggestion again.
Ah, OK. Never mind.
-
PR: https://git.openjdk.java.net/jdk/pull/8378
RFR: JDK-8286191: misc tests fail due to JDK-8286191
The isMusl method had to be handled in test/lib-test/jdk/test/lib/TestMutuallyExclusivePlatformPredicates.java . Additionally, the vm.musl predicate seem not to be available in the langtools tests. - Commit messages: - remove from ProblemList - Merge remote-tracking branch 'origin/master' into JDK-8286191 - JDK-8286191 Changes: https://git.openjdk.java.net/jdk/pull/8556/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8556=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8286191 Stats: 4 lines in 3 files changed: 0 ins; 3 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/8556.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8556/head:pull/8556 PR: https://git.openjdk.java.net/jdk/pull/8556
RFR: 8286199: ProblemList jdk/jshell/ExternalEditorTest.java
Put jdk/jshell/ExternalEditorTest.java on the problem list due to 8286191. - Commit messages: - 8286199: ProblemList jdk/jshell/ExternalEditorTest.java - 8286195: ProblemList test/lib-test/jdk/test/lib/TestMutuallyExclusivePlatformPredicates.java Changes: https://git.openjdk.java.net/jdk/pull/8557/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8557=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8286199 Stats: 3 lines in 2 files changed: 2 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/8557.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8557/head:pull/8557 PR: https://git.openjdk.java.net/jdk/pull/8557
Re: RFR: JDK-8285987: executing shell scripts without #! fails on Alpine linux
On Thu, 5 May 2022 15:24:25 GMT, Martin Doerr wrote: > Strange. The pre-submit tests are all green. Yes sorry those seem not to cover these 2 findings. And I think I locally run by mistake only the jdk test instead of the jdk + langtools tests with my script , this will of course not show the langtools issue. The green langtools/tier1 from presubmit might not include the ExternalEditorTest . - PR: https://git.openjdk.java.net/jdk/pull/8535
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). Strange. The pre-submit tests are all green. - PR: https://git.openjdk.java.net/jdk/pull/8535
Re: RFR: 8279986: methods Math::asXExact for safely checked primitive casts
On Thu, 5 May 2022 15:07:32 GMT, Michael Hixson wrote: > Rationale: It loses information. It truncates the sign, so the value can't be > round-tripped. I think it would be the only lossy transformation permitted by > the to*Exact methods? Right. - PR: https://git.openjdk.java.net/jdk/pull/8548
Re: RFR: JDK-8285987: executing shell scripts without #! fails on Alpine linux
On Thu, 5 May 2022 14:47:22 GMT, Roger Riggs wrote: >> Hi Alan, thanks for the advice; do you think we can put it into the IGNORED >> group ? >> https://github.com/openjdk/jdk/blob/master/test/lib-test/jdk/test/lib/TestMutuallyExclusivePlatformPredicates.java#L53 > > Its now on the ProblemList and Created Issue: > [JDK-8286191](https://bugs.openjdk.java.net/browse/JDK-8286191) Test library > test failure in TestMutuallyExclusivePlatformPredicates.java This PR also caused a test failure in ExternalEditorTest because there is no defined value for "vm.musl" used in @\requires. What test were run before integration? - PR: https://git.openjdk.java.net/jdk/pull/8535
Re: RFR: 8279986: methods Math::asXExact for safely checked primitive casts
On Thu, 5 May 2022 14:32:36 GMT, Raffaello Giulietti wrote: > So, what is the use case or the rationale for throwing on -0.0? Rationale: It loses information. It truncates the sign, so the value can't be round-tripped. I think it would be the only lossy transformation permitted by the `to*Exact` methods? Use case: None. I haven't worked on a program where -0.0 -> OL -> +0.0 would have caused me a problem. - PR: https://git.openjdk.java.net/jdk/pull/8548
Re: RFR: JDK-8285987: executing shell scripts without #! fails on Alpine linux
On Thu, 5 May 2022 14:15:30 GMT, Matthias Baesken wrote:
>> 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.
>
> Hi Alan, thanks for the advice; do you think we can put it into the IGNORED
> group ?
> https://github.com/openjdk/jdk/blob/master/test/lib-test/jdk/test/lib/TestMutuallyExclusivePlatformPredicates.java#L53
Its now on the ProblemList and Created Issue:
[JDK-8286191](https://bugs.openjdk.java.net/browse/JDK-8286191) Test library
test failure in TestMutuallyExclusivePlatformPredicates.java
-
PR: https://git.openjdk.java.net/jdk/pull/8535
Integrated: 8286195: ProblemList test/lib-test/jdk/test/lib/TestMutuallyExclusivePlatformPredicates.java
On Thu, 5 May 2022 14:33:51 GMT, Roger Riggs wrote: > Add a failing test library test to the ProblemList. > > test/lib-test/jdk/test/lib/TestMutuallyExclusivePlatformPredicates.java This pull request has now been integrated. Changeset: 7022543f Author:Roger Riggs URL: https://git.openjdk.java.net/jdk/commit/7022543fcfa04c628ef962749ed96c8f986dc822 Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod 8286195: ProblemList test/lib-test/jdk/test/lib/TestMutuallyExclusivePlatformPredicates.java Reviewed-by: dcubed, lancea - PR: https://git.openjdk.java.net/jdk/pull/8552
Re: RFR: 8286195: ProblemList test/lib-test/jdk/test/lib/TestMutuallyExclusivePlatformPredicates.java
On Thu, 5 May 2022 14:33:51 GMT, Roger Riggs wrote: > Add a failing test library test to the ProblemList. > > test/lib-test/jdk/test/lib/TestMutuallyExclusivePlatformPredicates.java Marked as reviewed by lancea (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8552
Re: RFR: 8286195: ProblemList test/lib-test/jdk/test/lib/TestMutuallyExclusivePlatformPredicates.java
On Thu, 5 May 2022 14:33:51 GMT, Roger Riggs wrote: > Add a failing test library test to the ProblemList. > > test/lib-test/jdk/test/lib/TestMutuallyExclusivePlatformPredicates.java Thumbs up. This is a trivial fix. - Marked as reviewed by dcubed (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8552
RFR: 8286195: ProblemList test/lib-test/jdk/test/lib/TestMutuallyExclusivePlatformPredicates.java
Add a failing test library test to the ProblemList. test/lib-test/jdk/test/lib/TestMutuallyExclusivePlatformPredicates.java - Commit messages: - 8286195: ProblemList test/lib-test/jdk/test/lib/TestMutuallyExclusivePlatformPredicates.java Changes: https://git.openjdk.java.net/jdk/pull/8552/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8552=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8286195 Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/8552.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8552/head:pull/8552 PR: https://git.openjdk.java.net/jdk/pull/8552
Re: RFR: 8279986: methods Math::asXExact for safely checked primitive casts
On Thu, 5 May 2022 10:11:05 GMT, Raffaello Giulietti wrote: > Add a family of "safe" cast methods. The JLS specifies that the cast (officially, "narrowing primitive conversion") (long)-0.0 returns 0L. As these methods are meant to be safer casts, I think we should follow the JLS as closely as possible. Besides, throwing on -0.0 would make the implementation slightly more convoluted. We want C2 to emit efficient inlineable code. So, what is the use case or the rationale for throwing on -0.0? - PR: https://git.openjdk.java.net/jdk/pull/8548
Re: RFR: 8279986: methods Math::asXExact for safely checked primitive casts
On Thu, 5 May 2022 10:11:05 GMT, Raffaello Giulietti wrote: > Add a family of "safe" cast methods. This PR also solves [JDK-8154433](https://bugs.openjdk.java.net/browse/JDK-8154433), though you went the other way on -0.0. - PR: https://git.openjdk.java.net/jdk/pull/8548
Re: RFR: JDK-8285987: executing shell scripts without #! fails on Alpine linux
On Thu, 5 May 2022 13:43:30 GMT, Alan Bateman 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.
Hi Alan, thanks for the advice; do you think we can put it into the IGNORED
group ?
https://github.com/openjdk/jdk/blob/master/test/lib-test/jdk/test/lib/TestMutuallyExclusivePlatformPredicates.java#L53
-
PR: https://git.openjdk.java.net/jdk/pull/8535
Re: RFR: 8285295: Need better testing for IdentityHashMap [v4]
On Wed, 4 May 2022 19:16:14 GMT, Stuart Marks wrote: >> Basic but fairly comprehensive set of tests for `IdentityHashMap`. The patch >> in the bug report that breaks `IdentityHashMap` now causes several cases in >> this new test to fail. There's more that could be done, but the new tests >> cover most of the core functions of `IdentityHashMap`. Unfortunately it >> seems difficult to merge this with the existing, comprehensive Collections >> tests (e.g., MOAT.java) because those tests implicity rely on >> `equals()`-based contract instead of the special-purpose `==`-based contract >> used by `IdentityHashMap`. > > Stuart Marks has updated the pull request incrementally with one additional > commit since the last revision: > > Add some assertions for entrySet.equals and keySet.equals As I said in [GH‑6532]: > There should probably be something like > [test/jdk/java/util/Collections/Wrappers.java] to check that > `IdentityHashMap` overrides all `default` methods from `java.util.Map` (with > `remove(K, V)` and `replace(K, V, V)` depending on [GH‑8259]). [GH‑6532]: https://github.com/openjdk/jdk/pull/6532#issuecomment-1104709021 [GH‑8259]: https://github.com/openjdk/jdk/pull/8259 [test/jdk/java/util/Collections/Wrappers.java]: https://github.com/openjdk/jdk/blob/master/test/jdk/java/util/Collections/Wrappers.java - PR: https://git.openjdk.java.net/jdk/pull/8354
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
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. Anybody interested in reviewing? - PR: https://git.openjdk.java.net/jdk/pull/8290
Integrated: 8282060: RemoteRuntimeImageTest is not actually testing on JDK 8
On Thu, 5 May 2022 09:00:47 GMT, Athijegannathan Sundararajan wrote: > This test requires jdk8 to be available while running jdk tests. But > JDK8_HOME is defined to be BOOT_JDK and so version check fails in the test. > The test vacuously passes just printing a message. There are already tests > that exercise jrt-fs.jar on the same JDK being tested and so basic jrt-fs.jar > is covered for same target JDK case. This pull request has now been integrated. Changeset: ede06c3c Author:Athijegannathan Sundararajan URL: https://git.openjdk.java.net/jdk/commit/ede06c3c5f74c64dac3889d35b02541897ba4d94 Stats: 202 lines in 3 files changed: 0 ins; 202 del; 0 mod 8282060: RemoteRuntimeImageTest is not actually testing on JDK 8 Reviewed-by: alanb, erikj - PR: https://git.openjdk.java.net/jdk/pull/8547
Integrated: 8285947: Avoid redundant HashMap.containsKey calls in ZoneName
On Fri, 29 Apr 2022 06:31:22 GMT, Andrey Turbanov wrote:
> `Map.containsKey` call is sometimes unnecessary, when it's known that Map
> doesn't contain `null` values.
> Instead we can just use Map.get and compare result with `null`.
> I found one of such place, where Map.containsKey calls could be eliminated -
> `java.time.format.ZoneName`.
> it gives a bit of performance for `toZid`.
>
>
> Benchmark Mode Cnt Score Error Units
> ZoneNamesBench.useExistingCountry avgt5 10,738 ± 0,065 ns/op
> ZoneNamesBench.useExistingCountryOld avgt5 13,679 ± 0,089 ns/op
>
>
>
> Benchmark
>
>
> @BenchmarkMode(value = Mode.AverageTime)
> @OutputTimeUnit(TimeUnit.NANOSECONDS)
> @Warmup(iterations = 10, time = 1, timeUnit = TimeUnit.SECONDS)
> @Measurement(iterations = 5, time = 1, timeUnit = TimeUnit.SECONDS)
> @Fork(1)
> @State(Scope.Benchmark)
> public class ZoneNamesBench {
>
> @Benchmark
> public String useExistingCountry() {
> return ZoneName.toZid("Africa/Tunis", Locale.ITALY);
> }
>
> @Benchmark
> public String useExistingCountryOld() {
> return ZoneName.toZidOld("Africa/Tunis", Locale.ITALY);
> }
> }
>
>
>
> public static String toZid(String zid, Locale locale) {
> String mzone = zidToMzone.get(zid);
> if (mzone == null) {
> String alias = aliases.get(zid);
> if (alias != null) {
> zid = alias;
> mzone = zidToMzone.get(zid);
> }
> }
> if (mzone != null) {
> Map map = mzoneToZidL.get(mzone);
> if (map == null || ((zid = map.get(locale.getCountry())) ==
> null)) {
> zid = mzoneToZid.get(mzone);
> }
> }
> return toZid(zid);
> }
>
> public static String toZid(String zid) {
> return aliases.getOrDefault(zid, zid);
> }
>
> public static String toZidOld(String zid, Locale locale) {
> String mzone = zidToMzone.get(zid);
> if (mzone == null && aliases.containsKey(zid)) {
> zid = aliases.get(zid);
> mzone = zidToMzone.get(zid);
> }
> if (mzone != null) {
> Map map = mzoneToZidL.get(mzone);
> if (map != null && map.containsKey(locale.getCountry())) {
> zid = map.get(locale.getCountry());
> } else {
> zid = mzoneToZid.get(mzone);
> }
> }
> return toZidOld(zid);
> }
>
> public static String toZidOld(String zid) {
> if (aliases.containsKey(zid)) {
> return aliases.get(zid);
> }
> return zid;
> }
>
>
This pull request has now been integrated.
Changeset: dce860aa
Author:Andrey Turbanov
URL:
https://git.openjdk.java.net/jdk/commit/dce860aa8a6b11501e82ace4ff016ee49d8e1fa4
Stats: 14 lines in 1 file changed: 3 ins; 5 del; 6 mod
8285947: Avoid redundant HashMap.containsKey calls in ZoneName
Reviewed-by: scolebourne, naoto, rriggs
-
PR: https://git.openjdk.java.net/jdk/pull/8463
Integrated: 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). This pull request has now been integrated. Changeset: 9d2f591e Author:Matthias Baesken URL: https://git.openjdk.java.net/jdk/commit/9d2f591e6a15dc155a8cc3b18a54456d5f9a3aa7 Stats: 35 lines in 3 files changed: 17 ins; 0 del; 18 mod 8285987: executing shell scripts without #! fails on Alpine linux Reviewed-by: mdoerr, goetz - PR: https://git.openjdk.java.net/jdk/pull/8535
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). Hi Martin and Goetz, thanks for the reviews. - PR: https://git.openjdk.java.net/jdk/pull/8535
Re: RFR: 8282060: RemoteRuntimeImageTest is not actually testing on JDK 8
On Thu, 5 May 2022 09:00:47 GMT, Athijegannathan Sundararajan wrote: > This test requires jdk8 to be available while running jdk tests. But > JDK8_HOME is defined to be BOOT_JDK and so version check fails in the test. > The test vacuously passes just printing a message. There are already tests > that exercise jrt-fs.jar on the same JDK being tested and so basic jrt-fs.jar > is covered for same target JDK case. Marked as reviewed by erikj (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8547
Re: RFR: 8262889: Compiler implementation for Record Patterns
On Thu, 5 May 2022 12:16:23 GMT, Maurizio Cimadamore wrote: >> I think this is i) from the domination relation: >> >>> A record pattern with type R and record component pattern list L dominates >>> another record pattern with type S and record component pattern list M if >>> (i) the erasure of S is a subtype of the erasure of R, and (ii) every >>> pattern, if any, in L dominates the corresponding pattern in M. > >> I think this is i) from the domination relation: >> >> > A record pattern with type R and record component pattern list L dominates >> > another record pattern with type S and record component pattern list M if >> > (i) the erasure of S is a subtype of the erasure of R, and (ii) every >> > pattern, if any, in L dominates the corresponding pattern in M. > > But this subtyping test seems to happen at the level of the component pattern > list, not at the R/S level, right? You are right. It is the ii) which iteratively checks the component pattern list L. - PR: https://git.openjdk.java.net/jdk/pull/8516
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). Looks good - Marked as reviewed by goetz (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8535
Re: RFR: 8262889: Compiler implementation for Record Patterns
On Thu, 5 May 2022 12:06:38 GMT, Aggelos Biboudis wrote: > I think this is i) from the domination relation: > > > A record pattern with type R and record component pattern list L dominates > > another record pattern with type S and record component pattern list M if > > (i) the erasure of S is a subtype of the erasure of R, and (ii) every > > pattern, if any, in L dominates the corresponding pattern in M. But this subtyping test seems to happen at the level of the component pattern list, not at the R/S level, right? - PR: https://git.openjdk.java.net/jdk/pull/8516
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). LGTM. - Marked as reviewed by mdoerr (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8535
Re: RFR: 8262889: Compiler implementation for Record Patterns
On Tue, 3 May 2022 12:07:50 GMT, Jan Lahoda wrote:
> 8262889: Compiler implementation for Record Patterns
>
> A first version of a patch that introduces record patterns into javac as a
> preview feature. For the specification, please see:
> http://cr.openjdk.java.net/~gbierman/jep427+405/jep427+405-20220426/specs/patterns-switch-record-patterns-jls.html
>
> There are two notable tricky parts:
> -in the parser, it was necessary to improve the `analyzePattern` method to
> handle nested/record patterns, while still keeping error recovery reasonable
> -in the `TransPatterns`, the desugaring of the record patterns is very
> straightforward - effectivelly the record patterns are desugared into
> guards/conditions. This will likely be improved in some future version/preview
>
> `MatchException` has been extended to cover additional cases related to
> record patterns.
src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Flow.java line 783:
> 781: }
> 782: for (Entry> e :
> categorizedDeconstructionPatterns.entrySet()) {
> 783: if (coversDeconstructionStartingFromComponent(pos,
> targetType, e.getValue(), 0)) {
Here, the result of `e.getValue` is a reversed list of the observed patterns.
For the switch below,
switch (r) {
case R(A a, A b) -> 1;
case R(A a, B b) -> 2;
case R(B a, A b) -> 3;
case R(B a, B b) -> 4;
}
The 0th element of that list is the `R(B a, B b)` pattern, the 1st the `R(B a,
A b)`. I am 99% sure that this is not a problem but I am pointing it out
regardless, in case there is any underlying danger to that.
-
PR: https://git.openjdk.java.net/jdk/pull/8516
Re: RFR: 8262889: Compiler implementation for Record Patterns
On Wed, 4 May 2022 10:51:38 GMT, Maurizio Cimadamore
wrote:
>> 8262889: Compiler implementation for Record Patterns
>>
>> A first version of a patch that introduces record patterns into javac as a
>> preview feature. For the specification, please see:
>> http://cr.openjdk.java.net/~gbierman/jep427+405/jep427+405-20220426/specs/patterns-switch-record-patterns-jls.html
>>
>> There are two notable tricky parts:
>> -in the parser, it was necessary to improve the `analyzePattern` method to
>> handle nested/record patterns, while still keeping error recovery reasonable
>> -in the `TransPatterns`, the desugaring of the record patterns is very
>> straightforward - effectivelly the record patterns are desugared into
>> guards/conditions. This will likely be improved in some future
>> version/preview
>>
>> `MatchException` has been extended to cover additional cases related to
>> record patterns.
>
> src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Flow.java line 824:
>
>> 822: }
>> 823: for (Symbol currentType : nestedCovered) {
>> 824: if (types.isSubtype(types.erasure(currentType.type),
>
> Not 100% what this test does
I think this is i) from the domination relation:
> A record pattern with type R and record component pattern list L dominates
> another record pattern with type S and record component pattern list M if (i)
> the erasure of S is a subtype of the erasure of R, and (ii) every pattern, if
> any, in L dominates the corresponding pattern in M.
-
PR: https://git.openjdk.java.net/jdk/pull/8516
Re: RFR: 8285295: Need better testing for IdentityHashMap [v2]
On Wed, 4 May 2022 15:02:43 GMT, liach wrote:
>> test/jdk/java/util/IdentityHashMap/Basic.java line 500:
>>
>>> 498: Box newKey = new Box(k1a);
>>> 499: Box newVal = new Box(v1a);
>>> 500: Box r = map.computeIfAbsent(newKey, k -> { called[0] = true;
>>> return newVal; });
>>
>> More of a curiosity than a review comment - I see that various places in
>> this PR use a boolean array with one element instead of just a boolean type.
>> Is that a personal coding preference or is there something more to it?
>
> This just serves as a modifiable boolean like an AtomicBoolean. Remember
> lambdas can only use final local var references (due to how they work), and
> it cannot access or modify the local variable in the caller method.
Thank you @liach and Stuart. I had overlooked the lambda aspect of this.
-
PR: https://git.openjdk.java.net/jdk/pull/8354
Re: RFR: 8285295: Need better testing for IdentityHashMap [v4]
On Wed, 4 May 2022 19:16:14 GMT, Stuart Marks wrote: >> Basic but fairly comprehensive set of tests for `IdentityHashMap`. The patch >> in the bug report that breaks `IdentityHashMap` now causes several cases in >> this new test to fail. There's more that could be done, but the new tests >> cover most of the core functions of `IdentityHashMap`. Unfortunately it >> seems difficult to merge this with the existing, comprehensive Collections >> tests (e.g., MOAT.java) because those tests implicity rely on >> `equals()`-based contract instead of the special-purpose `==`-based contract >> used by `IdentityHashMap`. > > Stuart Marks has updated the pull request incrementally with one additional > commit since the last revision: > > Add some assertions for entrySet.equals and keySet.equals Hello Stuart, these changes look good to me. - Marked as reviewed by jpai (Committer). PR: https://git.openjdk.java.net/jdk/pull/8354
Re: RFR: 8279986: methods Math::asXExact for safely checked primitive casts
On Thu, 5 May 2022 10:11:05 GMT, Raffaello Giulietti wrote: > Add a family of "safe" cast methods. For an in-depth rationale, please refer to the issue (Enhancement). Note that the method names are, in fact, of the to*Exact rather than of the as*Exact form - PR: https://git.openjdk.java.net/jdk/pull/8548
RFR: 8279986: methods Math::asXExact for safely checked primitive casts
Add a family of "safe" cast methods. - Commit messages: - 8279986: methods Math::asXExact for safely checked primitive casts - 8279986: methods Math::asXExact for safely checked primitive casts - 8279986: methods Math::asXExact for safely checked primitive casts Changes: https://git.openjdk.java.net/jdk/pull/8548/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8548=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8279986 Stats: 615 lines in 2 files changed: 609 ins; 0 del; 6 mod Patch: https://git.openjdk.java.net/jdk/pull/8548.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8548/head:pull/8548 PR: https://git.openjdk.java.net/jdk/pull/8548
Re: RFR: 8282060: RemoteRuntimeImageTest is not actually testing on JDK 8
On Thu, 5 May 2022 09:00:47 GMT, Athijegannathan Sundararajan wrote: > This test requires jdk8 to be available while running jdk tests. But > JDK8_HOME is defined to be BOOT_JDK and so version check fails in the test. > The test vacuously passes just printing a message. There are already tests > that exercise jrt-fs.jar on the same JDK being tested and so basic jrt-fs.jar > is covered for same target JDK case. Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8547
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v11]
On Wed, 4 May 2022 16:02:36 GMT, Aleksey Shipilev wrote: > So, does this PR pass current GHA checks? I see they are not enabled for this > PR. It would be unfortunate for this large integration to break builds/tests > for smaller PRs that would follow it. I've enabled it on my fork and we'll see if it kicks in. - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v12]
> 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 19 commits: - Refresh 12aa09ce7efd48425cc080d0b8761aca0f3e215f - Merge with 1bb4de2e2868a539846ec48dd43fd623c2ba69a5 - Refresh d77f7a49af75bcc5b20686805ff82a93a20dde05 - Merge with 4b2c82200fdc01de868cf414e40a4d891e753f89 - Refresh 6091080db743ece5f1b2111fcc35a5f2179a403a - Merge with cfcba1fccc8e3e6a68e1cb1826b70e076d5d83c4 - Refresh ee9fa8ed05ec22de7a13383052d68aa8aa7832ec - Merge with jdk-19+20 - Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e - Refresh 8d8f0a2fd646e57fe6b4e8ab669f836dc46dda69 - ... and 9 more: https://git.openjdk.java.net/jdk/compare/1bb4de2e...86bea8ec - Changes: https://git.openjdk.java.net/jdk/pull/8166/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8166=11 Stats: 99452 lines in 1130 files changed: 91184 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: 8282648: Problems due to conflicting specification of Inflater::inflate(..) and InflaterInputStream::read(..) [v2]
On Wed, 30 Mar 2022 10:26:56 GMT, Lance Andersen wrote:
>>> Hello Volker, An additional thing that we might have to consider here is
>>> whether adding this javadoc change to `InflaterInputStream` is ever going
>>> to "show up" to end user applications. What I mean is, I think in many
>>> cases the end user applications won't even know they are dealing with an
>>> `InflaterInputStream`. For example, the following code:
>>>
>>> ```
>>> ZipFile zf = ...
>>> ZipEntry ze = zf.getEntry("some-file");
>>> InputStream is = zf.getInputStream(ze);
>>> ```
>>>
>>> As we see above, none of these APIs talk about `InflaterInputStream` (the
>>> return type of `ZipFile.getInpustream(...)` is an `InputStream`). So end
>>> users won't be able to view this spec change. Perhaps we should also add
>>> some note in the `ZipFile.getInpustream()` API to make a mention of
>>> this potential difference in behaviour of the returned stream?
>>
>> You are right with your observation and I'll be happy to add a corresponding
>> comment if @LanceAndersen and @AlanBateman agree. Please let me know what
>> you think?
>
>> > Hello Volker, An additional thing that we might have to consider here is
>> > whether adding this javadoc change to `InflaterInputStream` is ever going
>> > to "show up" to end user applications. What I mean is, I think in many
>> > cases the end user applications won't even know they are dealing with an
>> > `InflaterInputStream`. For example, the following code:
>> > ```
>> > ZipFile zf = ...
>> > ZipEntry ze = zf.getEntry("some-file");
>> > InputStream is = zf.getInputStream(ze);
>> > ```
>> >
>> >
>> >
>> >
>> >
>> >
>> >
>> >
>> >
>> >
>> >
>> > As we see above, none of these APIs talk about `InflaterInputStream` (the
>> > return type of `ZipFile.getInpustream(...)` is an `InputStream`). So end
>> > users won't be able to view this spec change. Perhaps we should also add
>> > some note in the `ZipFile.getInpustream()` API to make a mention of
>> > this potential difference in behaviour of the returned stream?
>>
>> You are right with your observation and I'll be happy to add a corresponding
>> comment if @LanceAndersen and @AlanBateman agree. Please let me know what
>> you think?
>
> Hi Volker,
>
> I believe Jai raises a valid point given these javadocs probably have had
> limited updates if any since the API was originally added.We should look
> at ZipInputStream and GZipInputStream as well if we decide to update the
> ZipFile::getInputStream(where we could borrow some wording from the
> ZipInputStream class description as a start to some word smithing).
>
> As Roger points out we will need a release note for this change as well.
@LanceAndersen, @AlanBateman can you please comment on the
[CSR](https://bugs.openjdk.java.net/browse/JDK-8283758) for this issue. We now
circled back to the initial proposal in the
[CSR](https://bugs.openjdk.java.net/browse/JDK-8283758) but @jddarcy would
like to hear your opinion.
-
PR: https://git.openjdk.java.net/jdk/pull/7986
RFR: 8282060: RemoteRuntimeImageTest is not actually testing on JDK 8
This test requires jdk8 to be available while running jdk tests. But JDK8_HOME is defined to be BOOT_JDK and so version check fails in the test. The test vacuously passes just printing a message. There are already tests that exercise jrt-fs.jar on the same JDK being tested and so basic jrt-fs.jar is covered for same target JDK case. - Commit messages: - 8282060: RemoteRuntimeImageTest is not actually testing on JDK 8 Changes: https://git.openjdk.java.net/jdk/pull/8547/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8547=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8282060 Stats: 202 lines in 3 files changed: 0 ins; 202 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/8547.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8547/head:pull/8547 PR: https://git.openjdk.java.net/jdk/pull/8547
Re: RFR: 8283667: [vectorapi] Vectorization for masked load with IOOBE with predicate feature [v3]
> Currently the vector load with mask when the given index happens out of the
> array boundary is implemented with pure java scalar code to avoid the IOOBE
> (IndexOutOfBoundaryException). This is necessary for architectures that do
> not support the predicate feature. Because the masked load is implemented
> with a full vector load and a vector blend applied on it. And a full vector
> load will definitely cause the IOOBE which is not valid. However, for
> architectures that support the predicate feature like SVE/AVX-512/RVV, it can
> be vectorized with the predicated load instruction as long as the indexes of
> the masked lanes are within the bounds of the array. For these architectures,
> loading with unmasked lanes does not raise exception.
>
> This patch adds the vectorization support for the masked load with IOOBE
> part. Please see the original java implementation (FIXME: optimize):
>
>
> @ForceInline
> public static
> ByteVector fromArray(VectorSpecies species,
>byte[] a, int offset,
>VectorMask m) {
> ByteSpecies vsp = (ByteSpecies) species;
> if (offset >= 0 && offset <= (a.length - species.length())) {
> return vsp.dummyVector().fromArray0(a, offset, m);
> }
>
> // FIXME: optimize
> checkMaskFromIndexSize(offset, vsp, m, 1, a.length);
> return vsp.vOp(m, i -> a[offset + i]);
> }
>
> Since it can only be vectorized with the predicate load, the hotspot must
> check whether the current backend supports it and falls back to the java
> scalar version if not. This is different from the normal masked vector load
> that the compiler will generate a full vector load and a vector blend if the
> predicate load is not supported. So to let the compiler make the expected
> action, an additional flag (i.e. `usePred`) is added to the existing
> "loadMasked" intrinsic, with the value "true" for the IOOBE part while
> "false" for the normal load. And the compiler will fail to intrinsify if the
> flag is "true" and the predicate load is not supported by the backend, which
> means that normal java path will be executed.
>
> Also adds the same vectorization support for masked:
> - fromByteArray/fromByteBuffer
> - fromBooleanArray
> - fromCharArray
>
> The performance for the new added benchmarks improve about `1.88x ~ 30.26x`
> on the x86 AVX-512 system:
>
> Benchmark before After Units
> LoadMaskedIOOBEBenchmark.byteLoadArrayMaskIOOBE 737.542 1387.069 ops/ms
> LoadMaskedIOOBEBenchmark.doubleLoadArrayMaskIOOBE 118.366 330.776 ops/ms
> LoadMaskedIOOBEBenchmark.floatLoadArrayMaskIOOBE 233.832 6125.026 ops/ms
> LoadMaskedIOOBEBenchmark.intLoadArrayMaskIOOBE233.816 7075.923 ops/ms
> LoadMaskedIOOBEBenchmark.longLoadArrayMaskIOOBE 119.771 330.587 ops/ms
> LoadMaskedIOOBEBenchmark.shortLoadArrayMaskIOOBE 431.961 939.301 ops/ms
>
> Similar performance gain can also be observed on 512-bit SVE system.
Xiaohong Gong has updated the pull request incrementally with one additional
commit since the last revision:
Rename "use_predicate" to "needs_predicate"
-
Changes:
- all: https://git.openjdk.java.net/jdk/pull/8035/files
- new: https://git.openjdk.java.net/jdk/pull/8035/files/9b2d2f19..9c69206e
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jdk=8035=02
- incr: https://webrevs.openjdk.java.net/?repo=jdk=8035=01-02
Stats: 4 lines in 1 file changed: 0 ins; 0 del; 4 mod
Patch: https://git.openjdk.java.net/jdk/pull/8035.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/8035/head:pull/8035
PR: https://git.openjdk.java.net/jdk/pull/8035
Re: RFR: 8283667: [vectorapi] Vectorization for masked load with IOOBE with predicate feature [v2]
On Thu, 5 May 2022 02:14:08 GMT, Xiaohong Gong wrote:
>> src/hotspot/share/opto/vectorIntrinsics.cpp line 1232:
>>
>>> 1230: // out when current case uses the predicate feature.
>>> 1231: if (!supports_predicate) {
>>> 1232: bool use_predicate = false;
>>
>> If we rename this to needs_predicate it will be easier to understand.
>
> Thanks for the comment! This local variable will be removed after adding the
> similar intrinsify for store masked. Please help to see the PR
> https://github.com/openjdk/jdk/pull/8544. Thanks so much!
Renamed to "needs_predicate". Thanks!
-
PR: https://git.openjdk.java.net/jdk/pull/8035
Withdrawn: 8284050: [vectorapi] Optimize masked store for non-predicated architectures
On Thu, 5 May 2022 02:00:04 GMT, Xiaohong Gong wrote: > Currently the vectorization of masked vector store is implemented by the > masked store instruction only on architectures that support the predicate > feature. The compiler will fall back to the java scalar code for > non-predicate supported architectures like ARM NEON. However, for these > systems, the masked store can be vectorized with the non-masked vector `"load > + blend + store"`. For example, storing a vector` "v"` controlled by a mask` > "m"` into a memory with address` "addr" (i.e. "store(addr, v, m)")` can be > implemented with: > > > 1) mem_v = load(addr) ; non-masked load from the same memory > 2) v = blend(mem_v, v, m) ; blend with the src vector with the mask > 3) store(addr, v) ; non-masked store into the memory > > > Since the first full loading needs the array offset must be inside of the > valid array bounds, we make the compiler do the vectorization only when the > offset is in range of the array boundary. And the compiler will still fall > back to the java scalar code if not all offsets are valid. Besides, the > original offset check for masked lanes are only applied when the offset is > not always inside of the array range. This also improves the performance for > masked store when the offset is always valid. The whole process is similar to > the masked load API. > > Here is the performance data for the masked vector store benchmarks on a X86 > non avx-512 system, which improves about `20x ~ 50x`: > > Benchmark beforeafter Units > StoreMaskedBenchmark.byteStoreArrayMask 221.733 11094.126 ops/ms > StoreMaskedBenchmark.doubleStoreArrayMask 41.086 1034.408 ops/ms > StoreMaskedBenchmark.floatStoreArrayMask 73.820 1985.015 ops/ms > StoreMaskedBenchmark.intStoreArrayMask 75.028 2027.557 ops/ms > StoreMaskedBenchmark.longStoreArrayMask40.929 1032.928 ops/ms > StoreMaskedBenchmark.shortStoreArrayMask 135.794 5307.567 ops/ms > > Similar performance gain can also be observed on ARM NEON system. > > And here is the performance data on X86 avx-512 system, which improves about > `1.88x - 2.81x`: > > Benchmark before after Units > StoreMaskedBenchmark.byteStoreArrayMask 11185.956 21012.824 ops/ms > StoreMaskedBenchmark.doubleStoreArrayMask 1480.644 3911.720 ops/ms > StoreMaskedBenchmark.floatStoreArrayMask 2738.352 7708.365 ops/ms > StoreMaskedBenchmark.intStoreArrayMask 4191.904 9300.428 ops/ms > StoreMaskedBenchmark.longStoreArrayMask2025.031 4604.504 ops/ms > StoreMaskedBenchmark.shortStoreArrayMask 8339.389 17817.128 ops/ms > > Similar performance gain can also be observed on ARM SVE system. This pull request has been closed without being integrated. - PR: https://git.openjdk.java.net/jdk/pull/8544
Re: RFR: 8284050: [vectorapi] Optimize masked store for non-predicated architectures [v2]
On Thu, 5 May 2022 02:09:39 GMT, Xiaohong Gong wrote: >> Currently the vectorization of masked vector store is implemented by the >> masked store instruction only on architectures that support the predicate >> feature. The compiler will fall back to the java scalar code for >> non-predicate supported architectures like ARM NEON. However, for these >> systems, the masked store can be vectorized with the non-masked vector >> `"load + blend + store"`. For example, storing a vector` "v"` controlled by >> a mask` "m"` into a memory with address` "addr" (i.e. "store(addr, v, m)")` >> can be implemented with: >> >> >> 1) mem_v = load(addr) ; non-masked load from the same memory >> 2) v = blend(mem_v, v, m) ; blend with the src vector with the mask >> 3) store(addr, v) ; non-masked store into the memory >> >> >> Since the first full loading needs the array offset must be inside of the >> valid array bounds, we make the compiler do the vectorization only when the >> offset is in range of the array boundary. And the compiler will still fall >> back to the java scalar code if not all offsets are valid. Besides, the >> original offset check for masked lanes are only applied when the offset is >> not always inside of the array range. This also improves the performance for >> masked store when the offset is always valid. The whole process is similar >> to the masked load API. >> >> Here is the performance data for the masked vector store benchmarks on a X86 >> non avx-512 system, which improves about `20x ~ 50x`: >> >> Benchmark beforeafter Units >> StoreMaskedBenchmark.byteStoreArrayMask 221.733 11094.126 ops/ms >> StoreMaskedBenchmark.doubleStoreArrayMask 41.086 1034.408 ops/ms >> StoreMaskedBenchmark.floatStoreArrayMask 73.820 1985.015 ops/ms >> StoreMaskedBenchmark.intStoreArrayMask 75.028 2027.557 ops/ms >> StoreMaskedBenchmark.longStoreArrayMask40.929 1032.928 ops/ms >> StoreMaskedBenchmark.shortStoreArrayMask 135.794 5307.567 ops/ms >> >> Similar performance gain can also be observed on ARM NEON system. >> >> And here is the performance data on X86 avx-512 system, which improves about >> `1.88x - 2.81x`: >> >> Benchmark before after Units >> StoreMaskedBenchmark.byteStoreArrayMask 11185.956 21012.824 ops/ms >> StoreMaskedBenchmark.doubleStoreArrayMask 1480.644 3911.720 ops/ms >> StoreMaskedBenchmark.floatStoreArrayMask 2738.352 7708.365 ops/ms >> StoreMaskedBenchmark.intStoreArrayMask 4191.904 9300.428 ops/ms >> StoreMaskedBenchmark.longStoreArrayMask2025.031 4604.504 ops/ms >> StoreMaskedBenchmark.shortStoreArrayMask 8339.389 17817.128 ops/ms >> >> Similar performance gain can also be observed on ARM SVE system. > > Xiaohong Gong has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains one commit: > > 8284050: [vectorapi] Optimize masked store for non-predicated architectures > _Mailing list message from [John Rose](mailto:john.r.r...@oracle.com) on > [hotspot-dev](mailto:hotspot-...@mail.openjdk.java.net):_ > > > On May 4, 2022, at 8:29 PM, Xiaohong Gong wrote: > > The offset check could save the `checkMaskFromIndexSize` for cases that > > offset are in the valid array bounds, which also improves the performance. > > @rose00 , do you think this part of change is ok at least? > > That part is ok, yes. I wish we could get the same effect with loop > optimizations but I don?t know an easy way. The explicit check in the source > code gives the JIT a crutch but I hope we can figure out a way in the future > to integrate mask logic into range check elimination logic, making the > crutches unnecessary. For now it?s fine. Thanks! So I will separate this part out and fix it in another PR first. For the store masked vectorization with scatter or other ideas, I'm not quite sure whether they can always benefit cross architectures and need more investigation. I prefer to close this PR now. Thanks for all your comments! - PR: https://git.openjdk.java.net/jdk/pull/8544
Re: RFR: 8285517: System.getenv() returns unexpected value if environment variable has non ASCII character [v4]
On Thu, 5 May 2022 01:31:24 GMT, Naoto Sato wrote:
>> Ichiroh Takiguchi has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> 8285517: System.getenv() returns unexpected value if environment variable
>> has non ASCII character
>
> src/java.base/unix/classes/java/lang/ProcessEnvironment.java line 73:
>
>> 71: @SuppressWarnings("removal")
>> 72: String jnuEncoding =
>> AccessController.doPrivileged((PrivilegedAction) ()
>> 73: -> System.getProperty("sun.jnu.encoding"));
>
> No need to issue `doPrivileged()`, which is deprecated
Hello @naotoj .
If I just use `System.getProperty("sun.jnu.encoding")`, following testcases
were failed.
java/lang/ProcessBuilder/SecurityManagerClinit.java
java/lang/ProcessHandle/PermissionTest.java
Please give me your suggestion again.
-
PR: https://git.openjdk.java.net/jdk/pull/8378
Re: RFR: 8284050: [vectorapi] Optimize masked store for non-predicated architectures [v2]
> On May 4, 2022, at 8:29 PM, Xiaohong Gong wrote: > > The offset check could save the `checkMaskFromIndexSize` for cases that > offset are in the valid array bounds, which also improves the performance. > @rose00 , do you think this part of change is ok at least? That part is ok, yes. I wish we could get the same effect with loop optimizations but I don’t know an easy way. The explicit check in the source code gives the JIT a crutch but I hope we can figure out a way in the future to integrate mask logic into range check elimination logic, making the crutches unnecessary. For now it’s fine.
