Re: RFR: 8273797: Stop impersonating "server" VM in all VM variants
On Wed, 15 Sep 2021 10:02:19 GMT, Aleksey Shipilev wrote: > As the follow-up for Zero-specific JDK-8273494, we might want to clean up > build system logic for all VM variants: stop impersonating "server" VMs for > all of them. This basically leaves "core" and "custom" variants to be handled > with this patch. > > Additional testing: > - [x] Linux x86_64 `core` build passes, `libjvm.so` now in `core`, `jvm.cfg` > mentions `-core KNOWN` > - [x] Linux x86_64 `custom` (+serialgc,+compiler2) build passes, `libjvm.so` > now in `custom`, `jvm.cfg` mentions `-custom KNOWN` Thanks! - PR: https://git.openjdk.java.net/jdk/pull/5526
Re: RFR: 8272600: (test) Use native "sleep" in Basic.java [v6]
On Mon, 20 Sep 2021 13:01:29 GMT, Roger Riggs wrote:
>> The intermittent test in java/lang/ProcessBuilder/Basic.java has identified
>> unexpected messages from a child Java VM
>> as the cause of the test failure. Attempts to control the output of the
>> child VM have failed, the VM is unrepentant .
>>
>> There is no functionality in the child except to wait long enough for the
>> test to finish and the child is destroyed.
>> The fix is to switch from using a Java child to using a native child; a new
>> executable `sleepmillis`.
>
> Roger Riggs has updated the pull request incrementally with one additional
> commit since the last revision:
>
> The switch from a Java child to /bin/sleep caused another test
> to fail on Linux. The cleanup for a test used /usr/bin/pkill "sleep 60".
> A race between that cleanup and subsequent tests that used sleep 60 or 600
> could kill the sleep before the test of waitFor completed.
> Changing the test using pkill to use 59 seconds makes the test cleanup
> selective to the sleep spawned for that test.
> The test that was failing (lines 2626-2624) passes consistently.
Hi Roger,
One suggestion based on your latest change, but that can be handled later.
Otherwise this seems fine.
Thanks,
David
test/jdk/java/lang/ProcessBuilder/Basic.java line 2217:
> 2215: // A unique (59s) time is needed to avoid killing other
> sleep processes.
> 2216: final String[] cmd = { "/bin/bash", "-c", "(/bin/sleep
> 59)" };
> 2217: final String[] cmdkill = { "/bin/bash", "-c",
> "(/usr/bin/pkill -f \"sleep 59\")" };
Maybe future RFE but why do we even need pkill here when we can get the PID of
the sleep process we create and kill only that process?
-
Marked as reviewed by dholmes (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/5239
Re: RFR: 8231640: (prop) Canonical property storage [v21]
Thank you Joe. That link helped. I have now moved the CSR to the next state. -Jaikiran On 21/09/21 9:28 am, Joseph D. Darcy wrote: Hello Jaikiran, The CSR is in Draft state. As discussed in the CSR wiki (https://wiki.openjdk.java.net/display/csr/Main), the request needs to be moved by the assignee to either Finalized or Proposed state to request the CSR review the request. HTH, -Joe On 9/20/2021 8:46 PM, Jaikiran Pai wrote: On Sat, 18 Sep 2021 03:52:17 GMT, Jaikiran Pai wrote: The commit in this PR implements the proposal for enhancement that was discussed in the core-libs-dev mailing list recently[1], for https://bugs.openjdk.java.net/browse/JDK-8231640 At a high level - the `store()` APIs in `Properties` have been modified to now look for the `SOURCE_DATE_EPOCH` environment variable[2]. If that env variable is set, then instead of writing out the current date time as a date comment, the `store()` APIs instead will use the value set for this env variable to parse it to a `Date` and write out the string form of such a date. The implementation here uses the `d MMM HH:mm:ss 'GMT'` date format and `Locale.ROOT` to format and write out such a date. This should provide reproducibility whenever the `SOURCE_DATE_EPOCH` is set. Furthermore, intentionally, no changes in the date format of the "current date" have been done. These modified `store()` APIs work in the presence of the `SecurityManager` too. The caller is expected to have a read permission on the `SOURCE_DATE_EPOCH` environment variable. If the caller doesn't have that permission, then the implementation of these `store()` APIs will write out the "current date" and will ignore any value that has been set for the `SOURCE_DATE_EPOCH` env variable. This should allow for backward compatibility of existing applications, where, when they run under a `SecurityManager` and perhaps with an existing restrictive policy file, the presence of `SOURCE_DATE_EPOCH` shouldn't impact their calls to the `store()` APIs. The modified `store()` APIs will also ignore any value for `SOURCE_DATE_EPOCH` that cannot be parsed to an `long` value. In such cases, the `store()` APIs will write out the "current date" and ignore the value set for this environment variable. No exceptions will be thrown for such invalid values. This is an additional backward compatibility precaution to prevent any rogue value for `SOURCE_DATE_EPOCH` from breaking applications. An additional change in the implementation of these `store()` APIs and unrelated to the date comment, is that these APIs will now write out the property keys in a deterministic order. The keys will be written out in the natural ordering as specified by `java.lang.String#compareTo()` API. The combination of the ordering of the property keys when written out and the usage of `SOURCE_DATE_EPOCH` environment value to determine the date comment should together allow for reproducibility of the output generated by these `store()` APIs. New jtreg test classes have been introduced to verify these changes. The primary focus of `PropertiesStoreTest` is the ordering aspects of the property keys that are written out. On the other hand `StoreReproducibilityTest` focuses on the reproducibility of the output generated by these APIs. The `StoreReproducibilityTest` runs these tests both in the presence and absence of `SecurityManager`. Plus, in the presence of SecurityManager, it tests both the scenarios where the caller is granted the requisite permission and in other case not granted that permission. These new tests and existing tests under `test/jdk/java/util/Properties/` pass with these changes. [1] https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-August/080758.html [2] https://reproducible-builds.org/specs/source-date-epoch/ Jaikiran Pai has updated the pull request incrementally with one additional commit since the last revision: Roger's suggestion to reword the implSpec text What would be the next step to move the CSR forward? Should I be changing it's status or do something else? - PR: https://git.openjdk.java.net/jdk/pull/5372
Re: RFR: 8231640: (prop) Canonical property storage [v21]
Hello Jaikiran, The CSR is in Draft state. As discussed in the CSR wiki (https://wiki.openjdk.java.net/display/csr/Main), the request needs to be moved by the assignee to either Finalized or Proposed state to request the CSR review the request. HTH, -Joe On 9/20/2021 8:46 PM, Jaikiran Pai wrote: On Sat, 18 Sep 2021 03:52:17 GMT, Jaikiran Pai wrote: The commit in this PR implements the proposal for enhancement that was discussed in the core-libs-dev mailing list recently[1], for https://bugs.openjdk.java.net/browse/JDK-8231640 At a high level - the `store()` APIs in `Properties` have been modified to now look for the `SOURCE_DATE_EPOCH` environment variable[2]. If that env variable is set, then instead of writing out the current date time as a date comment, the `store()` APIs instead will use the value set for this env variable to parse it to a `Date` and write out the string form of such a date. The implementation here uses the `d MMM HH:mm:ss 'GMT'` date format and `Locale.ROOT` to format and write out such a date. This should provide reproducibility whenever the `SOURCE_DATE_EPOCH` is set. Furthermore, intentionally, no changes in the date format of the "current date" have been done. These modified `store()` APIs work in the presence of the `SecurityManager` too. The caller is expected to have a read permission on the `SOURCE_DATE_EPOCH` environment variable. If the caller doesn't have that permission, then the implementation of these `store()` APIs will write out the "current date" and will ignore any value that has been set for the `SOURCE_DATE_EPOCH` env variable. This should allow for backward compatibility of existing applications, where, when they run under a `SecurityManager` and perhaps with an existing restrictive policy file, the presence of `SOURCE_DATE_EPOCH` shouldn't impact their calls to the `store()` APIs. The modified `store()` APIs will also ignore any value for `SOURCE_DATE_EPOCH` that cannot be parsed to an `long` value. In such cases, the `store()` APIs will write out the "current date" and ignore the value set for this environment variable. No exceptions will be thrown for such invalid values. This is an additional backward compatibility precaution to prevent any rogue value for `SOURCE_DATE_EPOCH` from breaking applications. An additional change in the implementation of these `store()` APIs and unrelated to the date comment, is that these APIs will now write out the property keys in a deterministic order. The keys will be written out in the natural ordering as specified by `java.lang.String#compareTo()` API. The combination of the ordering of the property keys when written out and the usage of `SOURCE_DATE_EPOCH` environment value to determine the date comment should together allow for reproducibility of the output generated by these `store()` APIs. New jtreg test classes have been introduced to verify these changes. The primary focus of `PropertiesStoreTest` is the ordering aspects of the property keys that are written out. On the other hand `StoreReproducibilityTest` focuses on the reproducibility of the output generated by these APIs. The `StoreReproducibilityTest` runs these tests both in the presence and absence of `SecurityManager`. Plus, in the presence of SecurityManager, it tests both the scenarios where the caller is granted the requisite permission and in other case not granted that permission. These new tests and existing tests under `test/jdk/java/util/Properties/` pass with these changes. [1] https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-August/080758.html [2] https://reproducible-builds.org/specs/source-date-epoch/ Jaikiran Pai has updated the pull request incrementally with one additional commit since the last revision: Roger's suggestion to reword the implSpec text What would be the next step to move the CSR forward? Should I be changing it's status or do something else? - PR: https://git.openjdk.java.net/jdk/pull/5372
Re: RFR: 8231640: (prop) Canonical property storage [v21]
On Sat, 18 Sep 2021 03:52:17 GMT, Jaikiran Pai wrote: >> The commit in this PR implements the proposal for enhancement that was >> discussed in the core-libs-dev mailing list recently[1], for >> https://bugs.openjdk.java.net/browse/JDK-8231640 >> >> At a high level - the `store()` APIs in `Properties` have been modified to >> now look for the `SOURCE_DATE_EPOCH` environment variable[2]. If that env >> variable is set, then instead of writing out the current date time as a date >> comment, the `store()` APIs instead will use the value set for this env >> variable to parse it to a `Date` and write out the string form of such a >> date. The implementation here uses the `d MMM HH:mm:ss 'GMT'` date >> format and `Locale.ROOT` to format and write out such a date. This should >> provide reproducibility whenever the `SOURCE_DATE_EPOCH` is set. >> Furthermore, intentionally, no changes in the date format of the "current >> date" have been done. >> >> These modified `store()` APIs work in the presence of the `SecurityManager` >> too. The caller is expected to have a read permission on the >> `SOURCE_DATE_EPOCH` environment variable. If the caller doesn't have that >> permission, then the implementation of these `store()` APIs will write out >> the "current date" and will ignore any value that has been set for the >> `SOURCE_DATE_EPOCH` env variable. This should allow for backward >> compatibility of existing applications, where, when they run under a >> `SecurityManager` and perhaps with an existing restrictive policy file, the >> presence of `SOURCE_DATE_EPOCH` shouldn't impact their calls to the >> `store()` APIs. >> >> The modified `store()` APIs will also ignore any value for >> `SOURCE_DATE_EPOCH` that cannot be parsed to an `long` value. In such >> cases, the `store()` APIs will write out the "current date" and ignore the >> value set for this environment variable. No exceptions will be thrown for >> such invalid values. This is an additional backward compatibility precaution >> to prevent any rogue value for `SOURCE_DATE_EPOCH` from breaking >> applications. >> >> An additional change in the implementation of these `store()` APIs and >> unrelated to the date comment, is that these APIs will now write out the >> property keys in a deterministic order. The keys will be written out in the >> natural ordering as specified by `java.lang.String#compareTo()` API. >> >> The combination of the ordering of the property keys when written out and >> the usage of `SOURCE_DATE_EPOCH` environment value to determine the date >> comment should together allow for reproducibility of the output generated by >> these `store()` APIs. >> >> New jtreg test classes have been introduced to verify these changes. The >> primary focus of `PropertiesStoreTest` is the ordering aspects of the >> property keys that are written out. On the other hand >> `StoreReproducibilityTest` focuses on the reproducibility of the output >> generated by these APIs. The `StoreReproducibilityTest` runs these tests >> both in the presence and absence of `SecurityManager`. Plus, in the presence >> of SecurityManager, it tests both the scenarios where the caller is granted >> the requisite permission and in other case not granted that permission. >> >> These new tests and existing tests under `test/jdk/java/util/Properties/` >> pass with these changes. >> >> [1] >> https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-August/080758.html >> [2] https://reproducible-builds.org/specs/source-date-epoch/ > > Jaikiran Pai has updated the pull request incrementally with one additional > commit since the last revision: > > Roger's suggestion to reword the implSpec text What would be the next step to move the CSR forward? Should I be changing it's status or do something else? - PR: https://git.openjdk.java.net/jdk/pull/5372
Integrated: 8274031: Typo in StringBuilder.readObject
On Tue, 21 Sep 2021 02:06:47 GMT, Joe Darcy wrote: > Fix of a typo in the javadoc of StringBuilder,readObject, as previously > reported to core-libs-dev: > > https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-September/081277.html This pull request has now been integrated. Changeset: 9c91ff57 Author:Joe Darcy URL: https://git.openjdk.java.net/jdk/commit/9c91ff57e8b4b48e997e0424ff93b29e695ec527 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod 8274031: Typo in StringBuilder.readObject Reviewed-by: bpb - PR: https://git.openjdk.java.net/jdk/pull/5592
Re: RFR: 8274031: Typo in StringBuilder.readObject
On Tue, 21 Sep 2021 02:06:47 GMT, Joe Darcy wrote: > Fix of a typo in the javadoc of StringBuilder,readObject, as previously > reported to core-libs-dev: > > https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-September/081277.html Marked as reviewed by bpb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/5592
RFR: 8274031: Typo in StringBuilder.readObject
Fix of a typo in the javadoc of StringBuilder,readObject, as previously reported to core-libs-dev: https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-September/081277.html - Commit messages: - Fix for JDK-8274031. Changes: https://git.openjdk.java.net/jdk/pull/5592/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=5592=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8274031 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/5592.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5592/head:pull/5592 PR: https://git.openjdk.java.net/jdk/pull/5592
Re: Confusing javadoc on a method java.lang.StringBuilder#readObject
On 9/8/2021 1:28 PM, Andrey Turbanov wrote: Hello. I found a confusing javadoc of the method java.lang.StringBuilder#readObject: "readObject is called to restore the state of the StringBuffer from a stream." I believe there should be "StringBuilder" instead of "StringBuffer". Agreed; filed JDK-8274031: Typo in StringBuilder.readObject Thanks, -Joe
Re: What causes java.lang.InternalError: platform encoding not initialized ?
I've made an application image with jpackage. I've moved the default location of the runtime in the application image ... If I keep the runtime in $APPDIR\runtime, things don't fail in this way. If it works when using the default location of the Java runtime and no other changes, I'd guess that this is a bug in the jpackage app launcher, or maybe in the code that creates the runtime (e.g., if something isn't copied to the right place when not in the default location). Hopefully Andy or Alexey can comment. -- Kevin On 9/20/2021 2:37 PM, Scott Palmer wrote: This is likely not the right place to ask this (sorry).. but I'm trying to get info to write a bug report and want to make sure I'm not doing something stupid first. I've created a JRE image from JDK 17 with jlink. I've made an application image with jpackage. I've moved the default location of the runtime in the application image and modified the launcher .cfg file accordingly by adding a line to the [Application] section app.runtime=$APPDIR\..\my_own_folder\where_the_jre_is_deeper\jre My application launches. It shows a JavaFX GUI. It does a bunch of stuff that "works", but then one thread fails with this exception: Exception in thread "Reader-BG-1" java.lang.InternalError: platform encoding not initialized at java.base/java.net.Inet6AddressImpl.lookupAllHostAddr(Native Method) at java.base/java.net.InetAddress$PlatformNameService.lookupAllHostAddr(InetAddress.java:933) at java.base/java.net.InetAddress.getAddressesFromNameService(InetAddress.java:1519) at java.base/java.net.InetAddress$NameServiceAddresses.get(InetAddress.java:852) at java.base/java.net.InetAddress.getAllByName0(InetAddress.java:1509) at java.base/java.net.InetAddress.getAllByName(InetAddress.java:1367) at java.base/java.net.InetAddress.getAllByName(InetAddress.java:1301) at java.base/java.net.InetAddress.getByName(InetAddress.java:1251) at java.base/java.net.InetSocketAddress.(InetSocketAddress.java:229) at java.base/sun.net.NetworkClient.doConnect(NetworkClient.java:178) at java.base/sun.net.www.http.HttpClient.openServer(HttpClient.java:498) at java.base/sun.net.www.http.HttpClient.openServer(HttpClient.java:603) at java.base/sun.net.www.protocol.https.HttpsClient.(HttpsClient.java:266) at java.base/sun.net.www.protocol.https.HttpsClient.New(HttpsClient.java:380) at java.base/sun.net.www.protocol.https.AbstractDelegateHttpsURLConnection.getNewHttpClient(AbstractDelegateHttpsURLConnection.java:189) at java.base/sun.net.www.protocol.http.HttpURLConnection.plainConnect0(HttpURLConnection.java:1242) at java.base/sun.net.www.protocol.http.HttpURLConnection.plainConnect(HttpURLConnection.java:1128) at java.base/sun.net.www.protocol.https.AbstractDelegateHttpsURLConnection.connect(AbstractDelegateHttpsURLConnection.java:175) at java.base/sun.net.www.protocol.http.HttpURLConnection.getInputStream0(HttpURLConnection.java:1665) at java.base/sun.net.www.protocol.http.HttpURLConnection.getInputStream(HttpURLConnection.java:1589) at java.base/java.net.HttpURLConnection.getResponseCode(HttpURLConnection.java:529) at java.base/sun.net.www.protocol.https.HttpsURLConnectionImpl.getResponseCode(HttpsURLConnectionImpl.java:308) If I keep the runtime in $APPDIR\runtime, things don't fail in this way. Smells like a bug in the app launcher to me.. but maybe it's in the runtime? Any assistance would be appreciated (including telling me where I should go to ask this, if this list isn't appropriate). Thanks, Scott
What causes java.lang.InternalError: platform encoding not initialized ?
This is likely not the right place to ask this (sorry).. but I'm trying to get info to write a bug report and want to make sure I'm not doing something stupid first. I've created a JRE image from JDK 17 with jlink. I've made an application image with jpackage. I've moved the default location of the runtime in the application image and modified the launcher .cfg file accordingly by adding a line to the [Application] section app.runtime=$APPDIR\..\my_own_folder\where_the_jre_is_deeper\jre My application launches. It shows a JavaFX GUI. It does a bunch of stuff that "works", but then one thread fails with this exception: Exception in thread "Reader-BG-1" java.lang.InternalError: platform encoding not initialized at java.base/java.net.Inet6AddressImpl.lookupAllHostAddr(Native Method) at java.base/java.net.InetAddress$PlatformNameService.lookupAllHostAddr(InetAddress.java:933) at java.base/java.net.InetAddress.getAddressesFromNameService(InetAddress.java:1519) at java.base/java.net.InetAddress$NameServiceAddresses.get(InetAddress.java:852) at java.base/java.net.InetAddress.getAllByName0(InetAddress.java:1509) at java.base/java.net.InetAddress.getAllByName(InetAddress.java:1367) at java.base/java.net.InetAddress.getAllByName(InetAddress.java:1301) at java.base/java.net.InetAddress.getByName(InetAddress.java:1251) at java.base/java.net.InetSocketAddress.(InetSocketAddress.java:229) at java.base/sun.net.NetworkClient.doConnect(NetworkClient.java:178) at java.base/sun.net.www.http.HttpClient.openServer(HttpClient.java:498) at java.base/sun.net.www.http.HttpClient.openServer(HttpClient.java:603) at java.base/sun.net.www.protocol.https.HttpsClient.(HttpsClient.java:266) at java.base/sun.net.www.protocol.https.HttpsClient.New(HttpsClient.java:380) at java.base/sun.net.www.protocol.https.AbstractDelegateHttpsURLConnection.getNewHttpClient(AbstractDelegateHttpsURLConnection.java:189) at java.base/sun.net.www.protocol.http.HttpURLConnection.plainConnect0(HttpURLConnection.java:1242) at java.base/sun.net.www.protocol.http.HttpURLConnection.plainConnect(HttpURLConnection.java:1128) at java.base/sun.net.www.protocol.https.AbstractDelegateHttpsURLConnection.connect(AbstractDelegateHttpsURLConnection.java:175) at java.base/sun.net.www.protocol.http.HttpURLConnection.getInputStream0(HttpURLConnection.java:1665) at java.base/sun.net.www.protocol.http.HttpURLConnection.getInputStream(HttpURLConnection.java:1589) at java.base/java.net.HttpURLConnection.getResponseCode(HttpURLConnection.java:529) at java.base/sun.net.www.protocol.https.HttpsURLConnectionImpl.getResponseCode(HttpsURLConnectionImpl.java:308) If I keep the runtime in $APPDIR\runtime, things don't fail in this way. Smells like a bug in the app launcher to me.. but maybe it's in the runtime? Any assistance would be appreciated (including telling me where I should go to ask this, if this list isn't appropriate). Thanks, Scott
Re: RFR: 8273935: (zipfs) Files.getFileAttributeView() throws UOE instead of returning null when view not supported [v3]
On Mon, 20 Sep 2021 18:17:29 GMT, Lance Andersen wrote: >> Hi all, >> >> Please review this patch which addresses the issue where Zip FS will throw a >> UOE instead of returning null when Files.getFileAttributeView() is invoked >> and the view not supported. >> >> Mach5 tiers1 - tier3 are clean. >> >> Best >> Lance > > Lance Andersen has updated the pull request incrementally with one additional > commit since the last revision: > > fix minor typo Marked as reviewed by bpb (Reviewer). Marked as reviewed by bpb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/5579
Re: RFR: 8273935: (zipfs) Files.getFileAttributeView() throws UOE instead of returning null when view not supported [v3]
On Mon, 20 Sep 2021 18:17:29 GMT, Lance Andersen wrote: >> Hi all, >> >> Please review this patch which addresses the issue where Zip FS will throw a >> UOE instead of returning null when Files.getFileAttributeView() is invoked >> and the view not supported. >> >> Mach5 tiers1 - tier3 are clean. >> >> Best >> Lance > > Lance Andersen has updated the pull request incrementally with one additional > commit since the last revision: > > fix minor typo Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/5579
Re: RFR: 8271820: Implementation of JEP 416: Reimplement Core Reflection with Method Handle [v8]
On Wed, 1 Sep 2021 01:05:32 GMT, Mandy Chung wrote: >> This reimplements core reflection with method handles. >> >> For `Constructor::newInstance` and `Method::invoke`, the new implementation >> uses `MethodHandle`. For `Field` accessor, the new implementation uses >> `VarHandle`.For the first few invocations of one of these reflective >> methods on a specific reflective object we invoke the corresponding method >> handle directly. After that we spin a dynamic bytecode stub defined in a >> hidden class which loads the target `MethodHandle` or `VarHandle` from its >> class data as a dynamically computed constant. Loading the method handle >> from a constant allows JIT to inline the method-handle invocation in order >> to achieve good performance. >> >> The VM's native reflection methods are needed during early startup, before >> the method-handle mechanism is initialized. That happens soon after >> System::initPhase1 and before System::initPhase2, after which we switch to >> using method handles exclusively. >> >> The core reflection and method handle implementation are updated to handle >> chained caller-sensitive method calls [1] properly. A caller-sensitive >> method can define with a caller-sensitive adapter method that will take an >> additional caller class parameter and the adapter method will be annotated >> with `@CallerSensitiveAdapter` for better auditing. See the detailed >> description from [2]. >> >> Ran tier1-tier8 tests. >> >> [1] https://bugs.openjdk.java.net/browse/JDK-8013527 >> [2] >> https://bugs.openjdk.java.net/browse/JDK-8271820?focusedCommentId=14439430=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14439430 > > Mandy Chung has updated the pull request incrementally with one additional > commit since the last revision: > > minor cleanup and more test case. Thanks for doing the Jackson (de)serialization benchmark. I'll follow up with Claes when he returns from vacation end of this week and see if we can agree that performance improvements can be explored as follows-up works. - PR: https://git.openjdk.java.net/jdk/pull/5027
Re: RFR: 8273935: (zipfs) Files.getFileAttributeView() throws UOE instead of returning null when view not supported [v3]
> Hi all, > > Please review this patch which addresses the issue where Zip FS will throw a > UOE instead of returning null when Files.getFileAttributeView() is invoked > and the view not supported. > > Mach5 tiers1 - tier3 are clean. > > Best > Lance Lance Andersen has updated the pull request incrementally with one additional commit since the last revision: fix minor typo - Changes: - all: https://git.openjdk.java.net/jdk/pull/5579/files - new: https://git.openjdk.java.net/jdk/pull/5579/files/6ff49197..65184a5a Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=5579=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5579=01-02 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/5579.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5579/head:pull/5579 PR: https://git.openjdk.java.net/jdk/pull/5579
Re: RFR: 8273935: (zipfs) Files.getFileAttributeView() throws UOE instead of returning null when view not supported [v2]
> Hi all, > > Please review this patch which addresses the issue where Zip FS will throw a > UOE instead of returning null when Files.getFileAttributeView() is invoked > and the view not supported. > > Mach5 tiers1 - tier3 are clean. > > Best > Lance Lance Andersen has updated the pull request incrementally with one additional commit since the last revision: Updates based on feedback - Changes: - all: https://git.openjdk.java.net/jdk/pull/5579/files - new: https://git.openjdk.java.net/jdk/pull/5579/files/e814a9f1..6ff49197 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=5579=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5579=00-01 Stats: 12 lines in 3 files changed: 1 ins; 2 del; 9 mod Patch: https://git.openjdk.java.net/jdk/pull/5579.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5579/head:pull/5579 PR: https://git.openjdk.java.net/jdk/pull/5579
Re: RFR: 8273935: (zipfs) Files.getFileAttributeView() throws UOE instead of returning null when view not supported
On Mon, 20 Sep 2021 17:05:33 GMT, Severin Gehwolf wrote:
>> Hi all,
>>
>> Please review this patch which addresses the issue where Zip FS will throw a
>> UOE instead of returning null when Files.getFileAttributeView() is invoked
>> and the view not supported.
>>
>> Mach5 tiers1 - tier3 are clean.
>>
>> Best
>> Lance
>
> test/jdk/jdk/nio/zipfs/testng/test/PosixAttributeViewTest.java line 100:
>
>> 98: PosixFileAttributeView view =
>> Files.getFileAttributeView(entry,
>> 99: PosixFileAttributeView.class);
>> 100: System.out.printf("View returned: %s%n", view);
>
> It might be useful to also print whether the environment is empty:
> Suggestion: `System.out.printf("View returned: %s, (empty_env=%s)%n", view,
> env.isEmpty());`
Well, you can see that whether the Map is/is not empty in the jtr file for the
test run (based on the DataProvider Properties for a given run).
However, per your suggestion, I tweaked the test to dump the Map values.
-
PR: https://git.openjdk.java.net/jdk/pull/5579
Re: RFR: 8273935: (zipfs) Files.getFileAttributeView() throws UOE instead of returning null when view not supported
On Mon, 20 Sep 2021 11:28:10 GMT, Alan Bateman wrote:
>> Hi all,
>>
>> Please review this patch which addresses the issue where Zip FS will throw a
>> UOE instead of returning null when Files.getFileAttributeView() is invoked
>> and the view not supported.
>>
>> Mach5 tiers1 - tier3 are clean.
>>
>> Best
>> Lance
>
> src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipPath.java line 716:
>
>> 714: return (V)new ZipPosixFileAttributeView(this,true);
>> 715: }
>> 716: return (V) null;
>
> I assume (V) isn't needed here.
No, it is not and can also probably be removed from
open/src/java.base/unix/classes/sun/nio/fs/UnixFileSystemProvider.java at some
point
> test/jdk/jdk/nio/zipfs/testng/test/PosixAttributeViewTest.java line 59:
>
>> 57: public void setup() throws IOException {
>> 58: Files.deleteIfExists(ZIP_FILE);
>> 59: Entry e0 = Entry.of(ZIP_ENTRY, ZipEntry.DEFLATED,
>
> Is there a reason why this is named "e0"?
Not really. Historically that was the variable name format used in some of the
Zip FS tests. I changed the name to `entry`
> test/jdk/jdk/nio/zipfs/testng/test/PosixAttributeViewTest.java line 82:
>
>> 80: return new Object[][]{
>> 81: {Map.of()},
>> 82: { Map.of("enablePosixFileAttributes", "true")}
>
> Minor nit, inconsistent formatting with L81 vs. L82.
Fixed, thanks for catching that.
> test/jdk/jdk/nio/zipfs/testng/test/PosixAttributeViewTest.java line 89:
>
>> 87: * Verify that Files.getFileAttributeView will not throw
>> 88: * UnsupportedOperationException when the attribute view
>> 89: * PosixFileAttributeView is not available
>
> Might be clearer to say that it checks that Files.getFileAttributeView does
> not throw an exception (no need to mention UOE) when PosixFileAttributeView
> is not supported.
Updated per your suggestion!
> test/jdk/jdk/nio/zipfs/testng/test/PosixAttributeViewTest.java line 95:
>
>> 93: @Test(dataProvider = "zipfsMap")
>> 94: public void testPosixAttributeView(Map env) throws
>> Exception {
>> 95:
>
> Spurious blank line.
Removed.
-
PR: https://git.openjdk.java.net/jdk/pull/5579
Re: RFR: 8273935: (zipfs) Files.getFileAttributeView() throws UOE instead of returning null when view not supported
On Sun, 19 Sep 2021 15:34:44 GMT, Lance Andersen wrote: > Hi all, > > Please review this patch which addresses the issue where Zip FS will throw a > UOE instead of returning null when Files.getFileAttributeView() is invoked > and the view not supported. > > Mach5 tiers1 - tier3 are clean. > > Best > Lance Marked as reviewed by bpb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/5579
Re: RFR: 8273935: (zipfs) Files.getFileAttributeView() throws UOE instead of returning null when view not supported
On Sun, 19 Sep 2021 15:34:44 GMT, Lance Andersen wrote:
> Hi all,
>
> Please review this patch which addresses the issue where Zip FS will throw a
> UOE instead of returning null when Files.getFileAttributeView() is invoked
> and the view not supported.
>
> Mach5 tiers1 - tier3 are clean.
>
> Best
> Lance
test/jdk/jdk/nio/zipfs/testng/test/PosixAttributeViewTest.java line 100:
> 98: PosixFileAttributeView view =
> Files.getFileAttributeView(entry,
> 99: PosixFileAttributeView.class);
> 100: System.out.printf("View returned: %s%n", view);
It might be useful to also print whether the environment is empty:
Suggestion: `System.out.printf("View returned: %s, (empty_env=%s)%n", view,
env.isEmpty());`
-
PR: https://git.openjdk.java.net/jdk/pull/5579
Re: RFR: 8245095: Implementation of JEP 408: Simple Web Server [v4]
On Mon, 20 Sep 2021 16:09:14 GMT, Daniel Fuchs wrote: >> Julia Boes has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains 12 commits: >> >> - Merge branch 'master' into simpleserver >> - check isHidden, isSymlink, isReadable for all path segments >> - add checks for all path segments >> - Merge branch 'master' into componentcheck >> - Merge branch 'master' into simpleserver >> - improve output on startup >> - correct path handling >> - small spec rewording >> - add module main class to symbolgenerator >> - remove UnmodifiableHeaders constant >> - ... and 2 more: >> https://git.openjdk.java.net/jdk/compare/4d95a5d6...10523290 > > src/jdk.httpserver/share/classes/sun/net/httpserver/simpleserver/FileServerHandler.java > line 340: > >> 338: } >> 339: } >> 340: return false; > > This will start checking from the root of the file system. I believe we want > to start checking from the root of the FileServerHandler, root excluded. Maybe these checks should be made in `mapToPath` instead since you already walk the path there - and IIRC returning null from `mapToPath` will cause HTTP 404. - PR: https://git.openjdk.java.net/jdk/pull/5505
Re: RFR: 8245095: Implementation of JEP 408: Simple Web Server [v4]
On Mon, 20 Sep 2021 15:28:05 GMT, Julia Boes wrote: >> This change implements a simple web server that can be run on the >> command-line with `java -m jdk.httpserver`. >> >> This is facilitated by adding an entry point for the `jdk.httpserver` >> module, an implementation class whose main method is run when the above >> command is executed. This is the first such module entry point in the JDK. >> >> The server is a minimal HTTP server that serves the static files of a given >> directory, similar to existing alternatives on other platforms and >> convenient for testing, development, and debugging. >> >> Additionally, a small API is introduced for programmatic creation and >> customization. >> >> Testing: tier1-3. > > Julia Boes has updated the pull request with a new target base due to a merge > or a rebase. The pull request now contains 12 commits: > > - Merge branch 'master' into simpleserver > - check isHidden, isSymlink, isReadable for all path segments > - add checks for all path segments > - Merge branch 'master' into componentcheck > - Merge branch 'master' into simpleserver > - improve output on startup > - correct path handling > - small spec rewording > - add module main class to symbolgenerator > - remove UnmodifiableHeaders constant > - ... and 2 more: > https://git.openjdk.java.net/jdk/compare/4d95a5d6...10523290 src/jdk.httpserver/share/classes/sun/net/httpserver/simpleserver/FileServerHandler.java line 340: > 338: } > 339: } > 340: return false; This will start checking from the root of the file system. I believe we want to start checking from the root of the FileServerHandler, root excluded. - PR: https://git.openjdk.java.net/jdk/pull/5505
Integrated: 8272515: JFR: Names should only be valid Java identifiers
On Wed, 8 Sep 2021 13:00:17 GMT, Erik Gahlin wrote: > Hi, > > Could I have a review of change that prevents invalid Java identifiers or > type names in JFR events. For rationale and compatibility issues see the CSR > request. The only change to java.base is making > jdk.modules.internal.Checks::isJavaIdentifier(String) public, so it can be > reused by the jdk.jfr module. > > Testing: /jdk/jdk/jfr + tier1 + tier2 > > Thanks > Erik This pull request has now been integrated. Changeset: 48aff231 Author:Erik Gahlin URL: https://git.openjdk.java.net/jdk/commit/48aff23165db668eb9c06477d16a8e72b6dc6b56 Stats: 308 lines in 11 files changed: 259 ins; 23 del; 26 mod 8272515: JFR: Names should only be valid Java identifiers Reviewed-by: mgronlun - PR: https://git.openjdk.java.net/jdk/pull/5415
Re: Condition is always false in 'ProcessHandleImpl.Info#toString'
Thanks for reporting.
Filed: https://bugs.openjdk.java.net/browse/JDK-8274003
-Sundar
From: core-libs-dev on behalf of Andrey
Turbanov
Sent: 20 September 2021 20:49
To: core-libs-dev
Subject: Condition is always false in 'ProcessHandleImpl.Info#toString'
Hello.
I found suspicious code in the method java.lang.ProcessHandleImpl.Info#toString
https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/lang/ProcessHandleImpl.java#L645
```
StringBuilder sb = new StringBuilder(60);
sb.append('[');
if (user != null) {
sb.append("user: ");
sb.append(user());
}
if (command != null) {
if (sb.length() != 0) sb.append(", ");
sb.append("cmd: ");
sb.append(command);
}
```
Opening bracket '[' is added unconditionally to the StringBuilder.
But then the code checks "if (sb.length() != 0)"
This condition will always be *false*.
Looks like comparison with 1 should be used instead.
Andrey Turbanov
Re: RFR: 8245095: Implementation of JEP 408: Simple Web Server [v4]
> This change implements a simple web server that can be run on the > command-line with `java -m jdk.httpserver`. > > This is facilitated by adding an entry point for the `jdk.httpserver` module, > an implementation class whose main method is run when the above command is > executed. This is the first such module entry point in the JDK. > > The server is a minimal HTTP server that serves the static files of a given > directory, similar to existing alternatives on other platforms and convenient > for testing, development, and debugging. > > Additionally, a small API is introduced for programmatic creation and > customization. > > Testing: tier1-3. Julia Boes has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 12 commits: - Merge branch 'master' into simpleserver - check isHidden, isSymlink, isReadable for all path segments - add checks for all path segments - Merge branch 'master' into componentcheck - Merge branch 'master' into simpleserver - improve output on startup - correct path handling - small spec rewording - add module main class to symbolgenerator - remove UnmodifiableHeaders constant - ... and 2 more: https://git.openjdk.java.net/jdk/compare/4d95a5d6...10523290 - Changes: https://git.openjdk.java.net/jdk/pull/5505/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=5505=03 Stats: 7034 lines in 43 files changed: 6998 ins; 15 del; 21 mod Patch: https://git.openjdk.java.net/jdk/pull/5505.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5505/head:pull/5505 PR: https://git.openjdk.java.net/jdk/pull/5505
Condition is always false in 'ProcessHandleImpl.Info#toString'
Hello.
I found suspicious code in the method java.lang.ProcessHandleImpl.Info#toString
https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/lang/ProcessHandleImpl.java#L645
```
StringBuilder sb = new StringBuilder(60);
sb.append('[');
if (user != null) {
sb.append("user: ");
sb.append(user());
}
if (command != null) {
if (sb.length() != 0) sb.append(", ");
sb.append("cmd: ");
sb.append(command);
}
```
Opening bracket '[' is added unconditionally to the StringBuilder.
But then the code checks "if (sb.length() != 0)"
This condition will always be *false*.
Looks like comparison with 1 should be used instead.
Andrey Turbanov
Integrated: 8247980: Exclusive execution of java/util/stream tests slows down tier1
On Thu, 19 Aug 2021 15:15:31 GMT, Aleksey Shipilev wrote: > See the bug report for more details. I would appreciate if people with their > corporate testing systems would run this through their systems to avoid > surprises. (ping @RealCLanger, @iignatev). This pull request has now been integrated. Changeset: 544193a3 Author:Aleksey Shipilev URL: https://git.openjdk.java.net/jdk/commit/544193a3bb6431ee4bb0bd43cb29cc60c7709b25 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod 8247980: Exclusive execution of java/util/stream tests slows down tier1 Reviewed-by: iignatyev - PR: https://git.openjdk.java.net/jdk/pull/5189
Integrated: 8273314: Add tier4 test groups
On Fri, 3 Sep 2021 09:10:20 GMT, Aleksey Shipilev wrote:
> During the review of JDK-8272914 that added hotspot:tier{2,3} groups,
> @iignatev suggested to create tier4 groups that capture all tests not in
> tiers{1,2,3}.
>
> Caveats:
> - I excluded `applications` from `hotspot:tier4`, because they require test
> dependencies (e.g. jcstress).
> - `jdk:tier4` only runs well with `JTREG_KEYWORDS=!headful` or reduced
> concurrency with `TEST_JOBS=1`, because headful tests cannot run in parallel
>
> Sample run with `JTREG_KEYWORDS=!headful`:
>
>
> ==
> Test summary
> ==
>TEST TOTAL PASS FAIL ERROR
>
>>> jtreg:test/hotspot/jtreg:tier4 3585 3584 0 1 <<
>>> jtreg:test/jdk:tier4 2893 2887 5 1 <<
>jtreg:test/langtools:tier40 0 0 0
>
>jtreg:test/jaxp:tier4 0 0 0 0
>
> ==
>
> real 699m39.462s
> user 6626m8.448s
> sys 1110m43.704s
>
>
> There are interesting test failures on my machine, which I would address
> separately.
This pull request has now been integrated.
Changeset: 1f8af524
Author:Aleksey Shipilev
URL:
https://git.openjdk.java.net/jdk/commit/1f8af524ffe2d2d1469d8f07887b1f61c6e4d7b8
Stats: 20 lines in 4 files changed: 20 ins; 0 del; 0 mod
8273314: Add tier4 test groups
Reviewed-by: serb, iignatyev
-
PR: https://git.openjdk.java.net/jdk/pull/5357
Integrated: 8273187: jtools tests fail with missing markerName check
On Thu, 16 Sep 2021 01:08:45 GMT, Naoto Sato wrote: > Fixing failing regression tests caused by the JEP 400: UTF-8 by Default. > > `JcmdOutputEncodingTest` test case uses `file.encoding=UTF-8` in `C` locale. > The output from the agent library is in `UTF-8` so it succeeded before the > JEP has been implemented, as System.out used `UTF-8` converter. After the > JEP, it started failing because System.out is using `US-ASCII` which > generates series of '?', ending up with assertion failure in the test. > > The proposed fix is to pass `sun.stdout.encoding=UTF-8` as well as > `file.encoding` so that tool process' System.out is in `UTF-8` as well. This pull request has now been integrated. Changeset: f71df142 Author:Naoto Sato URL: https://git.openjdk.java.net/jdk/commit/f71df142a97f522b40e90c3105e0e5bd8d5af9a2 Stats: 10 lines in 3 files changed: 3 ins; 4 del; 3 mod 8273187: jtools tests fail with missing markerName check Reviewed-by: iris, sspitsyn, joehw - PR: https://git.openjdk.java.net/jdk/pull/5539
Re: RFR: 8272515: JFR: Names should only be valid Java identifiers
On Wed, 8 Sep 2021 13:00:17 GMT, Erik Gahlin wrote: > Hi, > > Could I have a review of change that prevents invalid Java identifiers or > type names in JFR events. For rationale and compatibility issues see the CSR > request. The only change to java.base is making > jdk.modules.internal.Checks::isJavaIdentifier(String) public, so it can be > reused by the jdk.jfr module. > > Testing: /jdk/jdk/jfr + tier1 + tier2 > > Thanks > Erik Looks good. - Marked as reviewed by mgronlun (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/5415
Re: RFR: 8231640: (prop) Canonical property storage [v21]
On Sat, 18 Sep 2021 03:52:17 GMT, Jaikiran Pai wrote: >> The commit in this PR implements the proposal for enhancement that was >> discussed in the core-libs-dev mailing list recently[1], for >> https://bugs.openjdk.java.net/browse/JDK-8231640 >> >> At a high level - the `store()` APIs in `Properties` have been modified to >> now look for the `SOURCE_DATE_EPOCH` environment variable[2]. If that env >> variable is set, then instead of writing out the current date time as a date >> comment, the `store()` APIs instead will use the value set for this env >> variable to parse it to a `Date` and write out the string form of such a >> date. The implementation here uses the `d MMM HH:mm:ss 'GMT'` date >> format and `Locale.ROOT` to format and write out such a date. This should >> provide reproducibility whenever the `SOURCE_DATE_EPOCH` is set. >> Furthermore, intentionally, no changes in the date format of the "current >> date" have been done. >> >> These modified `store()` APIs work in the presence of the `SecurityManager` >> too. The caller is expected to have a read permission on the >> `SOURCE_DATE_EPOCH` environment variable. If the caller doesn't have that >> permission, then the implementation of these `store()` APIs will write out >> the "current date" and will ignore any value that has been set for the >> `SOURCE_DATE_EPOCH` env variable. This should allow for backward >> compatibility of existing applications, where, when they run under a >> `SecurityManager` and perhaps with an existing restrictive policy file, the >> presence of `SOURCE_DATE_EPOCH` shouldn't impact their calls to the >> `store()` APIs. >> >> The modified `store()` APIs will also ignore any value for >> `SOURCE_DATE_EPOCH` that cannot be parsed to an `long` value. In such >> cases, the `store()` APIs will write out the "current date" and ignore the >> value set for this environment variable. No exceptions will be thrown for >> such invalid values. This is an additional backward compatibility precaution >> to prevent any rogue value for `SOURCE_DATE_EPOCH` from breaking >> applications. >> >> An additional change in the implementation of these `store()` APIs and >> unrelated to the date comment, is that these APIs will now write out the >> property keys in a deterministic order. The keys will be written out in the >> natural ordering as specified by `java.lang.String#compareTo()` API. >> >> The combination of the ordering of the property keys when written out and >> the usage of `SOURCE_DATE_EPOCH` environment value to determine the date >> comment should together allow for reproducibility of the output generated by >> these `store()` APIs. >> >> New jtreg test classes have been introduced to verify these changes. The >> primary focus of `PropertiesStoreTest` is the ordering aspects of the >> property keys that are written out. On the other hand >> `StoreReproducibilityTest` focuses on the reproducibility of the output >> generated by these APIs. The `StoreReproducibilityTest` runs these tests >> both in the presence and absence of `SecurityManager`. Plus, in the presence >> of SecurityManager, it tests both the scenarios where the caller is granted >> the requisite permission and in other case not granted that permission. >> >> These new tests and existing tests under `test/jdk/java/util/Properties/` >> pass with these changes. >> >> [1] >> https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-August/080758.html >> [2] https://reproducible-builds.org/specs/source-date-epoch/ > > Jaikiran Pai has updated the pull request incrementally with one additional > commit since the last revision: > > Roger's suggestion to reword the implSpec text Marked as reviewed by dfuchs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/5372
Re: RFR: 8272600: (test) Use native "sleep" in Basic.java [v6]
> The intermittent test in java/lang/ProcessBuilder/Basic.java has identified > unexpected messages from a child Java VM > as the cause of the test failure. Attempts to control the output of the > child VM have failed, the VM is unrepentant . > > There is no functionality in the child except to wait long enough for the > test to finish and the child is destroyed. > The fix is to switch from using a Java child to using a native child; a new > executable `sleepmillis`. Roger Riggs has updated the pull request incrementally with one additional commit since the last revision: The switch from a Java child to /bin/sleep caused another test to fail on Linux. The cleanup for a test used /usr/bin/pkill "sleep 60". A race between that cleanup and subsequent tests that used sleep 60 or 600 could kill the sleep before the test of waitFor completed. Changing the test using pkill to use 59 seconds makes the test cleanup selective to the sleep spawned for that test. The test that was failing (lines 2626-2624) passes consistently. - Changes: - all: https://git.openjdk.java.net/jdk/pull/5239/files - new: https://git.openjdk.java.net/jdk/pull/5239/files/43a54802..afa932d6 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=5239=05 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5239=04-05 Stats: 25 lines in 1 file changed: 7 ins; 10 del; 8 mod Patch: https://git.openjdk.java.net/jdk/pull/5239.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5239/head:pull/5239 PR: https://git.openjdk.java.net/jdk/pull/5239
Re: RFR: 8273797: Stop impersonating "server" VM in all VM variants
On Wed, 15 Sep 2021 10:02:19 GMT, Aleksey Shipilev wrote: > As the follow-up for Zero-specific JDK-8273494, we might want to clean up > build system logic for all VM variants: stop impersonating "server" VMs for > all of them. This basically leaves "core" and "custom" variants to be handled > with this patch. > > Additional testing: > - [x] Linux x86_64 `core` build passes, `libjvm.so` now in `core`, `jvm.cfg` > mentions `-core KNOWN` > - [x] Linux x86_64 `custom` (+serialgc,+compiler2) build passes, `libjvm.so` > now in `custom`, `jvm.cfg` mentions `-custom KNOWN` As for eliminating multi-JVM builds: yes, that is on my agenda, but as long as I'm still just working part-time it's hard to make any headway in that direction. The next step is to create a `--with-import-jvms=,[,...]`, so that it is possible to simulate a multi-JVM build by e.g. bash configure --with-jvm-variant=zero --with-conf-name=zero-hotspot make hotspot bash configure --with-jvm-variant=server --with-conf-name=combined --with-import-jvms=zero:./build/$PLATFORM/jdk/lib/zero or something like that. When that exists and is working, and all downstream users has had time to adapt, then we can remove multi-JVM build support (and boy am I longing for that day). If you want to help by implementing the `--with-import-jvms` options, you are more than welcome! - PR: https://git.openjdk.java.net/jdk/pull/5526
Re: RFR: 8273935: (zipfs) Files.getFileAttributeView() throws UOE instead of returning null when view not supported
On Sun, 19 Sep 2021 15:34:44 GMT, Lance Andersen wrote:
> Hi all,
>
> Please review this patch which addresses the issue where Zip FS will throw a
> UOE instead of returning null when Files.getFileAttributeView() is invoked
> and the view not supported.
>
> Mach5 tiers1 - tier3 are clean.
>
> Best
> Lance
Marked as reviewed by alanb (Reviewer).
src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipPath.java line 716:
> 714: return (V)new ZipPosixFileAttributeView(this,true);
> 715: }
> 716: return (V) null;
I assume (V) isn't needed here.
test/jdk/jdk/nio/zipfs/testng/test/PosixAttributeViewTest.java line 59:
> 57: public void setup() throws IOException {
> 58: Files.deleteIfExists(ZIP_FILE);
> 59: Entry e0 = Entry.of(ZIP_ENTRY, ZipEntry.DEFLATED,
Is there a reason why this is named "e0"?
test/jdk/jdk/nio/zipfs/testng/test/PosixAttributeViewTest.java line 82:
> 80: return new Object[][]{
> 81: {Map.of()},
> 82: { Map.of("enablePosixFileAttributes", "true")}
Minor nit, inconsistent formatting with L81 vs. L82.
test/jdk/jdk/nio/zipfs/testng/test/PosixAttributeViewTest.java line 89:
> 87: * Verify that Files.getFileAttributeView will not throw
> 88: * UnsupportedOperationException when the attribute view
> 89: * PosixFileAttributeView is not available
Might be clearer to say that it checks that Files.getFileAttributeView does not
throw an exception (no need to mention UOE) when PosixFileAttributeView is not
supported.
test/jdk/jdk/nio/zipfs/testng/test/PosixAttributeViewTest.java line 95:
> 93: @Test(dataProvider = "zipfsMap")
> 94: public void testPosixAttributeView(Map env) throws
> Exception {
> 95:
Spurious blank line.
-
PR: https://git.openjdk.java.net/jdk/pull/5579
Re: RFR: 8273797: Stop impersonating "server" VM in all VM variants
On Wed, 15 Sep 2021 10:02:19 GMT, Aleksey Shipilev wrote: > As the follow-up for Zero-specific JDK-8273494, we might want to clean up > build system logic for all VM variants: stop impersonating "server" VMs for > all of them. This basically leaves "core" and "custom" variants to be handled > with this patch. > > Additional testing: > - [x] Linux x86_64 `core` build passes, `libjvm.so` now in `core`, `jvm.cfg` > mentions `-core KNOWN` > - [x] Linux x86_64 `custom` (+serialgc,+compiler2) build passes, `libjvm.so` > now in `custom`, `jvm.cfg` mentions `-custom KNOWN` Marked as reviewed by ihse (Reviewer). Looks good. Thanks for waiting for my input; sorry for the delay. - PR: https://git.openjdk.java.net/jdk/pull/5526
RFR: 8273935: (zipfs) Files.getFileAttributeView() throws UOE instead of returning null when view not supported
Hi all, Please review this patch which addresses the issue where Zip FS will throw a UOE instead of returning null when Files.getFileAttributeView() is invoked and the view not supported. Mach5 tiers1 - tier3 are clean. Best Lance - Commit messages: - Add Cleanup Method - adjust copyright - Files.getFileAttributeView() throws UOE instead of returning null when view not supported Changes: https://git.openjdk.java.net/jdk/pull/5579/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=5579=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8273935 Stats: 107 lines in 3 files changed: 103 ins; 0 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/5579.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5579/head:pull/5579 PR: https://git.openjdk.java.net/jdk/pull/5579
Re: RFR: 8268231: Aarch64: Use ldp in intrinsics for String.compareTo [v6]
On Mon, 20 Sep 2021 09:52:45 GMT, Andrew Dinn wrote: >> Andrew, can you help us to approve this? > > I agree with Andrew Haley that this patch is not going to make an improvement > for anything but a very small number of applications. Processing of strings > over a few 10s of bytes is rare. On the other hand the doesn't seem to cause > any performance drop for the much more common case of processing short > strings. so it does no harm. Also, the new and old code are much the same in > terms of complexity so that is no reason to prefer one over the other. The > only real concern I have is that any change involves the risk of error and > the ratio of cases that might benefit to cases that might suffer from an > error is very low. I don't think that's a reason to avoid pushing this patch > upstream but it does suggest that we should not backport it. OK, thanks. That seems like a sensible compromise. - PR: https://git.openjdk.java.net/jdk/pull/4722
Re: RFR: 8268231: Aarch64: Use ldp in intrinsics for String.compareTo [v7]
On Mon, 30 Aug 2021 06:26:01 GMT, Wang Huang wrote: >> Dear all, >> Can you do me a favor to review this patch. This patch use `ldp` to >> implement String.compareTo. >> >> * We add a JMH test case >> * Here is the result of this test case >> >> Benchmark |(size)| Mode| Cnt|Score | Error |Units >> -|--|-||--||- >> StringCompare.compareLL | 64 | avgt| 5 |7.992 | ± 0.005|us/op >> StringCompare.compareLL | 72 | avgt| 5 |15.029| ± 0.006|us/op >> StringCompare.compareLL | 80 | avgt| 5 |14.655| ± 0.011|us/op >> StringCompare.compareLL | 91 | avgt| 5 |16.363| ± 0.12 |us/op >> StringCompare.compareLL | 101 | avgt| 5 |16.966| ± 0.007|us/op >> StringCompare.compareLL | 121 | avgt| 5 |19.276| ± 0.006|us/op >> StringCompare.compareLL | 181 | avgt| 5 |19.002| ± 0.417|us/op >> StringCompare.compareLL | 256 | avgt| 5 |24.707| ± 0.041|us/op >> StringCompare.compareLLWithLdp| 64 | avgt| 5 |8.001 | ± >> 0.121|us/op >> StringCompare.compareLLWithLdp| 72 | avgt| 5 |11.573| ± 0.003|us/op >> StringCompare.compareLLWithLdp| 80 | avgt| 5 |6.861 | ± 0.004|us/op >> StringCompare.compareLLWithLdp| 91 | avgt| 5 |12.774| ± 0.201|us/op >> StringCompare.compareLLWithLdp| 101 | avgt| 5 |8.691 | ± 0.004|us/op >> StringCompare.compareLLWithLdp| 121 | avgt| 5 |11.091| ± 1.342|us/op >> StringCompare.compareLLWithLdp| 181 | avgt| 5 |14.64 | ± 0.581|us/op >> StringCompare.compareLLWithLdp| 256 | avgt| 5 |25.879| ± 1.775|us/op >> StringCompare.compareUU | 64 | avgt| 5 |13.476| ± 0.01 |us/op >> StringCompare.compareUU | 72 | avgt| 5 |15.078| ± 0.006|us/op >> StringCompare.compareUU | 80 | avgt| 5 |23.512| ± 0.011|us/op >> StringCompare.compareUU | 91 | avgt| 5 |24.284| ± 0.008|us/op >> StringCompare.compareUU | 101 | avgt| 5 |20.707| ± 0.017|us/op >> StringCompare.compareUU | 121 | avgt| 5 |29.302| ± 0.011|us/op >> StringCompare.compareUU | 181 | avgt| 5 |39.31 | ± >> 0.016|us/op >> StringCompare.compareUU | 256 | avgt| 5 |54.592| ± 0.392|us/op >> StringCompare.compareUUWithLdp| 64 | avgt| 5 |16.389| ± 0.008|us/op >> StringCompare.compareUUWithLdp| 72 | avgt| 5 |10.71 | ± 0.158|us/op >> StringCompare.compareUUWithLdp| 80 | avgt| 5 |11.488| ± 0.024|us/op >> StringCompare.compareUUWithLdp| 91 | avgt| 5 |13.412| ± 0.006|us/op >> StringCompare.compareUUWithLdp| 101 | avgt| 5 |16.245| ± 0.434|us/op >> StringCompare.compareUUWithLdp| 121 | avgt| 5 |16.597| ± 0.016|us/op >> StringCompare.compareUUWithLdp| 181 | avgt| 5 |27.373| ± 0.017|us/op >> StringCompare.compareUUWithLdp| 256 | avgt| 5 |41.74 | ± 3.5 |us/op >> >> From this table, we can see that in most cases, our patch is better than old >> one. >> >> Thank you for your review. Any suggestions are welcome. > > Wang Huang has updated the pull request incrementally with one additional > commit since the last revision: > > fix windows build failed Marked as reviewed by aph (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/4722
Re: RFR: 8268231: Aarch64: Use ldp in intrinsics for String.compareTo [v6]
On Fri, 17 Sep 2021 07:13:24 GMT, Wu Yan wrote: >> It's fine. I don't think it'll affect any real programs, so it's rather >> pointless. I don't know if that's any reason not to approve it. > > Andrew, can you help us to approve this? I agree with Andrew Haley that this patch is not going to make an improvement for anything but a very small number of applications. Processing of strings over a few 10s of bytes is rare. On the other hand the doesn't seem to cause any performance drop for the much more common case of processing short strings. so it does no harm. Also, the new and old code are much the same in terms of complexity so that is no reason to prefer one over the other. The only real concern I have is that any change involves the risk of error and the ratio of cases that might benefit to cases that might suffer from an error is very low. I don't think that's a reason to avoid pushing this patch upstream but it does suggest that we should not backport it. - PR: https://git.openjdk.java.net/jdk/pull/4722
Re: RFR: 8247980: Exclusive execution of java/util/stream tests slows down tier1
On Thu, 19 Aug 2021 15:15:31 GMT, Aleksey Shipilev wrote: > See the bug report for more details. I would appreciate if people with their > corporate testing systems would run this through their systems to avoid > surprises. (ping @RealCLanger, @iignatev). Ok, let's try! - PR: https://git.openjdk.java.net/jdk/pull/5189
Re: RFR: 8273314: Add tier4 test groups [v4]
On Fri, 17 Sep 2021 06:59:09 GMT, Aleksey Shipilev wrote:
>> During the review of JDK-8272914 that added hotspot:tier{2,3} groups,
>> @iignatev suggested to create tier4 groups that capture all tests not in
>> tiers{1,2,3}.
>>
>> Caveats:
>> - I excluded `applications` from `hotspot:tier4`, because they require test
>> dependencies (e.g. jcstress).
>> - `jdk:tier4` only runs well with `JTREG_KEYWORDS=!headful` or reduced
>> concurrency with `TEST_JOBS=1`, because headful tests cannot run in parallel
>>
>> Sample run with `JTREG_KEYWORDS=!headful`:
>>
>>
>> ==
>> Test summary
>> ==
>>TEST TOTAL PASS FAIL ERROR
>>
jtreg:test/hotspot/jtreg:tier4 3585 3584 0 1
<<
jtreg:test/jdk:tier4 2893 2887 5 1
<<
>>jtreg:test/langtools:tier40 0 0 0
>>
>>jtreg:test/jaxp:tier4 0 0 0 0
>>
>> ==
>>
>> real 699m39.462s
>> user 6626m8.448s
>> sys 1110m43.704s
>>
>>
>> There are interesting test failures on my machine, which I would address
>> separately.
>
> Aleksey Shipilev has updated the pull request with a new target base due to a
> merge or a rebase. The incremental webrev excludes the unrelated changes
> brought in by the merge/rebase. The pull request contains five additional
> commits since the last revision:
>
> - Merge branch 'master' into JDK-8273314-tier4
> - Merge branch 'master' into JDK-8273314-tier4
> - Drop applications and fix the comment
> - Drop exceptions
> - Add tier4 test groups
All right, there goes.
-
PR: https://git.openjdk.java.net/jdk/pull/5357
