Re: RFR: 8273101: Eliminate the usage of threadgroup sandboxing in the java.util.logging
On Wed, 1 Sep 2021 06:31:16 GMT, Sergey Bylokhov wrote: > At the time Java supported applets and webstart, a special mechanism for > launching various applications in one JVM was used to reduce memory usage and > each application was isolated from each other. > > This isolation was implemented via ThreadGroups where each application > created its own thread group and all data for the application was stored in > the context of its own group. > > Some parts of the JDK used ThreadGroups directly, others used special classes > like sun.awt.AppContext. > > Such sandboxing became useless since the plugin and webstart are no longer > supported and were removed in jdk11. > > Note that this is just a first step for the overall cleanup, here is my plan: > 1. Cleanup the usage of AppContext in the “java.util.logging.LogManager" > class in the java.base module. > 2. Cleanup the usage of TheadGroup in the JavaSound > 3. Cleanup the usage of TheadGroup in the JavaBeans > 4. Cleanup the usage of AppContext in the Swing > 5. Cleanup the usage of AppContext in the AWT > 6. Delete the AppContext class. > > The current PR is for the first step. So this is the request to delete the > usage of AppContext in the LogManager and related tests. > > Tested by tier1/tier2/tier3 and jdk_desktop tests. The change looks okay. My suggestion is to get 1-6 all ready to push around the same time. It's okay to have separate JBS issues and PRs. - PR: https://git.openjdk.java.net/jdk/pull/5326
Re: RFR: 8273660: De-Serialization Stack is suppressing ClassNotFoundException [v2]
On Fri, 29 Oct 2021 16:14:57 GMT, Michael McMahon wrote: > How likely is it that existing code is using ObjectInputStream::getFields and > is already handling class not found by checking for null return from the > returned GetField? Very unlikely, a field value may be null for because it really is null or it is a field for which there is no value in the stream (supporting evolution). Class not found is supposed to show up as an exception. - PR: https://git.openjdk.java.net/jdk/pull/6053
Re: RFR: 8244202: Implementation of JEP 418: Internet-Address Resolution SPI [v10]
On Fri, 29 Oct 2021 16:17:46 GMT, Aleksei Efimov wrote: >> This change implements a new service provider interface for host name and >> address resolution, so that java.net.InetAddress API can make use of >> resolvers other than the platform's built-in resolver. >> >> The following API classes are added to `java.net.spi` package to facilitate >> this: >> - `InetAddressResolverProvider` - abstract class defining a service, and >> is, essentially, a factory for `InetAddressResolver` resolvers. >> - `InetAddressResolverProvider.Configuration ` - an interface describing the >> platform's built-in configuration for resolution operations that could be >> used to bootstrap a resolver construction, or to implement partial >> delegation of lookup operations. >> - `InetAddressResolver` - an interface that defines methods for the >> fundamental forward and reverse lookup operations. >> - `InetAddressResolver.LookupPolicy` - a class whose instances describe the >> characteristics of one forward lookup operation. >> >> More details in [JEP-418](https://openjdk.java.net/jeps/418). >> >> Testing: new and existing `tier1:tier3` tests > > Aleksei Efimov has updated the pull request incrementally with one additional > commit since the last revision: > > Replace 'system' with 'the system-wide', move comment section Marked as reviewed by dfuchs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/5822
Integrated: 8274750: java/io/File/GetXSpace.java failed: '/dev': 191488 != 190976
On Tue, 26 Oct 2021 19:00:44 GMT, Brian Burkhalter wrote: > Please consider this proposed change to ignore comparing the total size of > `/dev` as returned by `DF(1)` and `STAT(2)` on macOS. This pull request has now been integrated. Changeset: 13265f99 Author:Brian Burkhalter URL: https://git.openjdk.java.net/jdk/commit/13265f9901ab8334bbe1e7a571a9c5f386275dbf Stats: 5 lines in 1 file changed: 2 ins; 1 del; 2 mod 8274750: java/io/File/GetXSpace.java failed: '/dev': 191488 != 190976 Reviewed-by: rriggs, naoto - PR: https://git.openjdk.java.net/jdk/pull/6122
Re: RFR: 8244202: Implementation of JEP 418: Internet-Address Resolution SPI [v10]
> This change implements a new service provider interface for host name and > address resolution, so that java.net.InetAddress API can make use of > resolvers other than the platform's built-in resolver. > > The following API classes are added to `java.net.spi` package to facilitate > this: > - `InetAddressResolverProvider` - abstract class defining a service, and is, > essentially, a factory for `InetAddressResolver` resolvers. > - `InetAddressResolverProvider.Configuration ` - an interface describing the > platform's built-in configuration for resolution operations that could be > used to bootstrap a resolver construction, or to implement partial delegation > of lookup operations. > - `InetAddressResolver` - an interface that defines methods for the > fundamental forward and reverse lookup operations. > - `InetAddressResolver.LookupPolicy` - a class whose instances describe the > characteristics of one forward lookup operation. > > More details in [JEP-418](https://openjdk.java.net/jeps/418). > > Testing: new and existing `tier1:tier3` tests Aleksei Efimov has updated the pull request incrementally with one additional commit since the last revision: Replace 'system' with 'the system-wide', move comment section - Changes: - all: https://git.openjdk.java.net/jdk/pull/5822/files - new: https://git.openjdk.java.net/jdk/pull/5822/files/2ca78ba9..f660cc6e Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=5822=09 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5822=08-09 Stats: 8 lines in 1 file changed: 4 ins; 3 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/5822.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5822/head:pull/5822 PR: https://git.openjdk.java.net/jdk/pull/5822
Re: RFR: 8273660: De-Serialization Stack is suppressing ClassNotFoundException [v2]
On Fri, 29 Oct 2021 15:35:50 GMT, Roger Riggs wrote: >> The ObjectInputStream.GetField method `get(String name, Object val)` should >> have been throwing >> a ClassNotFoundException if the class was not found. Instead the >> implementation was returning null. >> A design error does not allow the `get(String name, Object val)` method to >> throw CNFE as it should. >> However, an exception must be thrown to prevent invalid data from being >> returned. >> Wrapping the CNFE in IOException allows it to be thrown and the exception >> handled. >> The call to `get(String name, Object val)` is always from within a >> `readObject` method >> so the deserialization logic can catch the IOException and unwrap it to >> handle the CNFE. > > Roger Riggs has updated the pull request incrementally with one additional > commit since the last revision: > > Correct comment on the handling of ClassNotFoundException Marked as reviewed by naoto (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/6053
Re: RFR: 8244202: Implementation of JEP 418: Internet-Address Resolution SPI [v9]
On Wed, 27 Oct 2021 16:23:29 GMT, Michael McMahon wrote: >> Aleksei Efimov has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Add @ throws NPE to hosts file resolver javadoc > > src/java.base/share/classes/java/net/InetAddress.java line 841: > >> 839: // 'resolver.lookupByAddress' and 'InetAddress.getAllByName0' >> delegate to the system-wide resolver, >> 840: // which could be a custom one. At that point we treat any >> unexpected RuntimeException thrown by >> 841: // the resolver as we would treat an UnknownHostException or an >> unmatched host name. > > indentation issue in comment above Thanks - moved the comment block inside `catch` block (f660cc6ecc7a31c83de220160b9fd8d0fbacd1be) > src/java.base/share/classes/java/net/InetAddress.java line 1308: > >> 1306: * Creates an InetAddress based on the provided host name and IP >> address. >> 1307: * System {@linkplain InetAddressResolver resolver} is not used to >> check >> 1308: * the validity of the address. > > Is this term "system resolver" defined somewhere? I think it means the > configured resolver for the current VM. Previously, it really was the system > resolver. So, I think it's potentially confusing, as there is also reference > to the java.net.preferIPv6Addresses system property as having a possible > value "system" which refers to the operating system. Since the CSR is > approved, I'm happy to discuss this point post integration. Thanks for highlighting that: Changed `"system"` to `"the system-wide"` - that's what was originally meant by `"system resolver"` (f660cc6ecc7a31c83de220160b9fd8d0fbacd1be). - PR: https://git.openjdk.java.net/jdk/pull/5822
Re: RFR: 8273660: De-Serialization Stack is suppressing ClassNotFoundException [v2]
On Fri, 29 Oct 2021 15:35:50 GMT, Roger Riggs wrote: >> The ObjectInputStream.GetField method `get(String name, Object val)` should >> have been throwing >> a ClassNotFoundException if the class was not found. Instead the >> implementation was returning null. >> A design error does not allow the `get(String name, Object val)` method to >> throw CNFE as it should. >> However, an exception must be thrown to prevent invalid data from being >> returned. >> Wrapping the CNFE in IOException allows it to be thrown and the exception >> handled. >> The call to `get(String name, Object val)` is always from within a >> `readObject` method >> so the deserialization logic can catch the IOException and unwrap it to >> handle the CNFE. > > Roger Riggs has updated the pull request incrementally with one additional > commit since the last revision: > > Correct comment on the handling of ClassNotFoundException How likely is it that existing code is using ObjectInputStream::getFields and is already handling class not found by checking for null return from the returned GetField? - PR: https://git.openjdk.java.net/jdk/pull/6053
Re: RFR: 8275766: (tz) Update Timezone Data to 2021e
On Thu, 28 Oct 2021 01:02:27 GMT, Yoshiki Sato wrote: > Please review the integration of tzdata2021e (including tzdata2021d) to the > JDK. > The fix has passed all relevant JTREG regression tests and JCK tests. > > 8275754: (tz) Update Timezone Data to 2021d > 8275849: TestZoneInfo310.java fails with tzdata2021e Marked as reviewed by erikj (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/6144
Re: RFR: JDK-8266431: Dual-Pivot Quicksort improvements (Radix sort) [v7]
> Sorting: > > - adopt radix sort for sequential and parallel sorts on int/long/float/double > arrays (almost random and length > 6K) > - fix tryMergeRuns() to better handle case when the last run is a single > element > - minor javadoc and comment changes > > Testing: > - add new data inputs in tests for sorting > - add min/max/infinity values to float/double testing > - add tests for radix sort iaroslavski has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains ten commits: - JDK-8266431: Dual-Pivot Quicksort improvements (Radix sort) Merge with external changes - JDK-8266431: Dual-Pivot Quicksort improvements (Radix sort) Added more comments - JDK-8266431: Dual-Pivot Quicksort improvements (Radix sort) Better naming of methods - JDK-8266431: Dual-Pivot Quicksort improvements (Radix sort) Simplified mixed insertion sort - Merge remote-tracking branch 'upstream/master' into JDK-8266431-Dual-Pivot-Quicksort-improvements-Radix-sort - JDK-8266431: Dual-Pivot Quicksort improvements (Radix sort) Update target version - JDK-8266431: Dual-Pivot Quicksort improvements (Radix sort) Testing: - remove @since and @date, otherwise jtreg tag parser fails - JDK-8266431: Dual-Pivot Quicksort improvements (Radix sort) Sorting: - move radix sort out from quicksort partitioning - rename radixSort to tryRadixSort - minor javadoc and comment changes Testing: - rename radixSort to tryRadixSort in helper - JDK-8266431: Dual-Pivot Quicksort improvements (Radix sort) Sorting: - adopt radix sort for sequential and parallel sorts on int/long/float/double arrays (almost random and length > 6K) - fix tryMergeRuns() to better handle case when the last run is a single element - minor javadoc and comment changes Testing: - add new data inputs in tests for sorting - add min/max/infinity values to float/double testing - add tests for radix sort - Changes: https://git.openjdk.java.net/jdk/pull/3938/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3938=06 Stats: 1288 lines in 3 files changed: 855 ins; 102 del; 331 mod Patch: https://git.openjdk.java.net/jdk/pull/3938.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3938/head:pull/3938 PR: https://git.openjdk.java.net/jdk/pull/3938
Re: RFR: 8273660: De-Serialization Stack is suppressing ClassNotFoundException [v2]
On Fri, 29 Oct 2021 15:35:50 GMT, Roger Riggs wrote: >> The ObjectInputStream.GetField method `get(String name, Object val)` should >> have been throwing >> a ClassNotFoundException if the class was not found. Instead the >> implementation was returning null. >> A design error does not allow the `get(String name, Object val)` method to >> throw CNFE as it should. >> However, an exception must be thrown to prevent invalid data from being >> returned. >> Wrapping the CNFE in IOException allows it to be thrown and the exception >> handled. >> The call to `get(String name, Object val)` is always from within a >> `readObject` method >> so the deserialization logic can catch the IOException and unwrap it to >> handle the CNFE. > > Roger Riggs has updated the pull request incrementally with one additional > commit since the last revision: > > Correct comment on the handling of ClassNotFoundException Please file a CSR for the change in behavior. - PR: https://git.openjdk.java.net/jdk/pull/6053
Re: RFR: 8273660: De-Serialization Stack is suppressing ClassNotFoundException [v2]
On Fri, 29 Oct 2021 15:06:12 GMT, Daniel Fuchs wrote: >> Roger Riggs has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Correct comment on the handling of ClassNotFoundException > > src/java.base/share/classes/java/io/ObjectInputStream.java line 2663: > >> 2661: ClassNotFoundException ex = >> handles.lookupException(objHandle); >> 2662: if (ex != null) { >> 2663: // Wrap the exception so it can be handled in >> GetField.get(String, Object) > > I am not sure I understand this comment. We are in `GetField.get(String, > Object)`, aren't we? Right, will correct comment to refer to the `invokeReadObject` call in `readSerialData`. - PR: https://git.openjdk.java.net/jdk/pull/6053
Re: RFR: 8273660: De-Serialization Stack is suppressing ClassNotFoundException [v2]
> The ObjectInputStream.GetField method `get(String name, Object val)` should > have been throwing > a ClassNotFoundException if the class was not found. Instead the > implementation was returning null. > A design error does not allow the `get(String name, Object val)` method to > throw CNFE as it should. > However, an exception must be thrown to prevent invalid data from being > returned. > Wrapping the CNFE in IOException allows it to be thrown and the exception > handled. > The call to `get(String name, Object val)` is always from within a > `readObject` method > so the deserialization logic can catch the IOException and unwrap it to > handle the CNFE. Roger Riggs has updated the pull request incrementally with one additional commit since the last revision: Correct comment on the handling of ClassNotFoundException - Changes: - all: https://git.openjdk.java.net/jdk/pull/6053/files - new: https://git.openjdk.java.net/jdk/pull/6053/files/bc467cab..438548e9 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=6053=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=6053=00-01 Stats: 2 lines in 1 file changed: 1 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/6053.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6053/head:pull/6053 PR: https://git.openjdk.java.net/jdk/pull/6053
Re: RFR: 8273660: De-Serialization Stack is suppressing ClassNotFoundException
On Wed, 20 Oct 2021 21:57:29 GMT, Roger Riggs wrote: > The ObjectInputStream.GetField method `get(String name, Object val)` should > have been throwing > a ClassNotFoundException if the class was not found. Instead the > implementation was returning null. > A design error does not allow the `get(String name, Object val)` method to > throw CNFE as it should. > However, an exception must be thrown to prevent invalid data from being > returned. > Wrapping the CNFE in IOException allows it to be thrown and the exception > handled. > The call to `get(String name, Object val)` is always from within a > `readObject` method > so the deserialization logic can catch the IOException and unwrap it to > handle the CNFE. src/java.base/share/classes/java/io/ObjectInputStream.java line 2663: > 2661: ClassNotFoundException ex = > handles.lookupException(objHandle); > 2662: if (ex != null) { > 2663: // Wrap the exception so it can be handled in > GetField.get(String, Object) I am not sure I understand this comment. We are in `GetField.get(String, Object)`, aren't we? - PR: https://git.openjdk.java.net/jdk/pull/6053
Re: RFR: 8273660: De-Serialization Stack is suppressing ClassNotFoundException
On Wed, 20 Oct 2021 21:57:29 GMT, Roger Riggs wrote: > The ObjectInputStream.GetField method `get(String name, Object val)` should > have been throwing > a ClassNotFoundException if the class was not found. Instead the > implementation was returning null. > A design error does not allow the `get(String name, Object val)` method to > throw CNFE as it should. > However, an exception must be thrown to prevent invalid data from being > returned. > Wrapping the CNFE in IOException allows it to be thrown and the exception > handled. > The call to `get(String name, Object val)` is always from within a > `readObject` method > so the deserialization logic can catch the IOException and unwrap it to > handle the CNFE. LGTM - PR: https://git.openjdk.java.net/jdk/pull/6053
Re: JDK-8275509: (jlink) SystemModulesPlugin generates a jdk.internal.module.SystemModules$all.class which isn't reproducible
Hello Ioi (and others), On 22/10/21 1:39 pm, Jaikiran Pai wrote: Hello Ioi, On 22/10/21 12:29 pm, Ioi Lam wrote: On 10/21/21 9:09 PM, Jaikiran Pai wrote: Hello Ioi, This is my initial analysis of the problem. >>> Can anyone think of similar problems that may happen elsewhere? The static constructors of enum classes are executed at both CDS dump time and run time. E.g., public enum Modifier { OPEN } The method essentially does this: public static final Modifier OPEN = new Modifier("OPEN"); If a reference of Modifier.OPEN is stored inside the CDS archived heap during dump time, it will be different than the value of Modifier.OPEN that is re-created at runtime by the execution of Modifier. I have almost next to nothing knowledge about CDS internals. My only understanding of it is based on some documentation that I have read. One of them being this one https://docs.oracle.com/en/java/javase/17/vm/class-data-sharing.html#GUID-7EAA3411-8CF0-4D19-BD05-DF5E1780AA91. Based on that documentation (and other similar ones), it was my understanding that CDS was meant to store/share class "metadata" like it states in that doc: "When the JVM starts, the shared archive is memory-mapped to allow sharing of read-only JVM metadata for these classes among multiple JVM processes." But from what you explain in that enum example, it looks like it also stores class instance data that is computed at build time on the host where the JDK image was generated? Did I understand it correctly? Is this only for enums or does it also store the static initialization data of "class" types too? If it does store the static init data of class types too, then wouldn't such data be host/build time specific and as such the classes that need to be enrolled into the default CDS archive of the JDK should be very selective (by reviewing what they do in their static init)? Like I said, I haven't looked into this in detail so perhaps it already is selective in the JDK build? Hi Jaikiran, Thank you very much for the detailed response. CDS also has the ability to archive Java heap object. Since https://bugs.openjdk.java.net/browse/JDK-8244778 , we have archived the entire module graph to improve start-up time. At run time, the module graph (as well as other archived heap objects) are loaded from the CDS archive and put into the Java heap (either through memory mapping or copying). That is interesting and something that I hadn't known. You can see the related code in jdk.internal.module.ModuleBootstrap::boot() I just had a look at it and it's quite elaborate and it'll take a me while to fully grasp it (if at all) given its understandable complexity. When the module system has started up, the module graph will reference a copy of the OPEN enum object that was created as part of the archive. However, the Modifier. will also be executed at VM start-up, causing a second copy of the OPEN enum object to be stored into the static field Modified::OPEN. Thank you for that detail. That helps me understand this a bit more (and opens a few questions). To be clear - the VM startup code which creates that other copy, ends up creating that copy because that piece of initialization happens even before the module system has fully started up and created those references from the archived state? Otherwise, the classloaders I believe would be smart enough to not run that static init again, had the module system with that graph from the archived state been fully "up"? So would this mean that this not just impacts enums but essentially every class referenced within the module system (of just boot layer?) that has state which is initialized during static init? To be more precise, consider the very common example of loggers which are typically static initialized and stored in a static (final) field: private static final java.util.logger.Logger logger = Logger.getLogger(SomeClass.class); If the boot layer module graph has any classes which has state like this, it would then mean that if such classes do get initialized very early on during VM startup, then they too are impacted and the module graph holding instances of such classes will end up using a different instance for such fields as compared to the rest of the application code? In essence, such classes which get accessed early (before module system with the archived graph is "up") during VM startup can end up _virtually_ having their static initialization run twice (I understand it won't be run twice, but that's the end result, isn't it)? I was really curious why this was only applicable to enums and why other static initialization (like the one I explained above) wasn't impacted. So I decided to do a small PoC. To come up with an illustration that this impacts regular static initialization where enums aren't involved, I decided to add a small piece of code in the
Re: RFR: 8276164: RandomAccessFile#write method could throw IndexOutOfBoundsException that is not described in javadoc [v2]
> Could you please review the 8276164 bug fixes? > > I suggest adding the missing javadoc of `@throws IndexOutOfBoundsException`. Masanori Yano has updated the pull request incrementally with two additional commits since the last revision: - 8276164: RandomAccessFile#write method could throw IndexOutOfBoundsException that is not described in javadoc - 8276164: RandomAccessFile#write method could throw IndexOutOfBoundsException that is not described in javadoc - Changes: - all: https://git.openjdk.java.net/jdk/pull/6168/files - new: https://git.openjdk.java.net/jdk/pull/6168/files/bf4b0e25..a1cf0531 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=6168=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=6168=00-01 Stats: 7 lines in 2 files changed: 3 ins; 2 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/6168.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6168/head:pull/6168 PR: https://git.openjdk.java.net/jdk/pull/6168
Re: RFR: JDK-8275650 Problemlist java/io/File/createTempFile/SpecialTempFile.java for Windows 11 [v5]
On Fri, 29 Oct 2021 10:38:41 GMT, Ivan Šipka wrote: >> cc @ctornqvi > > Ivan Šipka has updated the pull request incrementally with one additional > commit since the last revision: > > removed file added by accident Marked as reviewed by msheppar (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/6025
Re: RFR: 8276164: RandomAccessFile#write method could throw IndexOutOfBoundsException that is not described in javadoc
On Fri, 29 Oct 2021 10:37:07 GMT, Masanori Yano wrote: > Could you please review the 8276164 bug fixes? > > I suggest adding the missing javadoc of `@throws IndexOutOfBoundsException`. Thanks for separating this from the image I/O issue. For JDK-8276164 then I think we should add the @throws to DataOutput.write, then RAF can inherit it. - PR: https://git.openjdk.java.net/jdk/pull/6168
Re: RFR: 8274122: Problemlist java/io/File/createTempFile/SpecialTempFile.java for Windows 11 [v5]
On Fri, 29 Oct 2021 10:38:41 GMT, Ivan Šipka wrote: >> cc @ctornqvi > > Ivan Šipka has updated the pull request incrementally with one additional > commit since the last revision: > > removed file added by accident bugid in title should be 8275650: i.e. the bugid for problemlist update - PR: https://git.openjdk.java.net/jdk/pull/6025
RFR: 8276164: RandomAccessFile#write method could throw IndexOutOfBoundsException that is not described in javadoc
Could you please review the 8276164 bug fixes? I suggest adding the missing javadoc of `@throws IndexOutOfBoundsException`. - Commit messages: - 8276164: RandomAccessFile#write method could throw IndexOutOfBoundsException that is not described in javadoc Changes: https://git.openjdk.java.net/jdk/pull/6168/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=6168=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8276164 Stats: 3 lines in 1 file changed: 3 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/6168.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6168/head:pull/6168 PR: https://git.openjdk.java.net/jdk/pull/6168
Re: RFR: 8274122: Problemlist java/io/File/createTempFile/SpecialTempFile.java for Windows 11 [v3]
On Thu, 28 Oct 2021 18:21:29 GMT, Mark Sheppard wrote: > as per comment from Igor above, the bug id in the problemlist is incorrect, > and it should be 8274122 you have used the bugid for this problemlist update @msheppar the bug id has been updated - PR: https://git.openjdk.java.net/jdk/pull/6025
Re: RFR: 8274122: Problemlist java/io/File/createTempFile/SpecialTempFile.java for Windows 11 [v5]
> cc @ctornqvi Ivan Šipka has updated the pull request incrementally with one additional commit since the last revision: removed file added by accident - Changes: - all: https://git.openjdk.java.net/jdk/pull/6025/files - new: https://git.openjdk.java.net/jdk/pull/6025/files/f5ea03fa..d29620d1 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=6025=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk=6025=03-04 Stats: 15 lines in 1 file changed: 0 ins; 15 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/6025.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6025/head:pull/6025 PR: https://git.openjdk.java.net/jdk/pull/6025
Re: RFR: 8252990: Intrinsify Unsafe.storeStoreFence [v2]
On Thu, 28 Oct 2021 08:58:48 GMT, Aleksey Shipilev wrote: >> `Unsafe.storeStoreFence` currently delegates to stronger >> `Unsafe.storeFence`. We can teach compilers to map this directly to already >> existing rules that handle `MemBarStoreStore`. Like explicit >> `LoadFence`/`StoreFence`, we introduce the special node to differentiate >> explicit fence and implicit store-store barriers. `storeStoreFence` is >> usually used to simulate safe `final`-field like constructions in special >> JDK classes, like `ConstantCallSite` and friends. >> >> Motivational performance difference on benchmarks from JDK-8276054 on ARM32: >> >> >> Benchmark Mode Cnt ScoreError Units >> Multiple.plain avgt3 2.669 ± 0.004 ns/op >> Multiple.release avgt3 16.688 ± 0.057 ns/op >> Multiple.storeStoreavgt3 14.021 ± 0.144 ns/op // Better >> >> MultipleWithLoads.plainavgt3 4.672 ± 0.053 ns/op >> MultipleWithLoads.release avgt3 16.689 ± 0.044 ns/op >> MultipleWithLoads.storeStore avgt3 14.012 ± 0.010 ns/op // Better >> >> MultipleWithStores.plain avgt3 14.687 ± 0.009 ns/op >> MultipleWithStores.release avgt3 45.393 ± 0.192 ns/op >> MultipleWithStores.storeStore avgt3 38.048 ± 0.033 ns/op // Better >> >> Publishing.plain avgt3 27.079 ± 0.201 ns/op >> Publishing.release avgt3 27.088 ± 0.241 ns/op >> Publishing.storeStore avgt3 27.009 ± 0.259 ns/op // Within >> error, hidden by allocation >> >> Single.plain avgt3 2.670 ± 0.002 ns/op >> Single.releaseFenceavgt3 6.675 ± 0.001 ns/op >> Single.storeStoreFence avgt3 8.012 ± 0.027 ns/op // Worse, >> seems to be ARM32 implementation artifact >> >> >> As expected, this does not affect x86_64 at all, because both `release` and >> `storeStore` are effectively no-ops, only affecting compiler optimizations: >> >> >> Benchmark Mode Cnt Score Error Units >> >> Multiple.plain avgt3 0.406 ± 0.002 ns/op >> Multiple.release avgt3 0.409 ± 0.018 ns/op >> Multiple.storeStoreavgt3 0.406 ± 0.001 ns/op >> >> MultipleWithLoads.plainavgt3 4.328 ± 0.006 ns/op >> MultipleWithLoads.release avgt3 4.600 ± 0.014 ns/op >> MultipleWithLoads.storeStore avgt3 4.602 ± 0.006 ns/op >> >> MultipleWithStores.plain avgt3 0.812 ± 0.001 ns/op >> MultipleWithStores.release avgt3 0.812 ± 0.002 ns/op >> MultipleWithStores.storeStore avgt3 0.812 ± 0.002 ns/op >> >> Publishing.plain avgt3 6.370 ± 0.059 ns/op >> Publishing.release avgt3 6.358 ± 0.436 ns/op >> Publishing.storeStore avgt3 6.367 ± 0.054 ns/op >> >> Single.plain avgt3 0.407 ± 0.039 ns/op >> Single.releaseFenceavgt3 0.406 ± 0.001 ns/op >> Single.storeStoreFence avgt3 0.406 ± 0.001 ns/op >> >> >> Additional testing: >> - [x] Linux x86_64 fastdebug `tier1` > > Aleksey Shipilev has updated the pull request incrementally with one > additional commit since the last revision: > > Fix the comment to match JDK-8276096 LGTM - Marked as reviewed by whuang (Author). PR: https://git.openjdk.java.net/jdk/pull/6136
Re: RFR: 8276102: JDK-8245095 integration reverted JDK-8247980
On Thu, 28 Oct 2021 11:13:57 GMT, Aleksey Shipilev wrote: > See the [integration > commit](https://github.com/openjdk/jdk/commit/9d191fce55fa70d6a2affc724fad57b0e20e4bde#diff-5b9b15832385ab8e02ffca3ddef6d65a9dea73f45abbe5e4f0be561be02073ffR30 > ) for JDK-8245095. It reintroduced `java/util/stream` in > `exclusiveAccess.dirs`, which effectively reverts JDK-8247980. I noticed this > because jdk:tier1 became slow again. > > @FrauBoes, that was not intentional, right? > > The solution is to drop `java/util/stream` again. > > Additional testing: > - [x] Linux x86_64 fastdebug `jdk:tier1` tests are fast again All right, thanks for reviews. - PR: https://git.openjdk.java.net/jdk/pull/6153
Integrated: 8276102: JDK-8245095 integration reverted JDK-8247980
On Thu, 28 Oct 2021 11:13:57 GMT, Aleksey Shipilev wrote: > See the [integration > commit](https://github.com/openjdk/jdk/commit/9d191fce55fa70d6a2affc724fad57b0e20e4bde#diff-5b9b15832385ab8e02ffca3ddef6d65a9dea73f45abbe5e4f0be561be02073ffR30 > ) for JDK-8245095. It reintroduced `java/util/stream` in > `exclusiveAccess.dirs`, which effectively reverts JDK-8247980. I noticed this > because jdk:tier1 became slow again. > > @FrauBoes, that was not intentional, right? > > The solution is to drop `java/util/stream` again. > > Additional testing: > - [x] Linux x86_64 fastdebug `jdk:tier1` tests are fast again This pull request has now been integrated. Changeset: 15fd8a30 Author:Aleksey Shipilev URL: https://git.openjdk.java.net/jdk/commit/15fd8a300b503fada7611004b5cb1bda6ecc292e Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod 8276102: JDK-8245095 integration reverted JDK-8247980 Reviewed-by: dfuchs, redestad - PR: https://git.openjdk.java.net/jdk/pull/6153
Re: RFR: 8276096: Simplify Unsafe.{load|store}Fence fallbacks by delegating to fullFence
On Thu, 28 Oct 2021 08:47:31 GMT, Aleksey Shipilev wrote: > `Unsafe.{load|store}Fence` falls back to `unsafe.cpp` for > `OrderAccess::{acquire|release}Fence()`. It seems too heavy-handed (useless?) > to call to runtime for a single memory barrier. We can simplify the native > `Unsafe` interface by falling back to `fullFence` when `{load|store}Fence` > intrinsics are not available. This would be similar to what > `Unsafe.{loadLoad|storeStore}Fences` do. > > This is the behavior of these intrinsics now, on x86_64, using benchmarks > from JDK-8276054: > > > Benchmark Mode Cnt Score Error Units > > # Default > Single.acquire avgt3 0.407 ± 0.060 ns/op > Single.fullavgt3 4.693 ± 0.005 ns/op > Single.loadLoadavgt3 0.415 ± 0.095 ns/op > Single.plain avgt3 0.406 ± 0.002 ns/op > Single.release avgt3 0.408 ± 0.047 ns/op > Single.storeStore avgt3 0.408 ± 0.043 ns/op > > # -XX:DisableIntrinsic=_storeFence > Single.acquire avgt3 0.408 ± 0.016 ns/op > Single.fullavgt3 4.694 ± 0.002 ns/op > Single.loadLoadavgt3 0.406 ± 0.002 ns/op > Single.plain avgt3 0.406 ± 0.001 ns/op > Single.release avgt3 4.694 ± 0.003 ns/op <--- upgraded to full > Single.storeStore avgt3 4.690 ± 0.005 ns/op <--- upgraded to full > > # -XX:DisableIntrinsic=_loadFence > Single.acquire avgt3 4.691 ± 0.001 ns/op <--- upgraded to full > Single.fullavgt3 4.693 ± 0.009 ns/op > Single.loadLoadavgt3 4.693 ± 0.013 ns/op <--- upgraded to full > Single.plain avgt3 0.408 ± 0.072 ns/op > Single.release avgt3 0.415 ± 0.016 ns/op > Single.storeStore avgt3 0.416 ± 0.041 ns/op > > # -XX:DisableIntrinsic=_fullFence > Single.acquire avgt3 0.406 ± 0.014 ns/op > Single.fullavgt3 15.836 ± 0.151 ns/op <--- calls runtime > Single.loadLoadavgt3 0.406 ± 0.001 ns/op > Single.plain avgt3 0.426 ± 0.361 ns/op > Single.release avgt3 0.407 ± 0.021 ns/op > Single.storeStore avgt3 0.410 ± 0.061 ns/op > > # -XX:DisableIntrinsic=_fullFence,_loadFence > Single.acquire avgt3 15.822 ± 0.282 ns/op <--- upgraded, calls > runtime > Single.fullavgt3 15.851 ± 0.127 ns/op <--- calls runtime > Single.loadLoadavgt3 15.829 ± 0.045 ns/op <--- upgraded, calls > runtime > Single.plain avgt3 0.406 ± 0.001 ns/op > Single.release avgt3 0.414 ± 0.156 ns/op > Single.storeStore avgt3 0.422 ± 0.452 ns/op > > # -XX:DisableIntrinsic=_fullFence,_storeFence > Single.acquire avgt3 0.407 ± 0.016 ns/op > Single.fullavgt3 15.347 ± 6.783 ns/op <--- calls runtime > Single.loadLoadavgt3 0.406 ± 0.001 ns/op > Single.plain avgt3 0.406 ± 0.002 ns/op > Single.release avgt3 15.828 ± 0.019 ns/op <--- upgraded, calls > runtime > Single.storeStore avgt3 15.834 ± 0.045 ns/op <--- upgraded, calls > runtime > > # -XX:DisableIntrinsic=_fullFence,_loadFence,_storeFence > Single.acquire avgt3 15.838 ± 0.030 ns/op <--- upgraded, calls > runtime > Single.fullavgt3 15.854 ± 0.277 ns/op <--- calls runtime > Single.loadLoadavgt3 15.826 ± 0.160 ns/op <--- upgraded, calls > runtime > Single.plain avgt3 0.406 ± 0.003 ns/op > Single.release avgt3 15.838 ± 0.019 ns/op <--- upgraded, calls > runtime > Single.storeStore avgt3 15.844 ± 0.104 ns/op <--- upgraded, calls > runtime > > > Additional testing: > - [x] Linux x86_64 fastdebug `tier1` Marked as reviewed by aph (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/6149
Re: RFR: 8262297: ImageIO.write() method will throw IndexOutOfBoundsException [v2]
> Could you please review the 8262297 bug fixes? > > In this case, ImageIO.write() should throw java.io.IOException rather than > java.lang.IndexOutOfBoundsException. IndexOutOfBoundsException is caught and > wrapped in IIOException in ImageIO.write() with this fix. In addition, > IndexOutOfBoundsException is not expected to throw by > RandomAccessFile#write() according to its API specification. So it should be > fixed. Masanori Yano has updated the pull request incrementally with one additional commit since the last revision: 8262297: ImageIO.write() method will throw IndexOutOfBoundsException - Changes: - all: https://git.openjdk.java.net/jdk/pull/6151/files - new: https://git.openjdk.java.net/jdk/pull/6151/files/d6a652fa..08b54fff Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=6151=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=6151=00-01 Stats: 3 lines in 1 file changed: 0 ins; 3 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/6151.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6151/head:pull/6151 PR: https://git.openjdk.java.net/jdk/pull/6151
Re: RFR: 8262297: ImageIO.write() method will throw IndexOutOfBoundsException
On Fri, 29 Oct 2021 07:13:32 GMT, Alan Bateman wrote: >> Unfortunately the parent class does not specify this exception via javadoc >> tags like "@throws IndexOutOfBoundsException", but instead just describe it >> in the method description >> >> should we fix it separately from the imageio fix? > >> Unfortunately the parent class does not specify this exception via javadoc >> tags like "@throws IndexOutOfBoundsException", but instead just describe it >> in the method description >> >> should we fix it separately from the imageio fix? > > Yes, it might be better to separate them because we'll like need a CSR if we > are missing @throws in a few places. I understood to separate javadoc fix. I will remove the javadoc fix from this PR and issue another Bug ID. - PR: https://git.openjdk.java.net/jdk/pull/6151
Re: RFR: 8276102: JDK-8245095 integration reverted JDK-8247980
On Thu, 28 Oct 2021 11:13:57 GMT, Aleksey Shipilev wrote: > See the [integration > commit](https://github.com/openjdk/jdk/commit/9d191fce55fa70d6a2affc724fad57b0e20e4bde#diff-5b9b15832385ab8e02ffca3ddef6d65a9dea73f45abbe5e4f0be561be02073ffR30 > ) for JDK-8245095. It reintroduced `java/util/stream` in > `exclusiveAccess.dirs`, which effectively reverts JDK-8247980. I noticed this > because jdk:tier1 became slow again. > > @FrauBoes, that was not intentional, right? > > The solution is to drop `java/util/stream` again. > > Additional testing: > - [x] Linux x86_64 fastdebug `jdk:tier1` tests are fast again Marked as reviewed by redestad (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/6153
Re: RFR: 8276102: JDK-8245095 integration reverted JDK-8247980
On Thu, 28 Oct 2021 11:13:57 GMT, Aleksey Shipilev wrote: > See the [integration > commit](https://github.com/openjdk/jdk/commit/9d191fce55fa70d6a2affc724fad57b0e20e4bde#diff-5b9b15832385ab8e02ffca3ddef6d65a9dea73f45abbe5e4f0be561be02073ffR30 > ) for JDK-8245095. It reintroduced `java/util/stream` in > `exclusiveAccess.dirs`, which effectively reverts JDK-8247980. I noticed this > because jdk:tier1 became slow again. > > @FrauBoes, that was not intentional, right? > > The solution is to drop `java/util/stream` again. > > Additional testing: > - [x] Linux x86_64 fastdebug `jdk:tier1` tests are fast again Marked as reviewed by dfuchs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/6153
Re: RFR: 8276102: JDK-8245095 integration reverted JDK-8247980
On Thu, 28 Oct 2021 11:13:57 GMT, Aleksey Shipilev wrote: > See the [integration > commit](https://github.com/openjdk/jdk/commit/9d191fce55fa70d6a2affc724fad57b0e20e4bde#diff-5b9b15832385ab8e02ffca3ddef6d65a9dea73f45abbe5e4f0be561be02073ffR30 > ) for JDK-8245095. It reintroduced `java/util/stream` in > `exclusiveAccess.dirs`, which effectively reverts JDK-8247980. I noticed this > because jdk:tier1 became slow again. > > @FrauBoes, that was not intentional, right? > > The solution is to drop `java/util/stream` again. > > Additional testing: > - [x] Linux x86_64 fastdebug `jdk:tier1` tests are fast again That was definitely not intentional, thanks for spotting @shipilev! - PR: https://git.openjdk.java.net/jdk/pull/6153
Re: RFR: 8262297: ImageIO.write() method will throw IndexOutOfBoundsException
On Thu, 28 Oct 2021 09:29:13 GMT, Masanori Yano wrote: > Could you please review the 8262297 bug fixes? > > In this case, ImageIO.write() should throw java.io.IOException rather than > java.lang.IndexOutOfBoundsException. IndexOutOfBoundsException is caught and > wrapped in IIOException in ImageIO.write() with this fix. In addition, > IndexOutOfBoundsException is not expected to throw by > RandomAccessFile#write() according to its API specification. So it should be > fixed. This needs looking at closely .. I don't know what's best but the imaging APIs are very exception heavy because of the arrays, indices, you name it that they consume. Most of the time its probably best for an IIO user to just get an IIOException wrapping the original cause. But there could be exceptions. - PR: https://git.openjdk.java.net/jdk/pull/6151
Re: RFR: 8262297: ImageIO.write() method will throw IndexOutOfBoundsException
On Fri, 29 Oct 2021 07:01:04 GMT, Sergey Bylokhov wrote: > Unfortunately the parent class does not specify this exception via javadoc > tags like "@throws IndexOutOfBoundsException", but instead just describe it > in the method description > > should we fix it separately from the imageio fix? Yes, it might be better to separate them because we'll like need a CSR if we are missing @throws in a few places. - PR: https://git.openjdk.java.net/jdk/pull/6151
Re: RFR: 8262297: ImageIO.write() method will throw IndexOutOfBoundsException
On Thu, 28 Oct 2021 14:30:20 GMT, Alan Bateman wrote: >> Could you please review the 8262297 bug fixes? >> >> In this case, ImageIO.write() should throw java.io.IOException rather than >> java.lang.IndexOutOfBoundsException. IndexOutOfBoundsException is caught and >> wrapped in IIOException in ImageIO.write() with this fix. In addition, >> IndexOutOfBoundsException is not expected to throw by >> RandomAccessFile#write() according to its API specification. So it should be >> fixed. > > src/java.base/share/classes/java/io/RandomAccessFile.java line 558: > >> 556: * @throws IndexOutOfBoundsException If {@code off} is negative, >> 557: * {@code len} is negative, or {@code len} is greater >> than >> 558: * {@code b.length - off} > > The IOOBE is specified in the super interface, it's just not shown in the > javadoc because it's a runtime exception. So I think what you want here is: > > @throws IndexOutOfBoundsException {@inheritDoc} Unfortunately the parent class does not specify this exception via javadoc tags like "@throws IndexOutOfBoundsException", but instead just describe it in the method description should we fix it separately from the imageio fix? - PR: https://git.openjdk.java.net/jdk/pull/6151
Re: RFR: 8275766: (tz) Update Timezone Data to 2021e
On Thu, 28 Oct 2021 01:02:27 GMT, Yoshiki Sato wrote: > Please review the integration of tzdata2021e (including tzdata2021d) to the > JDK. > The fix has passed all relevant JTREG regression tests and JCK tests. > > 8275754: (tz) Update Timezone Data to 2021d > 8275849: TestZoneInfo310.java fails with tzdata2021e Marked as reviewed by iris (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/6144