Re: RFR: 8273101: Eliminate the usage of threadgroup sandboxing in the java.util.logging

2021-10-29 Thread Mandy Chung
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]

2021-10-29 Thread Roger Riggs
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]

2021-10-29 Thread Daniel Fuchs
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

2021-10-29 Thread Brian Burkhalter
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]

2021-10-29 Thread Aleksei Efimov
> 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]

2021-10-29 Thread Naoto Sato
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]

2021-10-29 Thread Aleksei Efimov
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]

2021-10-29 Thread Michael McMahon
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

2021-10-29 Thread Erik Joelsson
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]

2021-10-29 Thread iaroslavski
> 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]

2021-10-29 Thread Joe Darcy
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]

2021-10-29 Thread Roger Riggs
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]

2021-10-29 Thread Roger Riggs
> 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

2021-10-29 Thread Daniel Fuchs
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

2021-10-29 Thread Julia Boes
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

2021-10-29 Thread Jaikiran Pai

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]

2021-10-29 Thread Masanori Yano
> 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]

2021-10-29 Thread Mark Sheppard
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

2021-10-29 Thread Alan Bateman
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]

2021-10-29 Thread Mark Sheppard
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

2021-10-29 Thread Masanori Yano
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]

2021-10-29 Thread Ivan Šipka
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]

2021-10-29 Thread Ivan Šipka
> 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]

2021-10-29 Thread Wang Huang
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

2021-10-29 Thread Aleksey Shipilev
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

2021-10-29 Thread Aleksey Shipilev
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

2021-10-29 Thread Andrew Haley
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]

2021-10-29 Thread Masanori Yano
> 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

2021-10-29 Thread Masanori Yano
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

2021-10-29 Thread Claes Redestad
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

2021-10-29 Thread Daniel Fuchs
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

2021-10-29 Thread Julia Boes
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

2021-10-29 Thread Phil Race
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

2021-10-29 Thread Alan Bateman
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

2021-10-29 Thread Sergey Bylokhov
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

2021-10-29 Thread Iris Clark
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