RFR: JDK-8231857: App and Application folder icons are not aligned correctly

[Alexander Matveev]

Please review the jpackage fix for bug [1] at [2].

This is a fix for the JDK-8200758-branch branch of the open sandbox 
repository (jpackage).


- Not sure what problem was, but after moving hidden items at same Y 
value as non-hidden icons issue was resolved. Also, had to adjust Y 
value to align icons with background image. Now it works properly when 
hidden files enabled or disabled in Finder.


[1] https://bugs.openjdk.java.net/browse/JDK-8231857

[2] http://cr.openjdk.java.net/~almatvee/8231857/webrev.00/

Thanks,
Alexander

Re: RFR: 8231584: Deadlock with ClassLoader.findLibrary and System.loadLibrary call

[Mandy Chung]

On 10/11/19 2:35 AM, Anton Kozlov wrote:

On 11.10.2019 00:28, Mandy Chung wrote:

Since the method throws Exception, this try-catch block is not needed.

The start year of the copyright in the new test files should be 2019.

Thanks! I preserved Amazon's copyright year as 2018, as I derived the file from 
JDK-8194653 initially ([1])

[1]: 
https://bugs.openjdk.java.net/secure/attachment/79869/JDK-8194653-Deadlock-involving-FileSystems.patch


Ah, I see.

Otherwise, looks good.
Is there anyone sponsoring this patch for you?  If not, I can do that.

Not yet. It would be great if you sponsor the patch.

Updated webrev: http://cr.openjdk.java.net/~akozlov/8231584/webrev.05/




Pushed.

Mandy

Re: [14] RFR 8212749: DecimalFormat.setGroupingSize(int) allows setting negative grouping size, 8231984: Clarify semantics of DecimalFormat.getGroupingSize(0)

[naoto . sato]

Thanks, Roger. Modified readObject() accordingly:

https://cr.openjdk.java.net/~naoto/8212749.8231984/webrev.01/

Naoto

On 10/11/19 10:41 AM, Roger Riggs wrote:

Hi Naoto,

The javadoc/spec comments look fine.

Code comments at DecimalFormat:4035 give some latitute for the value to be
out of range and since getGroupingSize returns the groupingSize byte
it would be cleaner if the value was always in the valid range
regardless of the isGroupingUsed boolean.

Can there be code in the readObject method to correct out of range
values, perhaps to the default = 3?

Thanks, Roger



On 10/9/19 1:47 PM, naoto.s...@oracle.com wrote:
I revised the fix, incorporating the clarification of the value zero 
as the grouping size, which has separate JIRA issue and CSR as follows:


https://bugs.openjdk.java.net/browse/JDK-8231984
https://bugs.openjdk.java.net/browse/JDK-8232012

The merged changeset is as follows:

http://cr.openjdk.java.net/~naoto/8212749.8231984/webrev.00/

Please review.

Naoto

On 10/8/19 8:59 AM, naoto.s...@oracle.com wrote:

Hi Roger,

Thank you for the review. In fact, Joe commented about the validity 
of zero on the CSR, so I will need to modify the method description 
such as:


diff -r 9576895d0f9a 
src/java.base/share/classes/java/text/DecimalFormat.java

--- a/src/java.base/share/classes/java/text/DecimalFormat.java
+++ b/src/java.base/share/classes/java/text/DecimalFormat.java
@@ -2770,10 +2770,13 @@
  /**
   * Set the grouping size. Grouping size is the number of digits 
between
   * grouping separators in the integer portion of a number.  For 
example,

- * in the number "123,456.78", the grouping size is 3.
- * 
+ * in the number "123,456.78", the grouping size is 3. Grouping 
size of
+ * zero designates that grouping is not used, which provides the 
same

+ * formatting as if calling {@link #setGroupingUsed(boolean)
+ * setGroupingUsed(false)}.
+ * 
   * The value passed in is converted to a byte, which may lose 
information.

- * Invalid value, i.e., negative or greater than
+ * Values that are negative or greater than
   * {@link java.lang.Byte#MAX_VALUE Byte.MAX_VALUE}, will throw an
   * {@code IllegalArgumentException}.
   *

I will file a follow-on CSR and merge changesets.

Naoto

On 10/8/19 6:59 AM, Roger Riggs wrote:

Hi Naoto,

DecimalFormat.java: 2776:  "Invalid value, i.e.," -> "Values that are".

Otherwise looks fine. No need for another webrev.

Thanks, Roger




On 10/4/19 6:54 PM, naoto.s...@oracle.com wrote:

Hello,

Please review the fix to the following issue:

https://bugs.openjdk.java.net/browse/JDK-8212749

The proposed CSR and changeset are located at:

https://bugs.openjdk.java.net/browse/JDK-8231851
https://cr.openjdk.java.net/~naoto/8212749/webrev.00/

Naoto

RFR: JDK-8212780: JEP 343: Packaging Tool Implementation

[Andy Herrick]

Please review  changes for [1] which is the implementation bug for JEP-343.

The webrev at [2] is the total cumulative webrev of changes for the 
jpackage tool, currently in the JDK-8200758-branch branch of the open 
sandbox repository.


The webrev at [3] shows the changes since EA-06 (Build 13-jpackage+1-49 
) up to the current point.


The latest EA (Build 14-jpackage+1-49 ) is posted at [4].

Please send feedback to core-libs-dev@openjdk.java.net


[1] https://bugs.openjdk.java.net/browse/JDK-8212780

[2] http://cr.openjdk.java.net/~herrick/jpackage/webrev.08/

[3] http://cr.openjdk.java.net/~herrick/8212780/webrev.06-08/

[4] http://jdk.java.net/jpackage/


/Andy

Re: Apache Phoenix with OpenJDK

[Bernd Eckenfels]

Hello,

If you can’t find the needed information on the Project Website it is probably 
best to contact the project on the user mailing list.

https://phoenix.apache.org/mailing_list.html

You might want to give more information like which component is failing, what 
are the exact error logs and what are the versions of the associated products 
(like HBase).

Gruss
Bernd


--
http://bernd.eckenfels.net


Von: core-libs-dev  im Auftrag von 
Rachu Vmpp 
Gesendet: Freitag, Oktober 11, 2019 8:15 PM
An: core-libs-dev@openjdk.java.net
Betreff: Apache Phoenix with OpenJDK

Dear Fellas,

Is openJDK 11 compatible with Apache Phoenix? I could see some
PhoenixDialect unresolvable name error.

Can somebody help?

Apache Phoenix with OpenJDK

[Rachu Vmpp]

Dear Fellas,

Is openJDK 11 compatible with Apache Phoenix?  I could  see some
PhoenixDialect unresolvable name error.

Can somebody help?

Re: [8u] PING: RFR: 8213734: SAXParser.parse(File, ..) does not close resources when Exception occurs.

[Mario Torre]

Hi Severin,

The patch looks good to me.

Cheers,
Mario

On Fri, Sep 27, 2019 at 10:38 AM Severin Gehwolf  wrote:
>
> Hi!
>
> I'd appreciate a review of this one. Thanks in advance! See below.
>
> On Tue, 2019-09-03 at 17:50 +0200, Severin Gehwolf wrote:
> > On Mon, 2019-08-26 at 17:54 +0200, Severin Gehwolf wrote:
> > > Hi,
> > >
> > > Please review this 8u backport. The OpenJDK 11u patch does not apply
> > > cleanly after path-reshuffeling due to a) test and code for jaxp are
> > > split in JDK 8 b) Some rejects in comments - copyright and last
> > > modified date. I've omitted rejected comments. I can manually add them
> > > if that's preferred. See below.
> > >
> > > Bug: https://bugs.openjdk.java.net/browse/JDK-8213734
> > > webrevs:
> > >   jdk:  
> > > http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8213734/jdk8/jdk/01/webrev/
> > >   jaxp: 
> > > http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8213734/jdk8/jaxp/01/webrev/
> > >
> > > Testing: tier1 on Linux x86_64. javax/xml/jaxp tests. New test on
> > >  Windows without the fix fails and passes with it.
> > >
> > > Thanks to Simon Tooke who helped testing this patch on Windows.
> > >
> > > Rejects:
> > > $ cat 
> > > src/com/sun/org/apache/xerces/internal/impl/XMLEntityManager.java.rej
> > > --- XMLEntityManager.java
> > > +++ XMLEntityManager.java
> > > @@ -1,5 +1,5 @@
> > >  /*
> > > - * Copyright (c) 2009, 2017, Oracle and/or its affiliates. All rights 
> > > reserved.
> > > + * Copyright (c) 2009, 2018, Oracle and/or its affiliates. All rights 
> > > reserved.
> > >   */
> > >  /*
> > >   * Licensed to the Apache Software Foundation (ASF) under one or more
> > > @@ -89,7 +89,7 @@
> > >   * @author K.Venugopal SUN Microsystems
> > >   * @author Neeraj Bajaj SUN Microsystems
> > >   * @author Sunitha Reddy SUN Microsystems
> > > - * @LastModified: Oct 2017
> > > + * @LastModified: Nov 2018
> > >   */
> > >  public class XMLEntityManager implements XMLComponent, XMLEntityResolver 
> > > {
> > >
> >
> > Gentle reminder.
>
> Anyone? It's been a month :(
>
> FWIW, I've updated XMLEntityManager to include the copyright update
> upper bound to 2018. The @LastModified lines don't exist in JDK 8u, so
> I've omitted that. Latest webrevs:
>
> webrevs:
>jdk:  
> http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8213734/jdk8/jdk/01/webrev/
>jaxp: 
> http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8213734/jdk8/jaxp/02/webrev/
>
> Thanks,
> Severin
>


-- 
Mario Torre
Associate Manager, Software Engineering
Red Hat GmbH 
9704 A60C B4BE A8B8 0F30  9205 5D7E 4952 3F65 7898

Re: [14] RFR 8212749: DecimalFormat.setGroupingSize(int) allows setting negative grouping size, 8231984: Clarify semantics of DecimalFormat.getGroupingSize(0)

[Roger Riggs]

Hi Naoto,

The javadoc/spec comments look fine.

Code comments at DecimalFormat:4035 give some latitute for the value to be
out of range and since getGroupingSize returns the groupingSize byte
it would be cleaner if the value was always in the valid range
regardless of the isGroupingUsed boolean.

Can there be code in the readObject method to correct out of range
values, perhaps to the default = 3?

Thanks, Roger



On 10/9/19 1:47 PM, naoto.s...@oracle.com wrote:
I revised the fix, incorporating the clarification of the value zero 
as the grouping size, which has separate JIRA issue and CSR as follows:


https://bugs.openjdk.java.net/browse/JDK-8231984
https://bugs.openjdk.java.net/browse/JDK-8232012

The merged changeset is as follows:

http://cr.openjdk.java.net/~naoto/8212749.8231984/webrev.00/

Please review.

Naoto

On 10/8/19 8:59 AM, naoto.s...@oracle.com wrote:

Hi Roger,

Thank you for the review. In fact, Joe commented about the validity 
of zero on the CSR, so I will need to modify the method description 
such as:


diff -r 9576895d0f9a 
src/java.base/share/classes/java/text/DecimalFormat.java

--- a/src/java.base/share/classes/java/text/DecimalFormat.java
+++ b/src/java.base/share/classes/java/text/DecimalFormat.java
@@ -2770,10 +2770,13 @@
  /**
   * Set the grouping size. Grouping size is the number of digits 
between
   * grouping separators in the integer portion of a number.  For 
example,

- * in the number "123,456.78", the grouping size is 3.
- * 
+ * in the number "123,456.78", the grouping size is 3. Grouping 
size of
+ * zero designates that grouping is not used, which provides the 
same

+ * formatting as if calling {@link #setGroupingUsed(boolean)
+ * setGroupingUsed(false)}.
+ * 
   * The value passed in is converted to a byte, which may lose 
information.

- * Invalid value, i.e., negative or greater than
+ * Values that are negative or greater than
   * {@link java.lang.Byte#MAX_VALUE Byte.MAX_VALUE}, will throw an
   * {@code IllegalArgumentException}.
   *

I will file a follow-on CSR and merge changesets.

Naoto

On 10/8/19 6:59 AM, Roger Riggs wrote:

Hi Naoto,

DecimalFormat.java: 2776:  "Invalid value, i.e.," -> "Values that are".

Otherwise looks fine. No need for another webrev.

Thanks, Roger




On 10/4/19 6:54 PM, naoto.s...@oracle.com wrote:

Hello,

Please review the fix to the following issue:

https://bugs.openjdk.java.net/browse/JDK-8212749

The proposed CSR and changeset are located at:

https://bugs.openjdk.java.net/browse/JDK-8231851
https://cr.openjdk.java.net/~naoto/8212749/webrev.00/

Naoto

Re: [14] RFR: 8225435: Upgrade IANA Language Subtag Registry to the latest for JDK14

[Roger Riggs]

Hi Naoto,

Looks fine,

Roger


On 10/9/19 6:45 PM, naoto.s...@oracle.com wrote:

Hi,

Please review the fix to the following issue:

https://bugs.openjdk.java.net/browse/JDK-8225435

The proposed changeset is located at:

https://cr.openjdk.java.net/~naoto/8225435/webrev.00/

The change is to upgrade the data file to the latest (dated 
2019-09-16). A relevant test case is modified accordingly.


Naoto

Re: RFR 8231427 (test) Warning cleanup in tests of java.io.Serializable

[Joe Darcy]

Looks good Roger; thanks,

-Joe

On 10/11/2019 8:12 AM, Roger Riggs wrote:

Hi Joe,

Updated with comments added for the reasons of the 
@SuppressWarnings("serial").


http://cr.openjdk.java.net/~rriggs/webrev-ser-test-cleanup-8231427-3/

Thanks, Roger


On 9/27/19 2:45 PM, Joe Darcy wrote:

Hi Roger,

Generally looks fine. Adding some comments about why the 
@SuppressWarnings annotations were being applied might help future 
readers of the code.


Cheers,

-Joe

On 9/27/2019 10:02 AM, Roger Riggs wrote:
Please review cleanup of tests in java/io/Serializable. There are 
warnings for mis-use of
serialization methods and fields; some are intentional as they are 
the focus of the tests.
The test code had quite a few warnings about raw types, deprecated 
methods, and

missing serialVersionUIDs, and deprecated methods, etc.

Issue:
https://bugs.openjdk.java.net/browse/JDK-8231427

Webrev:
http://cr.openjdk.java.net/~rriggs/webrev-ser-test-cleanup-8231427-2/

Thanks, Roger

Re: RFR 8231427 (test) Warning cleanup in tests of java.io.Serializable

[Lance Andersen]

Updates look good to me :-)

> On Oct 11, 2019, at 11:12 AM, Roger Riggs  wrote:
> 
> Hi Joe,
> 
> Updated with comments added for the reasons of the 
> @SuppressWarnings("serial").
> 
> http://cr.openjdk.java.net/~rriggs/webrev-ser-test-cleanup-8231427-3/
> 
> Thanks, Roger
> 
> 
> On 9/27/19 2:45 PM, Joe Darcy wrote:
>> Hi Roger,
>> 
>> Generally looks fine. Adding some comments about why the @SuppressWarnings 
>> annotations were being applied might help future readers of the code.
>> 
>> Cheers,
>> 
>> -Joe
>> 
>> On 9/27/2019 10:02 AM, Roger Riggs wrote:
>>> Please review cleanup of tests in java/io/Serializable. There are warnings 
>>> for mis-use of
>>> serialization methods and fields; some are intentional as they are the 
>>> focus of the tests.
>>> The test code had quite a few warnings about raw types, deprecated methods, 
>>> and
>>> missing serialVersionUIDs, and deprecated methods, etc.
>>> 
>>> Issue:
>>> https://bugs.openjdk.java.net/browse/JDK-8231427
>>> 
>>> Webrev:
>>> http://cr.openjdk.java.net/~rriggs/webrev-ser-test-cleanup-8231427-2/
>>> 
>>> Thanks, Roger
>>> 
> 

 
  

 Lance Andersen| 
Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com 

Re: RFR [14] 8231297: java/lang/ProcessBuilder/Basic.java test fails intermittently

[Lance Andersen]

+1
> On Oct 11, 2019, at 10:38 AM, Roger Riggs  wrote:
> 
> Please review this addition to a test to add diagnostic output.
> There have been intermittent failures of ProcessBuilder/Basic.java that 
> indicate
> some output is being received from the child.  There should be none.
> The additional output will be added to the log file along with the failure.
> 
> Jira Issue for the diagnostics:
>  https://bugs.openjdk.java.net/browse/JDK-8232135
> 
> Base issue:
> 8231297 java/lang/ProcessBuilder/Basic.java test fails intermittently [1]
> 
> Webrev:
> http://cr.openjdk.java.net/~rriggs/webrev-basic-intermittent-8231297-1/
> 
> [1] https://bugs.openjdk.java.net/browse/JDK-8231297
> 
> Thanks, Roger

 
  

 Lance Andersen| 
Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com 

Re: RFR: 8231717: Improve performance of EBCDIC charset decoding for COMPACT_STRINGS

[Roger Riggs]

Pushed.

https://hg.openjdk.java.net/jdk/jdk/rev/3968bf3673c5

Many thanks Andrew!

On 10/10/19 8:32 AM, Andrew Leonard wrote:

Hi Roger,
I've updated the webrev here : 
http://cr.openjdk.java.net/~aleonard/8231717/webrev.02/

Thanks
Andrew

Re: RFR 8231427 (test) Warning cleanup in tests of java.io.Serializable

[Roger Riggs]

Hi Joe,

Updated with comments added for the reasons of the 
@SuppressWarnings("serial").


http://cr.openjdk.java.net/~rriggs/webrev-ser-test-cleanup-8231427-3/

Thanks, Roger


On 9/27/19 2:45 PM, Joe Darcy wrote:

Hi Roger,

Generally looks fine. Adding some comments about why the 
@SuppressWarnings annotations were being applied might help future 
readers of the code.


Cheers,

-Joe

On 9/27/2019 10:02 AM, Roger Riggs wrote:
Please review cleanup of tests in java/io/Serializable. There are 
warnings for mis-use of
serialization methods and fields; some are intentional as they are 
the focus of the tests.
The test code had quite a few warnings about raw types, deprecated 
methods, and

missing serialVersionUIDs, and deprecated methods, etc.

Issue:
https://bugs.openjdk.java.net/browse/JDK-8231427

Webrev:
http://cr.openjdk.java.net/~rriggs/webrev-ser-test-cleanup-8231427-2/

Thanks, Roger

Re: [8u] PING: RFR: 8213734: SAXParser.parse(File, ..) does not close resources when Exception occurs.

[Severin Gehwolf]

On Fri, 2019-10-11 at 16:47 +0200, Mario Torre wrote:
> Hi Severin,
> 
> The patch looks good to me.

Thanks for the review, Mario!

Cheers,
Severin

> On Fri, Sep 27, 2019 at 10:38 AM Severin Gehwolf  wrote:
> > Hi!
> > 
> > I'd appreciate a review of this one. Thanks in advance! See below.
> > 
> > On Tue, 2019-09-03 at 17:50 +0200, Severin Gehwolf wrote:
> > > On Mon, 2019-08-26 at 17:54 +0200, Severin Gehwolf wrote:
> > > > Hi,
> > > > 
> > > > Please review this 8u backport. The OpenJDK 11u patch does not apply
> > > > cleanly after path-reshuffeling due to a) test and code for jaxp are
> > > > split in JDK 8 b) Some rejects in comments - copyright and last
> > > > modified date. I've omitted rejected comments. I can manually add them
> > > > if that's preferred. See below.
> > > > 
> > > > Bug: https://bugs.openjdk.java.net/browse/JDK-8213734
> > > > webrevs:
> > > >   jdk:  
> > > > http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8213734/jdk8/jdk/01/webrev/
> > > >   jaxp: 
> > > > http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8213734/jdk8/jaxp/01/webrev/
> > > > 
> > > > Testing: tier1 on Linux x86_64. javax/xml/jaxp tests. New test on
> > > >  Windows without the fix fails and passes with it.
> > > > 
> > > > Thanks to Simon Tooke who helped testing this patch on Windows.
> > > > 
> > > > Rejects:
> > > > $ cat 
> > > > src/com/sun/org/apache/xerces/internal/impl/XMLEntityManager.java.rej
> > > > --- XMLEntityManager.java
> > > > +++ XMLEntityManager.java
> > > > @@ -1,5 +1,5 @@
> > > >  /*
> > > > - * Copyright (c) 2009, 2017, Oracle and/or its affiliates. All rights 
> > > > reserved.
> > > > + * Copyright (c) 2009, 2018, Oracle and/or its affiliates. All rights 
> > > > reserved.
> > > >   */
> > > >  /*
> > > >   * Licensed to the Apache Software Foundation (ASF) under one or more
> > > > @@ -89,7 +89,7 @@
> > > >   * @author K.Venugopal SUN Microsystems
> > > >   * @author Neeraj Bajaj SUN Microsystems
> > > >   * @author Sunitha Reddy SUN Microsystems
> > > > - * @LastModified: Oct 2017
> > > > + * @LastModified: Nov 2018
> > > >   */
> > > >  public class XMLEntityManager implements XMLComponent, 
> > > > XMLEntityResolver {
> > > > 
> > > 
> > > Gentle reminder.
> > 
> > Anyone? It's been a month :(
> > 
> > FWIW, I've updated XMLEntityManager to include the copyright update
> > upper bound to 2018. The @LastModified lines don't exist in JDK 8u, so
> > I've omitted that. Latest webrevs:
> > 
> > webrevs:
> >jdk:  
> > http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8213734/jdk8/jdk/01/webrev/
> >jaxp: 
> > http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8213734/jdk8/jaxp/02/webrev/
> > 
> > Thanks,
> > Severin
> > 
> 
> 

RFR [14] 8231297: java/lang/ProcessBuilder/Basic.java test fails intermittently

[Roger Riggs]

Please review this addition to a test to add diagnostic output.
There have been intermittent failures of ProcessBuilder/Basic.java that 
indicate

some output is being received from the child.  There should be none.
The additional output will be added to the log file along with the failure.

Jira Issue for the diagnostics:
 https://bugs.openjdk.java.net/browse/JDK-8232135

Base issue:
8231297 java/lang/ProcessBuilder/Basic.java test fails intermittently [1]

Webrev:
http://cr.openjdk.java.net/~rriggs/webrev-basic-intermittent-8231297-1/

[1] https://bugs.openjdk.java.net/browse/JDK-8231297

Thanks, Roger

RE: RFR: JDK-8229871: Improve performance of Method.copy() and leafCopy()

[Kazunori Ogata]

Hi Peter,

Thank you for the comment and suggestion of the fix.

I tried to pick up your change w.r.t. methodAccessor:
https://cr.openjdk.java.net/~ogatak/8229871/webrev.04/


Regarding micro benchmark, my original motivation of this change is to 
improve performance of Class.getMethods(), which calls Method.copy() for 
each declared method to create a copy of Method[].

I measured my simple microbench: 
https://cr.openjdk.java.net/~ogatak/8229871/GetMethodsBench.java

Base code: Elapsed time = 4808 ms
webrev.01: Elapsed time = 4536 ms  (+   6%)
webrev.02: Elapsed time = 2331 ms  (+106%)
webrev.04: Elapsed time = 3746 ms  (+ 28%)

I'll measure larger benchmark and try to think if we can reduce the 
overhead of memory barrier.


Regards,
Ogata


Peter Levart  wrote on 2019/10/09 16:44:13:

> From: Peter Levart 
> To: Kazunori Ogata , core-libs-dev@openjdk.java.net
> Date: 2019/10/09 16:44
> Subject: [EXTERNAL] Re: RFR: JDK-8229871: Improve performance of 
> Method.copy() and leafCopy()
> 
> Hi Ogata,
> 
> May I just add that volatile field ensured that MethodAccessor object 
was 
> correctly published. DelegatingMethodAccessortImpl is not safe to be 
> published via data race because it contains plain `delegate` field 
> initialized in the constructor. In addition, the object that is first 
> assigned to that field is NativeMethodAccessorImpl which has plain 
> `parent` field. You can get NPE when invoking the Method.invoke from 
> multuiple threads if Method.methodAccessor is not volatile.
> 
> In addition, It would be nice to have two microbenchmarks exercising:
> a) Method copy performance
> b) Method invocation performance
> 
> Regards, Peter
> 
> P.S. When exploring the possibility of an alternative MethodAccessor 
> implementation (using MethodHandle(s)):
> 
> 
http://cr.openjdk.java.net/~plevart/jdk-dev/6824466_MHReflectionAccessors/
> webrev.00.2/
> 
> ...I found out that it was possible to re-arrange the play between 
> DelegatingMethodAccessorImpl, NativeMethodAccessorImpl and generated 
> MethodAccessor in such a way that the DelegatingMethodAccessortImpl 
> becomes safe to be published via data race. This allowed for 
> Method.methodAccessor field to become plain field. In addition this 
field 
> can be made @Stable which further optimizes access to MethodAccessor 
> instance when Method instance can be constant-folded, which showed in 
> special microbenchmarks.
> 
> So perhaps you could try to use above implementation (just changes to 
> DelegatingMethodAccessorImpl, NativeMethodAccessorImpl and part of 
> Reflection factory but without MH* stuff) and measure it against current 

> and your implementation (which as shown above has a data-race flaw).

> On 10/8/19 12:23 PM, Kazunori Ogata wrote:
> Hi all,
> 
> I posted two changes and got reply that performance evaluation is 
needed. 
> I found that making Method.methodAccessor non-volatile (webrev.02) is 
> better than avoid copying methodAccessor value when it is null 
> (webrev.01), as shown below.
> 
> So I'd like to ask review of the former change.  I updated weberv using 
> the latest code base (though there was no difference from webrev.02):
> 
> Webrev: http://cr.openjdk.java.net/~ogatak/8229871/webrev.03/
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8229871
> 
> 
> For this performance evaluation, I calculated 75 percentile of 9 runs of 

> SPECjbb2015 and 60 percentile of 50 runs of DaCapo to omit outliers.  I 
> bound a JVM to a NUMA node and set the number of GC threads to the same 
as 
> the number of physical cores.  These tuning reduced run-to-run 
fluctuation 
> on POWER (as usual...).
> 
> SPECjbb2015:
>   webrev.02:  critical jOPS +1.6%, max jOPS +0.2%
>   webrev.01:  critical jOPS +0.4%, max jOPS +0.2%
> 
> 
> For DaCapo, some benchmark still improved performance and some degraded, 

> but the geometric mean of all benchmarks were small:
>   weberv.02: +0.3%
>   weberv.01: +0.2%
> 
> The difference of improvement/degradation between the two changes in 
each 
> benchmark were less than 0.8%.
> 
> The range of improvement/degradation in each benchmark were:
>   webrev.02: between +2.4% and -1.0%
>   webrev.01: between +1.6% and -1.8%
> 
> 
> So I think webrev.02 (i.e., making methodAccessor non-volatile) is a 
good 
> change, since it improved SPECjbb critical jOPS by 1.6%.
> 
> 
> Regards,
> Ogata
> 
> 
> Kazunori Ogata/Japan/IBM wrote on 2019/08/27 15:41:39:
> 

> From: Kazunori Ogata/Japan/IBM
> To: Mandy Chung 
> Cc: core-libs-dev@openjdk.java.net
> Date: 2019/08/27 15:41
> Subject: Re: RFR: JDK-8229871: Improve performance of Method.copy() and 

> leafCopy()

> 
> Hi Mandy,
> 
> Let me post interim results of the performance evaluation, though I'm 
> still measuring benchmarks and analyzing them.
> 
> For SPECjbb2015, skipping storing null (webrev.01) was faster than 

> making 

> methodAccessor non-volatile (webrev.02).  The improvements of each of 

> the 

> patches in maxJOPS/criticalJOPS were 2.6%/3.9% and 

RE: RFR: JDK-8229871: Improve performance of Method.copy() and leafCopy()

[Kazunori Ogata]

Hi Mandy,

Thank you for review.

I'll keep the constructor package-private, as the change will be 
non-trivial when I fix the code based on the Peter's comment.

Regards,
Ogata

Mandy Chung  wrote on 2019/10/09 04:24:35:

> From: Mandy Chung 
> To: Claes Redestad , Kazunori Ogata 

> Cc: core-libs-dev@openjdk.java.net
> Date: 2019/10/09 04:24
> Subject: [EXTERNAL] Re: RFR: JDK-8229871: Improve performance of 
> Method.copy() and leafCopy()
> 
> 
> 
> On 10/8/19 6:31 AM, Claes Redestad wrote:
> > Hi,
> >
> > webrev.02 looks good to me.
> >
> 
> webrev.02 looks fine to me as well.
> 
> > I think the performance results makes sense since avoiding a volatile
> > store (and the potentially expensive store barriers this involves)
> > should be the main benefit. Adding a branch to avoid storing null 
would
> > help partially, but not hot Methods.
> >
> > Pre-existing issue, but it's somewhat weird that we have two 
assignments
> > outside the package-private constructor: adding root and 
methodAccessor
> > to the constructor would make more immediate sense to me, since we do
> > the same thing at the only two callsites:
> >
> > Method res = new Method(clazz, name, parameterTypes, 
returnType,
> > exceptionTypes, modifiers, slot, signature,
> > annotations, parameterAnnotations, annotationDefault);
> > res.root = root;
> > res.methodAccessor = methodAccessor;
> >
> > ->
> >
> > Method res = new Method(clazz, name, parameterTypes, 
returnType,
> > exceptionTypes, modifiers, slot, signature,
> > annotations, parameterAnnotations, annotationDefault,
> > root, methodAccessor);
> >
> > This package-private constructor could also be made private. My guess 
is
> > there was some time when this was used from outside Method and 
changing
> > it was deemed unsavory..
> 
> The parameters of the Method constructor are the content from a 
> ClassFile to represent a method whereas root and methodAccessor fields 
> are not part of it. The current implementation allocates a Method 
> instance and sets the fields individually instead of calling the 
> constructor probably due to bootstrapping.
> 
> I suggest to keep it as it is.  OTOH making the constructor private is 
fine.
> 
> Mandy
> 

Re: RFR: 8231584: Deadlock with ClassLoader.findLibrary and System.loadLibrary call

[Anton Kozlov]

On 11.10.2019 00:28, Mandy Chung wrote:
> Since the method throws Exception, this try-catch block is not needed.
> 
> The start year of the copyright in the new test files should be 2019.

Thanks! I preserved Amazon's copyright year as 2018, as I derived the file from 
JDK-8194653 initially ([1])

[1]: 
https://bugs.openjdk.java.net/secure/attachment/79869/JDK-8194653-Deadlock-involving-FileSystems.patch

> Otherwise, looks good.
> Is there anyone sponsoring this patch for you?  If not, I can do that.

Not yet. It would be great if you sponsor the patch.

Updated webrev: http://cr.openjdk.java.net/~akozlov/8231584/webrev.05/

Thanks,
Anton

Jar manifest and EOF

[Philipp Kunz]

Hi,

The Jar File Specification [1] states that,

> If the last character of the file is an EOF character (code 26), the
EOF is treated as whitespace. Two newlines are appended (one for
editors that don't put a newline at the end of the last line, and one
so that the grammar doesn't have to special-case the last entry, which
may not have a blank line after it).

But I can't see that this is actually the case. See the attached simple
test that indicates that this statement is not implemented.

See also potentially remotely related bugs [2] and [3] which both refer
in my opinion to EOF as the end of the byte stream as opposed to the
EOF character 26.

After nobody has fixed this point inside of many years I tend to assume
that it is not at all necessary or useful. However, I cannot understand
the intention behind it or how that statement got there in the first
place and hence might not be aware of the full context.

Considering the activity about the last line of a manifest not
terminated by a line break resulting in that line ignored unexpectedly 
(see [3] through [14]) I doubt that it is promising to attempt to
implement the EOF according to the specs.

Instead I'd rather like to propose to update the specification by
removing the quoted statement.

I'm not sure if the specification [1] has changed since 13 or where the
most up to date one is.

Regards,
Philipp



[1] 
https://docs.oracle.com/en/java/javase/13/docs/specs/jar/jar.html#notes-on-manifest-and-signature-files

EOF-related:
[2] https://bugs.openjdk.java.net/browse/JDK-7148584
[3] https://bugs.openjdk.java.net/browse/JDK-4639129

Last line break required or missing related:
[4] https://bugs.openjdk.java.net/browse/JDK-4274235
[5] https://bugs.openjdk.java.net/browse/JDK-4894998
[6] https://bugs.openjdk.java.net/browse/JDK-4333854
[7] https://bugs.openjdk.java.net/browse/JDK-6594305
[8] 
http://mail.openjdk.java.net/pipermail/core-libs-dev/2019-February/058774.html
[9] https://bugs.openjdk.java.net/browse/JDK-4489716
[10] https://bugs.openjdk.java.net/browse/JDK-4639129
[11] https://bugs.openjdk.java.net/browse/JDK-4625822
[12] 
https://stackoverflow.com/questions/21859417/header-must-be-terminated-by-a-line-break-eclipse-manifest-mf
[13] https://bugs.eclipse.org/bugs/show_bug.cgi?id=377249
[14] https://bugs.java.com/bugdatabase/view_bug.do?bug_id=4274235

diff -r d6058bd73982 test/jdk/java/util/jar/Manifest/EOF.java
--- /dev/null	Thu Jan 01 00:00:00 1970 +
+++ b/test/jdk/java/util/jar/Manifest/EOF.java	Fri Oct 11 08:10:37 2019 +0200
@@ -0,0 +1,48 @@
+/*
+ * Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+
+import java.io.ByteArrayInputStream;
+import java.util.jar.Attributes.Name;
+import java.util.jar.Manifest;
+
+import org.testng.annotations.Test;
+import static org.testng.Assert.*;
+
+/**
+ * @test
+ * @run testng EOF
+ * @summary Tests parsing a manifest with an EOF character (code 26) at its end.
+ */
+public class EOF {
+
+@Test
+public void test() throws Exception {
+byte[] mfBytes = (Name.MANIFEST_VERSION + ": 1.0\r\n"
++ "Key: Value\u001a").getBytes(UTF_8);
+Manifest mf = new Manifest(new ByteArrayInputStream(mfBytes));
+assertEquals(mf.getMainAttributes().getValue("Key"), "Value");
+}
+
+}