RFR[9]: 8133719 java.lang.InternalError in java.lang.invoke.MethodHandleImpl$BindCaller.bindCaller

[shilpi.rast...@oracle.com]

Hi All,

Please review fix for

https://bugs.openjdk.java.net/browse/JDK-8133719
http://cr.openjdk.java.net/~srastogi/8133719/webrev.01/


Thanks,
Shilpi

RE: RFR JDK-8170192 [JAXP] [TESTBUG] test/javax/xml/jaxp/libs/jaxp/library/JAXPPolicyManager.java should grant permissions to jtreg, javatest, and testng jars

[Frank Yuan]

Hi Jon

 

Thank you for your advice!

 

Please check the update http://cr.openjdk.java.net/~fyuan/8170192/webrev.01/ , 
which contains jcommander.jar and removes the extra
blank lines following Christoph's suggestion.

 

Frank

 

From: Jonathan Gibbons [mailto:jonathan.gibb...@oracle.com] 
Sent: Thursday, November 24, 2016 4:26 AM
Subject: Re: RFR JDK-8170192 [JAXP] [TESTBUG] 
test/javax/xml/jaxp/libs/jaxp/library/JAXPPolicyManager.java should grant 
permissions
to jtreg, javatest, and testng jars

 

Frank,

More recent builds of testng.jar, such as the builds available on Maven,  have 
separated out the jcommander component so that two
jar files are required: testng.jar and jcommander.jar.

You should consider taking jcommander.jar into account.  This will be more 
important/noticeable to folk outside Oracle who build
their own copy of jtreg to use.

-- Jon

On 11/22/2016 08:41 PM, Frank Yuan wrote:

Hi All

 

Would you like to review http://cr.openjdk.java.net/~fyuan/8170192/webrev.00/
 ?

Bug: https://bugs.openjdk.java.net/browse/JDK-8170192

 

This patch is fully same as Daniel provided except a few lines of additional 
cleaning, thanks to Daniel for providing the patch!

 

Thanks

Frank

 

 

Re: RFR 8170155: StringBuffer and StringBuilder stream methods are not late-binding

[Xueming Shen]

On 11/23/2016 02:39 PM, Paul Sandoz wrote:

Hi,

Please review:

   
http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8170155-string-buffer-builder-late-binding/webrev/

This patch:

1) updates the StringBuilder/StringBuffer.chars()/codePoints() so they are late 
binding.

2) refactors the spliterator late binding and fail fast tests, separating them 
out, and to the former additional tests are added for CharSequence 
implementations and BitSet.

The code in AbstractStringBuilder has the following bounds check call to 
checkOffset:

1558 return StreamSupport.intStream(
1559 () ->  {
1560 byte[] val = this.value; int count = this.count;
1561 checkOffset(count, val.length>>  coder);
1562 return coder == LATIN1
1563? new StringLatin1.CharsSpliterator(val, 0, 
count, 0)
1564: new StringUTF16.CharsSpliterator(val, 0, 
count, 0);
1565 },
1566 Spliterator.ORDERED | Spliterator.SIZED | 
Spliterator.SUBSIZED,
1567 false);

(Separately checkOffset could be replaced with the internal 
Preconditions.checkIndex.)

On initial inspection that should be a no-op, but talking to Sherman we believe 
it is required for UTF-16 access, since the intrinsics do not perform bounds 
checks, so they need to be guarded (for conformance purposes if a no-op).


One of the purposes of having StringUTF16.putChar() (other than the charAt())
is to avoid the expensive boundary check on each/every char access. I'm 
forwarding
the email to Tobias to make sure it's still the case on hotspot side.

I'm not sure if it is still desired to do the same boundary check in case of 
LATIN1
for the benefit of consistency.  Assume there might be concurrent 
access/operation
between val = this.value and count = this.count; (for StringBuilder) for 
example,
the this.value got doubled and the this.count got increased. One will end up 
with
StringIndexOutOfBoundsException() from checkOffset, but the other will be ioobe
from vm?

Sherman



If so i propose the following to make this clearer:

return StreamSupport.intStream(
 () ->  {
 byte[] val = this.value; int count = this.count;
 if (coder == LATIN1) {
 return new StringLatin1.CharsSpliterator(val, 0, count, 0);
 } else {
 // Perform an explicit bounds check since HotSpot
 // intrinsics to access UTF-16 characters in the byte[]
 // array will not perform bounds checks
 checkOffset(count, val.length>>  coder);
 return new StringUTF16.CharsSpliterator(val, 0, count, 0);
 }
 },
 Spliterator.ORDERED | Spliterator.SIZED | Spliterator.SUBSIZED,
 false);

Paul.

RFR 8170155: StringBuffer and StringBuilder stream methods are not late-binding

[Paul Sandoz]

Hi,

Please review:

  
http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8170155-string-buffer-builder-late-binding/webrev/

This patch:

1) updates the StringBuilder/StringBuffer.chars()/codePoints() so they are late 
binding.

2) refactors the spliterator late binding and fail fast tests, separating them 
out, and to the former additional tests are added for CharSequence 
implementations and BitSet.

The code in AbstractStringBuilder has the following bounds check call to 
checkOffset:

1558 return StreamSupport.intStream(
1559 () -> {
1560 byte[] val = this.value; int count = this.count;
1561 checkOffset(count, val.length >> coder);
1562 return coder == LATIN1
1563? new StringLatin1.CharsSpliterator(val, 0, 
count, 0)
1564: new StringUTF16.CharsSpliterator(val, 0, 
count, 0);
1565 },
1566 Spliterator.ORDERED | Spliterator.SIZED | 
Spliterator.SUBSIZED,
1567 false);

(Separately checkOffset could be replaced with the internal 
Preconditions.checkIndex.)

On initial inspection that should be a no-op, but talking to Sherman we believe 
it is required for UTF-16 access, since the intrinsics do not perform bounds 
checks, so they need to be guarded (for conformance purposes if a no-op).

If so i propose the following to make this clearer:

return StreamSupport.intStream(
() -> {
byte[] val = this.value; int count = this.count;
if (coder == LATIN1) {
return new StringLatin1.CharsSpliterator(val, 0, count, 0);
} else {
// Perform an explicit bounds check since HotSpot
// intrinsics to access UTF-16 characters in the byte[]
// array will not perform bounds checks
checkOffset(count, val.length >> coder);
return new StringUTF16.CharsSpliterator(val, 0, count, 0);
}
},
Spliterator.ORDERED | Spliterator.SIZED | Spliterator.SUBSIZED,
false);

Paul.

Re: RFR: JDK-8167648: java.io.PrintWriter should have PrintWriter((String|File), Charset) constructors

[Patrick Reinhart]

Hi Stephen,

I changed the webrev accordingly:

http://cr.openjdk.java.net/~reinhapa/reviews/8167648/webrev.00

-Patrick

> Am 23.11.2016 um 14:45 schrieb Stephen Colebourne :
> 
> Returning the writer was my intention. I also intended it to return a
> new instance, to avoid changing the variable from final to non-final.
> Stephen
> 

Re: Issues running JAXP jtreg tests ("java.lang.RuntimePermission" "accessDeclaredMembers")

[Jonathan Gibbons]

On 11/22/2016 11:50 AM, Langer, Christoph wrote:

  I agree that it would be nicer if jtreg would leave the jtreg lib path as 
java property to be able to elevate all of its contents. But the current 
proposal with a set of TEST_JARS of jtreg, javatest and testng is probably 
sufficient for jaxp testing.


https://bugs.openjdk.java.net/browse/CODETOOLS-7901844

-- Jon

Re: RFR JDK-8170192 [JAXP] [TESTBUG] test/javax/xml/jaxp/libs/jaxp/library/JAXPPolicyManager.java should grant permissions to jtreg, javatest, and testng jars

[Jonathan Gibbons]

Frank,

More recent builds of testng.jar, such as the builds available on 
Maven,  have separated out the jcommander component so that two jar 
files are required: testng.jar and jcommander.jar.


You should consider taking jcommander.jar into account.  This will be 
more important/noticeable to folk outside Oracle who build their own 
copy of jtreg to use.


-- Jon

On 11/22/2016 08:41 PM, Frank Yuan wrote:


Hi All

Would you like to review 
http://cr.openjdk.java.net/~fyuan/8170192/webrev.00/ 
?


Bug: https://bugs.openjdk.java.net/browse/JDK-8170192

This patch is fully same as Daniel provided except a few lines of 
additional cleaning, thanks to Daniel for providing the patch!


Thanks

Frank

Re: RFR 8168745 Iterator.forEachRemaining vs. Iterator.remove

[Paul Sandoz]

Stuart and I refined the text to mention a concurrent modification policy (e.g. 
fail-fast or weakly consistent) of any overriding class.

Paul.


diff -r c7b932897909 src/java.base/share/classes/java/lang/Iterable.java
--- a/src/java.base/share/classes/java/lang/Iterable.java   Wed Nov 23 
10:35:44 2016 -0800
+++ b/src/java.base/share/classes/java/lang/Iterable.java   Wed Nov 23 
10:40:53 2016 -0800
@@ -53,10 +53,13 @@
 /**
  * Performs the given action for each element of the {@code Iterable}
  * until all elements have been processed or the action throws an
- * exception.  Unless otherwise specified by the implementing class,
- * actions are performed in the order of iteration (if an iteration order
- * is specified).  Exceptions thrown by the action are relayed to the
+ * exception.  Actions are performed in the order of iteration, if that
+ * order is specified.  Exceptions thrown by the action are relayed to the
  * caller.
+ * 
+ * The behavior of this method is unspecified if the action performs
+ * side-effects that modify the underlying source of elements, unless an
+ * overriding class has specified a concurrent modification policy.
  *
  * @implSpec
  * The default implementation behaves as if:
diff -r c7b932897909 src/java.base/share/classes/java/util/Iterator.java
--- a/src/java.base/share/classes/java/util/Iterator.java   Wed Nov 23 
10:35:44 2016 -0800
+++ b/src/java.base/share/classes/java/util/Iterator.java   Wed Nov 23 
10:40:53 2016 -0800
@@ -76,10 +76,15 @@
 /**
  * Removes from the underlying collection the last element returned
  * by this iterator (optional operation).  This method can be called
- * only once per call to {@link #next}.  The behavior of an iterator
- * is unspecified if the underlying collection is modified while the
- * iteration is in progress in any way other than by calling this
- * method.
+ * only once per call to {@link #next}.
+ * 
+ * The behavior of an iterator is unspecified if the underlying collection
+ * is modified while the iteration is in progress in any way other than by
+ * calling this method, unless an overriding class has specified a
+ * concurrent modification policy.
+ * 
+ * The behavior of an iterator is unspecified if this method is called
+ * after a call to the {@link #forEachRemaining forEachRemaining} method.
  *
  * @implSpec
  * The default implementation throws an instance of
@@ -102,6 +107,13 @@
  * have been processed or the action throws an exception.  Actions are
  * performed in the order of iteration, if that order is specified.
  * Exceptions thrown by the action are relayed to the caller.
+ * 
+ * The behavior of an iterator is unspecified if the action modifies the
+ * collection in any way (even by calling the {@link #remove remove} 
method),
+ * unless an overriding class has specified a concurrent modification 
policy.
+ * 
+ * Subsequent behavior of an iterator is unspecified if the action throws 
an
+ * exception.
  *
  * @implSpec
  * The default implementation behaves as if:

Re: RFR 8169808 Stream returning methods should specify if they are late binding

[Paul Sandoz]

> On 23 Nov 2016, at 07:50, Doug Lea  wrote:
> 
> On 11/23/2016 10:05 AM, Martin Buchholz wrote:
>> Right now, PBQ's spliterator is in violation of
>> 
>> """A Spliterator that does not report IMMUTABLE or CONCURRENT is
>> expected to have a documented policy concerning: when the spliterator
>> binds to the element source;"""
>> 
>> so I think we only get a choice between IMMUTABLE and CONCURRENT.  I
>> propose:
> 
> Well, OK. Hopefully no user program will hardwire which
> choice is made, in case the implementation changes.
> 

I think we boxed ourselves into the corner when we specified that the 
PBQ.spliterator reports SIZED. Although the spliterator is specified to be 
weakly consistent, it cannot reflect any modifications subsequent to first size 
query, traversal or split.

Paul.

Re: RFR JDK-8170192 [JAXP] [TESTBUG] test/javax/xml/jaxp/libs/jaxp/library/JAXPPolicyManager.java should grant permissions to jtreg, javatest, and testng jars

[Joe Wang]

+1. Thanks Frank!

-Joe

On 11/23/16, 6:00 AM, Daniel Fuchs wrote:

Hi Frank,

Thanks for taking this on.
Looks good to me.

-- daniel

On 23/11/16 04:41, Frank Yuan wrote:

Hi All



Would you like to review
http://cr.openjdk.java.net/~fyuan/8170192/webrev.00/?

Bug: https://bugs.openjdk.java.net/browse/JDK-8170192



This patch is fully same as Daniel provided except a few lines of
additional cleaning, thanks to Daniel for providing the patch!



Thanks

Frank

Re: JDK 9 RFR of JDK-8169479: java.lang.reflect.Constructor class has wrong api documentation

[Brian Burkhalter]

+1

Brian

On Nov 23, 2016, at 9:25 AM, Paul Sandoz  wrote:

> +1
> 
> Paul.
> 
>> On 23 Nov 2016, at 08:10, joe darcy  wrote:
>> 
>> Hello,
>> 
>> Please review the changes to address
>> 
>>   JDK-8169479: java.lang.reflect.Constructor class has wrong api 
>> documentation
>>   http://cr.openjdk.java.net/~darcy/8169479.0/
>> 
>> Patch inline below. There are a few issues reported in the bug:
>> 
>> * The Constructor.toString method fails to state it prints out "throws" 
>> information. The implementation already does this and the analogous methods 
>> Constructor.toGenericString, Method.toString, etc. explicitly mention throws 
>> information already.
>> 
>> * The specification of Executable.getDeclaringClass can be overridden to be 
>> more precise in the Constructor and Method subclasses.
>> 
>> * In Constructor.toGenericString, "return type" is used where "class name" 
>> is meant.
>> 
>> Thanks,
>> 
>> -Joe

Re: JDK 9 RFR of JDK-8169479: java.lang.reflect.Constructor class has wrong api documentation

[Paul Sandoz]

+1

Paul.

> On 23 Nov 2016, at 08:10, joe darcy  wrote:
> 
> Hello,
> 
> Please review the changes to address
> 
>JDK-8169479: java.lang.reflect.Constructor class has wrong api 
> documentation
>http://cr.openjdk.java.net/~darcy/8169479.0/
> 
> Patch inline below. There are a few issues reported in the bug:
> 
> * The Constructor.toString method fails to state it prints out "throws" 
> information. The implementation already does this and the analogous methods 
> Constructor.toGenericString, Method.toString, etc. explicitly mention throws 
> information already.
> 
> * The specification of Executable.getDeclaringClass can be overridden to be 
> more precise in the Constructor and Method subclasses.
> 
> * In Constructor.toGenericString, "return type" is used where "class name" is 
> meant.
> 
> Thanks,
> 
> -Joe

Re: RFR 9: 8169645 : ObjectInputFilter.Config spec is ambiguous regarding overriding the filter via System properties

[Brian Burkhalter]

Hi Roger,

This seems sufficiently clear.

Brian

On Nov 23, 2016, at 7:43 AM, Roger Riggs  wrote:

> Please review a clarification of the serialization filtering configuration of 
> pattern based filters
> requested and reviewed by the JCK team.
> 
> Webrev:
>   http://cr.openjdk.java.net/~rriggs/webrev-filter-config-8169645/
> 
> Thanks, Roger
> 

JDK 9 RFR of JDK-8169479: java.lang.reflect.Constructor class has wrong api documentation

[joe darcy]

Hello,

Please review the changes to address

JDK-8169479: java.lang.reflect.Constructor class has wrong api 
documentation

http://cr.openjdk.java.net/~darcy/8169479.0/

Patch inline below. There are a few issues reported in the bug:

* The Constructor.toString method fails to state it prints out "throws" 
information. The implementation already does this and the analogous 
methods Constructor.toGenericString, Method.toString, etc. explicitly 
mention throws information already.


* The specification of Executable.getDeclaringClass can be overridden to 
be more precise in the Constructor and Method subclasses.


* In Constructor.toGenericString, "return type" is used where "class 
name" is meant.


Thanks,

-Joe

--- old/src/java.base/share/classes/java/lang/reflect/Constructor.java 
2016-11-23 07:33:00.095083685 -0800
+++ new/src/java.base/share/classes/java/lang/reflect/Constructor.java 
2016-11-23 07:32:59.927083679 -0800

@@ -200,7 +200,8 @@
 }

 /**
- * {@inheritDoc}
+ * Returns the {@code Class} object representing the class that
+ * declares the constructor represented by this object.
  */
 @Override
 public Class getDeclaringClass() {
@@ -321,6 +322,11 @@
  *public java.util.Hashtable(int,float)
  * }
  *
+ * If the constructor is declared to throw exceptions, the
+ * parameter list is followed by a space, followed by the word
+ * "{@code throws}" followed by a comma-separated list of the
+ * thrown exception types.
+ *
  * The only possible modifiers for constructors are the access
  * modifiers {@code public}, {@code protected} or
  * {@code private}.  Only one of these may appear, or none if the
@@ -357,13 +363,13 @@
  * "Type...".
  *
  * A space is used to separate access modifiers from one another
- * and from the type parameters or return type.  If there are no
+ * and from the type parameters or class name.  If there are no
  * type parameters, the type parameter list is elided; if the type
  * parameter list is present, a space separates the list from the
  * class name.  If the constructor is declared to throw
  * exceptions, the parameter list is followed by a space, followed
  * by the word "{@code throws}" followed by a
- * comma-separated list of the thrown exception types.
+ * comma-separated list of the generic thrown exception types.
  *
  * The only possible modifiers for constructors are the access
  * modifiers {@code public}, {@code protected} or
--- old/src/java.base/share/classes/java/lang/reflect/Method.java 
2016-11-23 07:33:00.495083699 -0800
+++ new/src/java.base/share/classes/java/lang/reflect/Method.java 
2016-11-23 07:33:00.343083694 -0800

@@ -211,7 +211,8 @@
 }

 /**
- * {@inheritDoc}
+ * Returns the {@code Class} object representing the class or interface
+ * that declares the method represented by this object.
  */
 @Override
 public Class getDeclaringClass() {
@@ -372,7 +373,7 @@
  * the method name, followed by a parenthesized, comma-separated
  * list of the method's formal parameter types. If the method
  * throws checked exceptions, the parameter list is followed by a
- * space, followed by the word throws followed by a
+ * space, followed by the word "{@code throws}" followed by a
  * comma-separated list of the thrown exception types.
  * For example:
  * 
@@ -428,8 +429,8 @@
  * parameter list is present, a space separates the list from the
  * class name.  If the method is declared to throw exceptions, the
  * parameter list is followed by a space, followed by the word
- * throws followed by a comma-separated list of the generic thrown
- * exception types.
+ * "{@code throws}" followed by a comma-separated list of the generic
+ * thrown exception types.
  *
  * The access modifiers are placed in canonical order as
  * specified by "The Java Language Specification".  This is

Re: RFR(s) 8170153: PPC64: Poor StrictMath performance due to non-optimized compilation

[Gustavo Romero]

Hi Erik,

On 23-11-2016 12:29, Erik Joelsson wrote:
> Build changes look ok.
> 
> In CoreLibraries.gmk, I think it would have been ok to keep the conditional 
> checking (OPENJDK_TARGET_CPU_ARCH, ppc), but this certainly works too.

Thanks a lot for reviewing the change.


Regards,
Gustavo

Re: RFR 8169808 Stream returning methods should specify if they are late binding

[Doug Lea]

On 11/23/2016 10:05 AM, Martin Buchholz wrote:

Right now, PBQ's spliterator is in violation of

"""A Spliterator that does not report IMMUTABLE or CONCURRENT is
expected to have a documented policy concerning: when the spliterator
binds to the element source;"""

so I think we only get a choice between IMMUTABLE and CONCURRENT.  I
propose:


Well, OK. Hopefully no user program will hardwire which
choice is made, in case the implementation changes.

-Doug




--- src/main/java/util/concurrent/PriorityBlockingQueue.java23 Nov 2016
14:51:10 -1.116
+++ src/main/java/util/concurrent/PriorityBlockingQueue.java23 Nov 2016
15:01:29 -
@@ -962,3 +962,6 @@
 public int characteristics() {
-return Spliterator.NONNULL | Spliterator.SIZED |
Spliterator.SUBSIZED;
+return (Spliterator.IMMUTABLE |
+Spliterator.NONNULL |
+Spliterator.SIZED |
+Spliterator.SUBSIZED);
 }



On Wed, Nov 23, 2016 at 6:14 AM, Doug Lea > wrote:

On 11/22/2016 08:33 PM, Martin Buchholz wrote:

PriorityBlockingQueue has a late-binding-style, snapshot spliterator
that does not report IMMUTABLE.  It *is* immutable ... after the
first
access!  Should we add IMMUTABLE?


Probably not.
This might overly constrain future improvements; for example, someday
replacing with some linked structure.
Arguably, if so, we could just change the reported characteristics,
but that could be surprising to users, and possibly lead to subtle
user bugs.

-Doug

RFR 9: 8169645 : ObjectInputFilter.Config spec is ambiguous regarding overriding the filter via System properties

[Roger Riggs]

Please review a clarification of the serialization filtering 
configuration of pattern based filters

requested and reviewed by the JCK team.

Webrev:
   http://cr.openjdk.java.net/~rriggs/webrev-filter-config-8169645/

Thanks, Roger

Re: RFR(s) 8170153: PPC64: Poor StrictMath performance due to non-optimized compilation

[Gustavo Romero]

Hi Martin,

On 23-11-2016 12:38, Doerr, Martin wrote:
> Hi Gustavo,
> 
> thanks for providing the webrevs.
> 
> I have ran the StrictMath jck tests which fail when building with -O3 and 
> without -ffp-contract=off:
> FailedTests:
> api/java_lang/StrictMath/desc.html#acos 
> javasoft.sqe.tests.api.java.lang.StrictMath.acos_test
> api/java_lang/StrictMath/desc.html#asin 
> javasoft.sqe.tests.api.java.lang.StrictMath.asin_test
> api/java_lang/StrictMath/desc.html#atan 
> javasoft.sqe.tests.api.java.lang.StrictMath.atan_test
> api/java_lang/StrictMath/desc.html#atan2 
> javasoft.sqe.tests.api.java.lang.StrictMath.atan2_test
> api/java_lang/StrictMath/desc.html#cos 
> javasoft.sqe.tests.api.java.lang.StrictMath.cos_test
> api/java_lang/StrictMath/desc.html#exp 
> javasoft.sqe.tests.api.java.lang.StrictMath.exp_test
> api/java_lang/StrictMath/desc.html#log 
> javasoft.sqe.tests.api.java.lang.StrictMath.log_test
> api/java_lang/StrictMath/desc.html#sin 
> javasoft.sqe.tests.api.java.lang.StrictMath.sin_test
> api/java_lang/StrictMath/desc.html#tan 
> javasoft.sqe.tests.api.java.lang.StrictMath.tan_test
> api/java_lang/StrictMath/index.html#expm1 
> javasoft.sqe.tests.api.java.lang.StrictMath.expm1Tests -TestCaseID ALL
> api/java_lang/StrictMath/index.html#log10 
> javasoft.sqe.tests.api.java.lang.StrictMath.log10Tests -TestCaseID ALL
> api/java_lang/StrictMath/index.html#log1p 
> javasoft.sqe.tests.api.java.lang.StrictMath.log1pTests -TestCaseID ALL
> 
> All of them have passed when building with -O3 and -ffp-contract=off (on 
> linuxppc64le).

Thank you very much for running the additional StrictMath jck tests against the 
change!


Best regards,
Gustavo

Re: RFR(s) 8170153: PPC64: Poor StrictMath performance due to non-optimized compilation

[Gustavo Romero]

Hi Volker,

On 23-11-2016 12:05, Volker Simonis wrote:
> thanks a lot for tracking this down!

Happy to contribute :)


> The change looks good and I a can sponsor it once you get another
> review from the build group and the FC Extension Request was approved.

Thanks a lot for sponsoring it!


> In general I'd advise to sign the OCTLA [1] to get access to the Java
> SE TCK [2] as this contains quite a lot of additional conformance
> tests which can be quite valuable for changes like this.
> 
> Regards,
> Volker
> 
> [1] http://openjdk.java.net/legal/octla-java-se-8.pdf
> [2] http://openjdk.java.net/groups/conformance/JckAccess/

Right. I'll check the documentation and find a way to get access to the TCK.


Best regards,
Gustavo

Re: RFR 8169808 Stream returning methods should specify if they are late binding

[Martin Buchholz]

Right now, PBQ's spliterator is in violation of

"""A Spliterator that does not report IMMUTABLE or CONCURRENT is expected
to have a documented policy concerning: when the spliterator binds to the
element source;"""

so I think we only get a choice between IMMUTABLE and CONCURRENT.  I
propose:

--- src/main/java/util/concurrent/PriorityBlockingQueue.java 23 Nov 2016
14:51:10 - 1.116
+++ src/main/java/util/concurrent/PriorityBlockingQueue.java 23 Nov 2016
15:01:29 -
@@ -962,3 +962,6 @@
 public int characteristics() {
-return Spliterator.NONNULL | Spliterator.SIZED |
Spliterator.SUBSIZED;
+return (Spliterator.IMMUTABLE |
+Spliterator.NONNULL |
+Spliterator.SIZED |
+Spliterator.SUBSIZED);
 }



On Wed, Nov 23, 2016 at 6:14 AM, Doug Lea  wrote:

> On 11/22/2016 08:33 PM, Martin Buchholz wrote:
>
>> PriorityBlockingQueue has a late-binding-style, snapshot spliterator
>> that does not report IMMUTABLE.  It *is* immutable ... after the first
>> access!  Should we add IMMUTABLE?
>>
>
> Probably not.
> This might overly constrain future improvements; for example, someday
> replacing with some linked structure.
> Arguably, if so, we could just change the reported characteristics,
> but that could be surprising to users, and possibly lead to subtle user
> bugs.
>
> -Doug
>
>

Re: RFR(s) 8170153: PPC64: Poor StrictMath performance due to non-optimized compilation

[Erik Joelsson]

Build changes look ok.

In CoreLibraries.gmk, I think it would have been ok to keep the 
conditional checking (OPENJDK_TARGET_CPU_ARCH, ppc), but this certainly 
works too.


/Erik


On 2016-11-22 01:43, Gustavo Romero wrote:

Hi,

Could the following change be reviewed, please?

webrev 1/2: http://cr.openjdk.java.net/~gromero/8170153/
webrev 2/2: http://cr.openjdk.java.net/~gromero/8170153/jdk/
bug:https://bugs.openjdk.java.net/browse/JDK-8170153

It enables fdlibm optimization on Linux PPC64 LE & BE and hence speeds up the
StrictMath methods (in some cases up to 3x) on that platform.

On PPC64 fdlibm optimization can be done without precision issues if
floating-point expression contraction is disable, i.e. if the compiler does not
use floating-point multiply-add (FMA). For further details please refer to gcc
bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78386

No regression was observed on Math and StrictMath tests:

Passed: java/lang/Math/AbsPositiveZero.java
Passed: java/lang/Math/Atan2Tests.java
Passed: java/lang/Math/CeilAndFloorTests.java
Passed: java/lang/Math/CubeRootTests.java
Passed: java/lang/Math/DivModTests.java
Passed: java/lang/Math/ExactArithTests.java
Passed: java/lang/Math/Expm1Tests.java
Passed: java/lang/Math/FusedMultiplyAddTests.java
Passed: java/lang/Math/HyperbolicTests.java
Passed: java/lang/Math/HypotTests.java
Passed: java/lang/Math/IeeeRecommendedTests.java
Passed: java/lang/Math/Log10Tests.java
Passed: java/lang/Math/Log1pTests.java
Passed: java/lang/Math/MinMax.java
Passed: java/lang/Math/MultiplicationTests.java
Passed: java/lang/Math/PowTests.java
Passed: java/lang/Math/Rint.java
Passed: java/lang/Math/RoundTests.java
Passed: java/lang/Math/SinCosCornerCasesTests.java
Passed: java/lang/Math/TanTests.java
Passed: java/lang/Math/WorstCaseTests.java
Test results: passed: 21

Passed: java/lang/StrictMath/CubeRootTests.java
Passed: java/lang/StrictMath/ExactArithTests.java
Passed: java/lang/StrictMath/Expm1Tests.java
Passed: java/lang/StrictMath/HyperbolicTests.java
Passed: java/lang/StrictMath/HypotTests.java
Passed: java/lang/StrictMath/Log10Tests.java
Passed: java/lang/StrictMath/Log1pTests.java
Passed: java/lang/StrictMath/PowTests.java
Test results: passed: 8

and also on the following hotspot tests:

Passed: compiler/intrinsics/mathexact/sanity/AddExactIntTest.java
Passed: compiler/intrinsics/mathexact/sanity/AddExactLongTest.java
Passed: compiler/intrinsics/mathexact/sanity/DecrementExactIntTest.java
Passed: compiler/intrinsics/mathexact/sanity/DecrementExactLongTest.java
Passed: compiler/intrinsics/mathexact/sanity/IncrementExactIntTest.java
Passed: compiler/intrinsics/mathexact/sanity/IncrementExactLongTest.java
Passed: compiler/intrinsics/mathexact/sanity/MultiplyExactIntTest.java
Passed: compiler/intrinsics/mathexact/sanity/MultiplyExactLongTest.java
Passed: compiler/intrinsics/mathexact/sanity/NegateExactIntTest.java
Passed: compiler/intrinsics/mathexact/sanity/NegateExactLongTest.java
Passed: compiler/intrinsics/mathexact/sanity/SubtractExactIntTest.java
Passed: compiler/intrinsics/mathexact/sanity/SubtractExactLongTest.java
Passed: compiler/intrinsics/mathexact/AddExactICondTest.java
Passed: compiler/intrinsics/mathexact/AddExactIConstantTest.java
Passed: compiler/intrinsics/mathexact/AddExactILoadTest.java
Passed: compiler/intrinsics/mathexact/AddExactILoopDependentTest.java
Passed: compiler/intrinsics/mathexact/AddExactINonConstantTest.java
Passed: compiler/intrinsics/mathexact/AddExactIRepeatTest.java
Passed: compiler/intrinsics/mathexact/AddExactLConstantTest.java
Passed: compiler/intrinsics/mathexact/AddExactLNonConstantTest.java
Passed: compiler/intrinsics/mathexact/CompareTest.java
Passed: compiler/intrinsics/mathexact/DecExactITest.java
Passed: compiler/intrinsics/mathexact/DecExactLTest.java
Passed: compiler/intrinsics/mathexact/GVNTest.java
Passed: compiler/intrinsics/mathexact/IncExactITest.java
Passed: compiler/intrinsics/mathexact/IncExactLTest.java
Passed: compiler/intrinsics/mathexact/MulExactICondTest.java
Passed: compiler/intrinsics/mathexact/MulExactIConstantTest.java
Passed: compiler/intrinsics/mathexact/MulExactILoadTest.java
Passed: compiler/intrinsics/mathexact/MulExactILoopDependentTest.java
Passed: compiler/intrinsics/mathexact/MulExactINonConstantTest.java
Passed: compiler/intrinsics/mathexact/MulExactIRepeatTest.java
Passed: compiler/intrinsics/mathexact/MulExactLConstantTest.java
Passed: compiler/intrinsics/mathexact/MulExactLNonConstantTest.java
Passed: compiler/intrinsics/mathexact/NegExactIConstantTest.java
Passed: compiler/intrinsics/mathexact/NegExactILoadTest.java
Passed: compiler/intrinsics/mathexact/NegExactILoopDependentTest.java
Passed: compiler/intrinsics/mathexact/NegExactINonConstantTest.java
Passed: compiler/intrinsics/mathexact/NegExactLConstantTest.java
Passed: compiler/intrinsics/mathexact/NegExactLNonConstantTest.java
Passed: compiler/intrinsics/mathexact/NestedMathExactTest.java
Passed: 

Re: RFR 8169808 Stream returning methods should specify if they are late binding

[Doug Lea]

On 11/22/2016 08:33 PM, Martin Buchholz wrote:

PriorityBlockingQueue has a late-binding-style, snapshot spliterator
that does not report IMMUTABLE.  It *is* immutable ... after the first
access!  Should we add IMMUTABLE?


Probably not.
This might overly constrain future improvements; for example, someday
replacing with some linked structure.
Arguably, if so, we could just change the reported characteristics,
but that could be surprising to users, and possibly lead to subtle user 
bugs.


-Doug

Re: RFR(s) 8170153: PPC64: Poor StrictMath performance due to non-optimized compilation

[Volker Simonis]

Hi Gustavo,

thanks a lot for tracking this down!

The change looks good and I a can sponsor it once you get another
review from the build group and the FC Extension Request was approved.

In general I'd advise to sign the OCTLA [1] to get access to the Java
SE TCK [2] as this contains quite a lot of additional conformance
tests which can be quite valuable for changes like this.

Regards,
Volker

[1] http://openjdk.java.net/legal/octla-java-se-8.pdf
[2] http://openjdk.java.net/groups/conformance/JckAccess/


On Tue, Nov 22, 2016 at 1:43 AM, Gustavo Romero
 wrote:
> Hi,
>
> Could the following change be reviewed, please?
>
> webrev 1/2: http://cr.openjdk.java.net/~gromero/8170153/
> webrev 2/2: http://cr.openjdk.java.net/~gromero/8170153/jdk/
> bug:https://bugs.openjdk.java.net/browse/JDK-8170153
>
> It enables fdlibm optimization on Linux PPC64 LE & BE and hence speeds up the
> StrictMath methods (in some cases up to 3x) on that platform.
>
> On PPC64 fdlibm optimization can be done without precision issues if
> floating-point expression contraction is disable, i.e. if the compiler does 
> not
> use floating-point multiply-add (FMA). For further details please refer to gcc
> bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78386
>
> No regression was observed on Math and StrictMath tests:
>
> Passed: java/lang/Math/AbsPositiveZero.java
> Passed: java/lang/Math/Atan2Tests.java
> Passed: java/lang/Math/CeilAndFloorTests.java
> Passed: java/lang/Math/CubeRootTests.java
> Passed: java/lang/Math/DivModTests.java
> Passed: java/lang/Math/ExactArithTests.java
> Passed: java/lang/Math/Expm1Tests.java
> Passed: java/lang/Math/FusedMultiplyAddTests.java
> Passed: java/lang/Math/HyperbolicTests.java
> Passed: java/lang/Math/HypotTests.java
> Passed: java/lang/Math/IeeeRecommendedTests.java
> Passed: java/lang/Math/Log10Tests.java
> Passed: java/lang/Math/Log1pTests.java
> Passed: java/lang/Math/MinMax.java
> Passed: java/lang/Math/MultiplicationTests.java
> Passed: java/lang/Math/PowTests.java
> Passed: java/lang/Math/Rint.java
> Passed: java/lang/Math/RoundTests.java
> Passed: java/lang/Math/SinCosCornerCasesTests.java
> Passed: java/lang/Math/TanTests.java
> Passed: java/lang/Math/WorstCaseTests.java
> Test results: passed: 21
>
> Passed: java/lang/StrictMath/CubeRootTests.java
> Passed: java/lang/StrictMath/ExactArithTests.java
> Passed: java/lang/StrictMath/Expm1Tests.java
> Passed: java/lang/StrictMath/HyperbolicTests.java
> Passed: java/lang/StrictMath/HypotTests.java
> Passed: java/lang/StrictMath/Log10Tests.java
> Passed: java/lang/StrictMath/Log1pTests.java
> Passed: java/lang/StrictMath/PowTests.java
> Test results: passed: 8
>
> and also on the following hotspot tests:
>
> Passed: compiler/intrinsics/mathexact/sanity/AddExactIntTest.java
> Passed: compiler/intrinsics/mathexact/sanity/AddExactLongTest.java
> Passed: compiler/intrinsics/mathexact/sanity/DecrementExactIntTest.java
> Passed: compiler/intrinsics/mathexact/sanity/DecrementExactLongTest.java
> Passed: compiler/intrinsics/mathexact/sanity/IncrementExactIntTest.java
> Passed: compiler/intrinsics/mathexact/sanity/IncrementExactLongTest.java
> Passed: compiler/intrinsics/mathexact/sanity/MultiplyExactIntTest.java
> Passed: compiler/intrinsics/mathexact/sanity/MultiplyExactLongTest.java
> Passed: compiler/intrinsics/mathexact/sanity/NegateExactIntTest.java
> Passed: compiler/intrinsics/mathexact/sanity/NegateExactLongTest.java
> Passed: compiler/intrinsics/mathexact/sanity/SubtractExactIntTest.java
> Passed: compiler/intrinsics/mathexact/sanity/SubtractExactLongTest.java
> Passed: compiler/intrinsics/mathexact/AddExactICondTest.java
> Passed: compiler/intrinsics/mathexact/AddExactIConstantTest.java
> Passed: compiler/intrinsics/mathexact/AddExactILoadTest.java
> Passed: compiler/intrinsics/mathexact/AddExactILoopDependentTest.java
> Passed: compiler/intrinsics/mathexact/AddExactINonConstantTest.java
> Passed: compiler/intrinsics/mathexact/AddExactIRepeatTest.java
> Passed: compiler/intrinsics/mathexact/AddExactLConstantTest.java
> Passed: compiler/intrinsics/mathexact/AddExactLNonConstantTest.java
> Passed: compiler/intrinsics/mathexact/CompareTest.java
> Passed: compiler/intrinsics/mathexact/DecExactITest.java
> Passed: compiler/intrinsics/mathexact/DecExactLTest.java
> Passed: compiler/intrinsics/mathexact/GVNTest.java
> Passed: compiler/intrinsics/mathexact/IncExactITest.java
> Passed: compiler/intrinsics/mathexact/IncExactLTest.java
> Passed: compiler/intrinsics/mathexact/MulExactICondTest.java
> Passed: compiler/intrinsics/mathexact/MulExactIConstantTest.java
> Passed: compiler/intrinsics/mathexact/MulExactILoadTest.java
> Passed: compiler/intrinsics/mathexact/MulExactILoopDependentTest.java
> Passed: compiler/intrinsics/mathexact/MulExactINonConstantTest.java
> Passed: compiler/intrinsics/mathexact/MulExactIRepeatTest.java
> Passed: compiler/intrinsics/mathexact/MulExactLConstantTest.java
> Passed: 

Re: RFR JDK-8170192 [JAXP] [TESTBUG] test/javax/xml/jaxp/libs/jaxp/library/JAXPPolicyManager.java should grant permissions to jtreg, javatest, and testng jars

[Daniel Fuchs]

Hi Frank,

Thanks for taking this on.
Looks good to me.

-- daniel

On 23/11/16 04:41, Frank Yuan wrote:

Hi All



Would you like to review
http://cr.openjdk.java.net/~fyuan/8170192/webrev.00/?

Bug: https://bugs.openjdk.java.net/browse/JDK-8170192



This patch is fully same as Daniel provided except a few lines of
additional cleaning, thanks to Daniel for providing the patch!



Thanks

Frank

Re: Did the tests fail due to TestNG dataprovider parameter type check?

[Daniel Fuchs]

Hi Frank,

I ran only the tests under jaxp/tests/javax - and one test was failing
(DurationTest) with a similar error (something about a mismatch between
Integer & Long in data provider).

This looked to be completely unrelated to the permission changes, so I
thought it would be better to investigate it as a separate issue.
The failure in the Duration test looked simpler - so it might be a good
place to look at to try and figure out what is going on.

best regards,

-- daniel


On 23/11/16 08:31, Frank Yuan wrote:

Hi Jon

I run the whole jdk regression tests with jtreg-4.2-b03 [1], and then found 
lots of tests in different test groups failed with the error message like 
following:
test test.java.time.format.TestNumberPrinter.test_pad_ALWAYS(): failure
org.testng.internal.reflect.MethodMatcherException:
Data provider mismatch
Method: test_pad_ALWAYS([Parameter{index=0, type=int, declaredAnnotations=[]}, 
Parameter{index=1, type=int, declaredAnnotations=[]}, Parameter{index=2, 
type=long, declaredAnnotations=[]}, Parameter{index=3, type=java.lang.String, 
declaredAnnotations=[]}])
Arguments: 
[(java.lang.Integer)1,(java.lang.Integer)1,(java.lang.Integer)-10,null]
at 
org.testng.internal.reflect.DataProviderMethodMatcher.getConformingArguments(DataProviderMethodMatcher.java:52)
at org.testng.internal.Invoker.injectParameters(Invoker.java:1228)
at org.testng.internal.Invoker.invokeTestMethods(Invoker.java:1125)
at 
org.testng.internal.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:129)
at org.testng.internal.TestMethodWorker.run(TestMethodWorker.java:112)
at org.testng.TestRunner.privateRun(TestRunner.java:778)
at org.testng.TestRunner.run(TestRunner.java:632)
at org.testng.SuiteRunner.runTest(SuiteRunner.java:366)
at org.testng.SuiteRunner.runSequentially(SuiteRunner.java:361)
at org.testng.SuiteRunner.privateRun(SuiteRunner.java:319)
at org.testng.SuiteRunner.run(SuiteRunner.java:268)
at org.testng.SuiteRunnerWorker.runSuite(SuiteRunnerWorker.java:52)
at org.testng.SuiteRunnerWorker.run(SuiteRunnerWorker.java:86)
at org.testng.TestNG.runSuitesSequentially(TestNG.java:1225)
at org.testng.TestNG.runSuitesLocally(TestNG.java:1150)
at org.testng.TestNG.runSuites(TestNG.java:1075)
at org.testng.TestNG.run(TestNG.java:1047)
at 
com.sun.javatest.regtest.TestNGAction$TestNGRunner.main(TestNGAction.java:220)
at 
com.sun.javatest.regtest.TestNGAction$TestNGRunner.main(TestNGAction.java:184)
at 
java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at 
java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at 
java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.base/java.lang.reflect.Method.invoke(Method.java:537)
at 
com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapper.java:110)
at java.base/java.lang.Thread.run(Thread.java:844)


This test methd signature is:
 public void test_pad_ALWAYS(int minPad, int maxPad, long value, String result)

The provider return the following data
Object[][] provider_pad() {
return new Object[][] {
{1, 1, -10, null},
{1, 1, -9, "9"},
{1, 1, -1, "1"},
...

However, I got message "Cannot find class java/lang/reflect/JTRegModuleHelper.class 
to init patch directory" when I run jtreg, although I didn't find any code related 
to testng dataprovider in jtreg source, I would double confirm with you if this is a 
definite issue or anything I made incorrectly?

To Christoph

Except above issue, I didn't find any other issue in jdk and langtools repo so 
far.


[1] 
https://adopt-openjdk.ci.cloudbees.com/job/jtreg/lastSuccessfulBuild/artifact/jtreg-4.2-b03.tar.gz


Thanks
Frank


-Original Message-
From: Langer, Christoph [mailto:christoph.lan...@sap.com]
Sent: Wednesday, November 23, 2016 3:41 PM
To: Frank Yuan
Cc: code-tools-...@openjdk.java.net; core-libs-dev@openjdk.java.net; 
jtreg-...@openjdk.java.net; 'Volker Simonis'; 'Daniel Fuchs'
Subject: RE: Issues running JAXP jtreg tests ("java.lang.RuntimePermission" 
"accessDeclaredMembers")

Thanks, Frank.

we run scheduled jtreg tests for jdk every night so we should have encountered 
issues if there were some. But langtools could be interesting, I
don't think those run automatically for OpenJDK in our environment.

Best regards
Christoph


-Original Message-
From: Frank Yuan [mailto:frank.y...@oracle.com]
Sent: Mittwoch, 23. November 2016 06:26
To: Langer, Christoph ; 'Volker Simonis'
; 'Daniel Fuchs' 
Cc: code-tools-...@openjdk.java.net; core-libs-dev@openjdk.java.net; jtreg-
u...@openjdk.java.net
Subject: RE: Issues running JAXP jtreg tests 

Re: RFR[9] JDK-8158916: ProblemList.txt update for com/sun/jndi/ldap/DeadSSLLdapTimeoutTest.java

[Daniel Fuchs]

Looks good!

On 23/11/16 05:49, John Jiang wrote:

Hi Daniel,
I'll push the below patch:
diff -r 5cd2aa3f3e9b test/ProblemList.txt
--- a/test/ProblemList.txt  Tue Nov 22 10:45:48 2016 -0800
+++ b/test/ProblemList.txt  Tue Nov 22 21:34:47 2016 -0800
@@ -305,6 +305,6 @@

 # jdk_other

-com/sun/jndi/ldap/DeadSSLLdapTimeoutTest.java 8141370
linux-i586,macosx-all
+com/sun/jndi/ldap/DeadSSLLdapTimeoutTest.java 8169942
linux-i586,macosx-all,windows-x64

 


Best regards,
John Jiang

On 2016/11/18 18:03, Daniel Fuchs wrote:

Hi John,

Looks good to me. Should windows-amd64 also be listed?
It appears the test also failing there (JDK-8152654)

best regards,

-- daniel

On 18/11/16 06:02, John Jiang wrote:

Hi,
Track this test issue with a new JBS bug JDK-8169942.
Please take a look at the updated webrew:
http://cr.openjdk.java.net/~jjiang/8158916/webrev.01/, or the below,

--- old/test/ProblemList.txt2016-11-17 21:29:12.112822913 -0800
+++ new/test/ProblemList.txt2016-11-17 21:29:11.964822915 -0800
@@ -298,6 +298,6 @@

 # jdk_other

-com/sun/jndi/ldap/DeadSSLLdapTimeoutTest.java 8141370
linux-i586,macosx-all
+com/sun/jndi/ldap/DeadSSLLdapTimeoutTest.java 8169942
linux-i586,macosx-all

 



Best regards,
John Jiang


On 2016/11/8 18:53, Daniel Fuchs wrote:

Hi John,

On 08/11/16 01:30, John Jiang wrote:

Hi Daniel,
Thanks for your comments.

On 2016/11/7 19:21, Daniel Fuchs wrote:

Hi John,

Maybe you should reach out to Rob who fixed JDK-8141370 to
make sure this is the right thing to do.

just cc Rob Mckenna



It seems the exclusion on problematic platforms [1] was deliberate:


Basically I've separated the failing subtest out into its own
file and
excluded it on the (intermittently) failing platforms.

I see. But it should use an open issue.
I would like to open a new issue to track this issue. But I don't get
the exact failure log on DeadSSLLdapTimeoutTest.java, though the
failure
may still be related to SSLHandshakeException.
When this test was born, it was in the ProblemList, so I suppose it
has
no chance to be executed by our automatic testing systems.


I see. I agree it's better to have an open bug in the ProblemList.
But I am concerned if that bug is something that we never intend to
fix because the test is not appropriate for those platforms.

best regards,

-- daniel



Best regards,
John Jiang



best regards,

-- daniel

[1]
http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-December/037385.html





On 07/11/16 02:37, John Jiang wrote:

Hi,
com/sun/jndi/ldap/DeadSSLLdapTimeoutTest.java is associated to
8141370
in ProblemList, though issue JDK-8141370 has been resolved.
In fact, the fix for JDK-8141370 extracted some codes from
com/sun/jndi/ldap/LdapTimeoutTest.java to create
com/sun/jndi/ldap/DeadSSLLdapTimeoutTest.java, and put the new
test to
ProblemList.
It would be better to remove this item from ProblemList. If
com/sun/jndi/ldap/DeadSSLLdapTimeoutTest.java still fails
intermittently, we can put it back to ProblemList with the exact
failure
logs.

Issue: https://bugs.openjdk.java.net/browse/JDK-8158916
Webrev: http://cr.openjdk.java.net/~jjiang/8158916/webrev.00/

Best regards,
John Jiang

Re: RFR: JDK-8167648: java.io.PrintWriter should have PrintWriter((String|File), Charset) constructors

[Stephen Colebourne]

Returning the writer was my intention. I also intended it to return a
new instance, to avoid changing the variable from final to non-final.
Stephen


On 23 November 2016 at 13:35, Jonathan Bluett-Duncan
 wrote:
> Hi Patrick,
>
> Have you considered making `withAutoFlush()` return the `PrintWriter`
> itself, allowing fluent code snippets like the following?
>
> ```
> PrintWriter writer = new PrintWriter(new File("path/to/file.txt"),
> StandardCharsets.UTF_8).withAutoFlush();
> ```
>
> Kind regards,
> Jonathan
>
> On 23 November 2016 at 13:09, Patrick Reinhart  wrote:
>>
>> Added those new public constructors:
>>
>> PrintWriter(OutputStream, Charset)
>> PrintWriter(File, Charset)
>> withAutoFlush()
>>
>> Also added a new private constructor:
>>
>> PrintWriter(OutputStream, Charset, boolean)
>>
>> and rewired the OutputStream constructor calls to this private
>> constructor.
>>
>>
>> Here's the webrev:
>>
>> http://cr.openjdk.java.net/~reinhapa/reviews/8167648/webrev.00
>>
>>
>> -Patrick
>
>

Re: RFR: JDK-8167648: java.io.PrintWriter should have PrintWriter((String|File), Charset) constructors

[Patrick Reinhart]

On 2016-11-23 14:35, Jonathan Bluett-Duncan wrote:

Hi Patrick,

Have you considered making `withAutoFlush()` return the `PrintWriter`
itself, allowing fluent code snippets like the following?

```
PrintWriter writer = new PrintWriter(new File("path/to/file.txt"),
StandardCharsets.UTF_8).withAutoFlush();
```



Good point. I taught about it, but it somehow got lost on the way of 
re-wiring the constructors... :-)


Just updated the webrev


-Patrick

Re: RFR: JDK-8167648: java.io.PrintWriter should have PrintWriter((String|File), Charset) constructors

[Jonathan Bluett-Duncan]

Hi Patrick,

Have you considered making `withAutoFlush()` return the `PrintWriter`
itself, allowing fluent code snippets like the following?

```
PrintWriter writer = new PrintWriter(new File("path/to/file.txt"),
StandardCharsets.UTF_8).withAutoFlush();
```

Kind regards,
Jonathan

On 23 November 2016 at 13:09, Patrick Reinhart  wrote:

> Added those new public constructors:
>
> PrintWriter(OutputStream, Charset)
> PrintWriter(File, Charset)
> withAutoFlush()
>
> Also added a new private constructor:
>
> PrintWriter(OutputStream, Charset, boolean)
>
> and rewired the OutputStream constructor calls to this private constructor.
>
>
> Here's the webrev:
>
> http://cr.openjdk.java.net/~reinhapa/reviews/8167648/webrev.00
>
>
> -Patrick
>

RFR: JDK-8167648: java.io.PrintWriter should have PrintWriter((String|File), Charset) constructors

[Patrick Reinhart]

Added those new public constructors:

PrintWriter(OutputStream, Charset)
PrintWriter(File, Charset)
withAutoFlush()

Also added a new private constructor:

PrintWriter(OutputStream, Charset, boolean)

and rewired the OutputStream constructor calls to this private 
constructor.



Here's the webrev:

http://cr.openjdk.java.net/~reinhapa/reviews/8167648/webrev.00


-Patrick

Re: RFR 8160036: Java API doc for method minusMonths in LocalDateTime class needs correction

[Stephen Colebourne]

+1
Stephen

On 7 November 2016 at 21:23, Roger Riggs  wrote:
> Looks good,
>
> Thanks, Roger
>
>
>
> On 11/7/2016 7:27 AM, Bhanu Gopularam wrote:
>>
>> Thanks Nadeesh. Here is the updated webrev:
>>
>> http://cr.openjdk.java.net/~bgopularam/8160036/webrev.01
>>
>> Thanks,
>> Bhanu
>>
>> -Original Message-
>> From: nadeesh tv
>> Sent: Monday, November 07, 2016 5:38 PM
>> To: core-libs-dev@openjdk.java.net
>> Subject: Re: RFR 8160036: Java API doc for method minusMonths in
>> LocalDateTime class needs correction
>>
>> Hi Bhanu,
>> Same issues with OffsetDateTime minusWeeks and minusDays
>>
>> Thanks and Regards,
>> Nadeesh
>>
>> On 11/7/2016 5:31 PM, Bhanu Gopularam wrote:
>>>
>>> Hi all,
>>>
>>> Could you please review fix for following issue?
>>>
>>> Bug id: https://bugs.openjdk.java.net/browse/JDK-8160036
>>>
>>> Solution: Corrected documentation for couple of methods in LocalDateTime
>>> and OffsetDateTime classes
>>>
>>> Webrev: http://cr.openjdk.java.net/~bgopularam/8160036/webrev.00
>>>
>>> Thanks,
>>> Bhanu
>
>

Re: RFR: JDK-8167648: java.io.PrintWriter should have PrintWriter((String|File), Charset) constructors

[Patrick Reinhart]

Hi Stephen,

On 2016-11-23 12:04, Stephen Colebourne wrote:



Perhaps a method withAutoFlush(boolean) could reduce the number of


To make that work, we would have to make the boolean field "autoFlush" 
writeable. Also the auto flush behaviour could be changed at any time. I 
would then prefer to only have something like a one-way-street as this:


withAutoFlush() intentionally no argument to only change it to enabled.



constructors? And perhaps the String filename is an anachronism?

Thus, adding these three methods might be enough:

PrintWriter(OutputStream, Charset)
PrintWriter(File, Charset)
withAutoFlush(boolean)

Stephen




I would think, this would be a good change.

-Patrick

Re: Request/discussion: BufferedReader reading using async API while providing sync API

[Brunoais]

Hey! Is this forgotten?


On 03/11/2016 14:35, Brunoais wrote:

Any information you can give on this?


On 29/10/2016 19:10, Brunoais wrote:

Here's my try on going async.

On my tests on my local windows 10 machine (I don't have access to 
the linux one without a VM ATM) , now with 1GB file, I noticed:


~2s of speed improvement from BufferedInputStream to 
BufferedNonBlockStream when BufferedNonBlockStream is at its most 
advantageous state (with CPU work between reads). Otherwise, it has 
~0.3s speed improvement; maybe less.


~0.8s of speed improvement from BufferedNonBlockStream to 
BufferedAsyncStream when BufferedNonBlockStream/BufferedAsyncStream 
is at its most advantageous state. Otherwise, ~0 speed improvement.


I noticed: Until you process as fast as you read, both 
BufferedNonBlockStream and BufferedAsyncStream gain advantage towards 
BufferedInputStream. From the point as you take longer to process 
than to I/O, BufferedNonBlockStream and BufferedAsyncStream tend to 
keep the advantage.


If the file is in cache, BufferedInputStream takes ~1.6-1.9s to read 
the file, BufferedNonBlockStream takes a steady ~2.05-2.1s to read 
the file and BufferedAsyncStream ~1.2s.


BufferedNonBlockStream and BufferedAsyncStream win more when given 
more buffer memory than BufferedInputStream but only until a certain 
limit. On my hardware, the best speed I got was with 655360B for the 
new ones. Any more than that was not producing any visible results. I 
guess it is due to the speed data was processing for the test.


On 28/10/2016 09:16, Brunoais wrote:
I'll try going back to a previous version I worked on which used the 
java7's AsynchronousFileChannel and work from there. My small 
research shows it can also work with AsynchronousFileChannel mostly 
without changes.


For now, 1 question:
Is Thread.sleep() a possible way of dealing the block requirements 
of read()? Do I need to use LockSupport.park() or something like that?


I'll call back here when it is done.


On 27/10/2016 22:09, David Holmes wrote:
You might try discussing on net-dev rather than core-libs-dev, to 
get additional historical info related to the io and nio file APIs.


David

On 28/10/2016 5:08 AM, Brunoais wrote:

You are right. Even in windows it does not set the flags for async
reads. It seems like it is windows itself that does the decision to
buffer the contents based on its own heuristics.

But... Why? Why won't it be? Why is there no API for it? How am I
getting 100% HDD use and faster times when I fake work to delay 
getting

more data and I only have a fluctuating 60-90% (always going up and
down) when I use an InputStream?
Is it related to how both classes cache and how frequently and how 
much

each one asks for data?

I really would prefer not having to read the source code because it
takes a real long time T.T.

I end up reinstating... And wondering...

Why doesn't java provide a single-threaded non-block API for file 
reads

for all OS that support it? I simply cannot find that information no
matter how much I search on google, bing, duck duck go... Can any 
of you

point me to whomever knows?

On 27/10/2016 14:11, Vitaly Davidovich wrote:

I don't know about Windows specifically, but generally file systems
across major OS's will implement readahead in their IO scheduler 
when

they detect sequential scans.

On Linux, you can also strace your test to confirm which syscalls 
are

emitted (you should be seeing plain read()'s there, with
FileInputStream and FileChannel).

On Thu, Oct 27, 2016 at 9:06 AM, Brunoais > wrote:

Thanks for the heads up.

I'll try that later. These tests are still useful then. 
Meanwhile,

I'll end up also checking how FileChannel queries the OS on
windows. I'm getting 100% HDD reads... Could it be that the OS
reads the file ahead on its own?... Anyway, I'll look into it.
Thanks for the heads up.


On 27/10/2016 13:53, Vitaly Davidovich wrote:



On Thu, Oct 27, 2016 at 8:34 AM, Brunoais > wrote:

Oh... I see. In that case, it means something is terribly
wrong. It can be my initial tests, though.

I'm testing on both linux and windows and I'm getting
performance gains from using the FileChannel compared to
using FileInputStream... The tests also make sense based on
my predictions O_O...

FileInputStream requires copying native buffers holding the 
read

data to the java byte[].  If you're using direct ByteBuffer for
FileChannel, that whole memcpy is skipped.  Try comparing
FileChannel with HeapByteBuffer instead.


On 27/10/2016 11:47, Vitaly Davidovich wrote:



On Thursday, October 27, 2016, Brunoais 
> wrote:

Did you read the C code?

I looked at the Linux code in the JDK.

Have you got any 

Re: RFR: JDK-8167648: java.io.PrintWriter should have PrintWriter((String|File), Charset) constructors

[Stephen Colebourne]

These are the current constructors:

PrintWriter(Writer)
PrintWriter(Writer, boolean)
PrintWriter(OutputStream)
PrintWriter(OutputStream, boolean)
PrintWriter(String)
PrintWriter(String, String)
PrintWriter(File)
PrintWriter(File, String)

These are the annoying missing ones (not all of the possible combinations):

PrintWriter(OutputStream, Charset)
PrintWriter(OutputStream, Charset, boolean)
PrintWriter(String, Charset)
PrintWriter(String, Charset, boolean)
PrintWriter(File, Charset)
PrintWriter(File, Charset, boolean)

there are other missing Charset methods on FileWriter.

Perhaps a method withAutoFlush(boolean) could reduce the number of
constructors? And perhaps the String filename is an anachronism?

Thus, adding these three methods might be enough:

PrintWriter(OutputStream, Charset)
PrintWriter(File, Charset)
withAutoFlush(boolean)

Stephen


On 23 November 2016 at 09:52, Patrick Reinhart  wrote:
> Are there any obligations to add those constructors?
>
> -Patrick
>
> On 2016-11-18 10:19, Patrick Reinhart wrote:
>
>> I was looking at the existing JDK 9 issues for some simple ones I
>> could solve and found this one. I wanted to know if it makes sense to
>> add additional constructors here?
>>
>> Now you need to do this:
>> 
>> try {
>>   new PrintWriter(file, "UTF-8");
>> } catch (UnsupportedEncodingException e) {
>>   // Ignore, this is required to be supported by the JVM.
>> }
>>
>> The same applies also to the String constructor...
>>
>>
>>
>> Instead the following behaviour is requested:
>> -
>> new PrintWriter(file, StandardCharsets.UTF_8));
>>
>>
>>
>> On the other hand then the next request will be to add constructors
>> also to specify autoflush and so on...
>>
>> -Patrick

RFR: JDK-8167648: java.io.PrintWriter should have PrintWriter((String|File), Charset) constructors

[Patrick Reinhart]

Are there any obligations to add those constructors?

-Patrick

On 2016-11-18 10:19, Patrick Reinhart wrote:


I was looking at the existing JDK 9 issues for some simple ones I
could solve and found this one. I wanted to know if it makes sense to
add additional constructors here?

Now you need to do this:

try {
  new PrintWriter(file, "UTF-8");
} catch (UnsupportedEncodingException e) {
  // Ignore, this is required to be supported by the JVM.
}

The same applies also to the String constructor...



Instead the following behaviour is requested:
-
new PrintWriter(file, StandardCharsets.UTF_8));



On the other hand then the next request will be to add constructors
also to specify autoflush and so on...

-Patrick

RFR of JDK-8019538: TEST_BUG: java/rmi/activation/rmidViaInheritedChannel tests may fail

[Hamlin Li]

Would you please review the fix for below bug?

bug: https://bugs.openjdk.java.net/browse/JDK-8019538
webrev: http://cr.openjdk.java.net/~mli/8019538/webrev.00/

There are 4 issues in the bug,
2 in RmidViaInheritedChannel.java:  "port in use" in registry, "port in 
use" in rmid start.
2 InheritedChannelNotServerSocket.java: "port in use" in registry, "port 
in use" in rmid start.


This patch fixes 2 issues in RmidViaInheritedChannel, and only "port in 
use" in registry in InheritedChannelNotServerSocket.
The "port in use" in rmid in InheritedChannelNotServerSocket is little 
bit hard, as it intends to test rmid when inherited channel not work. 
Currently the only solution in my mind is to retry when rmid fails with 
"port in use", but as we discussed earlier, it's not a good solution as 
it might impact other programs or tests, and it's not efficient.
So I hope to push the fix for the other issues first to improve the 
stability of RMI tests, and keep studying if there are other better 
solutions for the "port in use" in rmid in InheritedChannelNotServerSocket.


Thank you
-Hamlin

Re: [9] RFR (XS): 8169000: Define reference reachability more precisely in java.lang.ref package

[Zoltán Majó]

Hi,


Mandy and I have agreed in a personal discussion that Mandy will take 
this issue and target it for a future issue.


So I retract this RFR. Thank you, everybody, for the discussions and help!

Best regards,


Zoltan

Did the tests fail due to TestNG dataprovider parameter type check?

[Frank Yuan]

Hi Jon

I run the whole jdk regression tests with jtreg-4.2-b03 [1], and then found 
lots of tests in different test groups failed with the error message like 
following:
test test.java.time.format.TestNumberPrinter.test_pad_ALWAYS(): failure
org.testng.internal.reflect.MethodMatcherException: 
Data provider mismatch
Method: test_pad_ALWAYS([Parameter{index=0, type=int, declaredAnnotations=[]}, 
Parameter{index=1, type=int, declaredAnnotations=[]}, Parameter{index=2, 
type=long, declaredAnnotations=[]}, Parameter{index=3, type=java.lang.String, 
declaredAnnotations=[]}])
Arguments: 
[(java.lang.Integer)1,(java.lang.Integer)1,(java.lang.Integer)-10,null]
at 
org.testng.internal.reflect.DataProviderMethodMatcher.getConformingArguments(DataProviderMethodMatcher.java:52)
at org.testng.internal.Invoker.injectParameters(Invoker.java:1228)
at org.testng.internal.Invoker.invokeTestMethods(Invoker.java:1125)
at 
org.testng.internal.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:129)
at org.testng.internal.TestMethodWorker.run(TestMethodWorker.java:112)
at org.testng.TestRunner.privateRun(TestRunner.java:778)
at org.testng.TestRunner.run(TestRunner.java:632)
at org.testng.SuiteRunner.runTest(SuiteRunner.java:366)
at org.testng.SuiteRunner.runSequentially(SuiteRunner.java:361)
at org.testng.SuiteRunner.privateRun(SuiteRunner.java:319)
at org.testng.SuiteRunner.run(SuiteRunner.java:268)
at org.testng.SuiteRunnerWorker.runSuite(SuiteRunnerWorker.java:52)
at org.testng.SuiteRunnerWorker.run(SuiteRunnerWorker.java:86)
at org.testng.TestNG.runSuitesSequentially(TestNG.java:1225)
at org.testng.TestNG.runSuitesLocally(TestNG.java:1150)
at org.testng.TestNG.runSuites(TestNG.java:1075)
at org.testng.TestNG.run(TestNG.java:1047)
at 
com.sun.javatest.regtest.TestNGAction$TestNGRunner.main(TestNGAction.java:220)
at 
com.sun.javatest.regtest.TestNGAction$TestNGRunner.main(TestNGAction.java:184)
at 
java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at 
java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at 
java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.base/java.lang.reflect.Method.invoke(Method.java:537)
at 
com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapper.java:110)
at java.base/java.lang.Thread.run(Thread.java:844)


This test methd signature is:
 public void test_pad_ALWAYS(int minPad, int maxPad, long value, String result)

The provider return the following data
Object[][] provider_pad() {
return new Object[][] {
{1, 1, -10, null},
{1, 1, -9, "9"},
{1, 1, -1, "1"},
...

However, I got message "Cannot find class 
java/lang/reflect/JTRegModuleHelper.class to init patch directory" when I run 
jtreg, although I didn't find any code related to testng dataprovider in jtreg 
source, I would double confirm with you if this is a definite issue or anything 
I made incorrectly?

To Christoph

Except above issue, I didn't find any other issue in jdk and langtools repo so 
far.


[1] 
https://adopt-openjdk.ci.cloudbees.com/job/jtreg/lastSuccessfulBuild/artifact/jtreg-4.2-b03.tar.gz


Thanks
Frank

> -Original Message-
> From: Langer, Christoph [mailto:christoph.lan...@sap.com]
> Sent: Wednesday, November 23, 2016 3:41 PM
> To: Frank Yuan
> Cc: code-tools-...@openjdk.java.net; core-libs-dev@openjdk.java.net; 
> jtreg-...@openjdk.java.net; 'Volker Simonis'; 'Daniel Fuchs'
> Subject: RE: Issues running JAXP jtreg tests ("java.lang.RuntimePermission" 
> "accessDeclaredMembers")
> 
> Thanks, Frank.
> 
> we run scheduled jtreg tests for jdk every night so we should have 
> encountered issues if there were some. But langtools could be interesting, I
> don't think those run automatically for OpenJDK in our environment.
> 
> Best regards
> Christoph
> 
> > -Original Message-
> > From: Frank Yuan [mailto:frank.y...@oracle.com]
> > Sent: Mittwoch, 23. November 2016 06:26
> > To: Langer, Christoph ; 'Volker Simonis'
> > ; 'Daniel Fuchs' 
> > Cc: code-tools-...@openjdk.java.net; core-libs-dev@openjdk.java.net; jtreg-
> > u...@openjdk.java.net
> > Subject: RE: Issues running JAXP jtreg tests ("java.lang.RuntimePermission"
> > "accessDeclaredMembers")
> >
> > Hi Christoph and Volker
> >
> > I have been launching jdk and langtools tests with the new jtreg, will 
> > update to
> > you once I get the result.
> > Hope jaxp test is special because most of tests should control the Security
> > Manager setting inside the test methods.
> >
> > Thanks
> > Frank
> >
> >
> >
> > > -Original Message-
> > > From: core-libs-dev 

RE: RFR: 8169772: [JAXP] XALAN: Transformation of DOM with null valued text node causes NPE

[Langer, Christoph]

Hi Joe,

tests went well and I pushed: 
http://hg.openjdk.java.net/jdk9/dev/jaxp/rev/31ac7aab52da

Thanks
Christoph

From: Joe Wang [mailto:huizhe.w...@oracle.com]
Sent: Dienstag, 22. November 2016 23:02
To: Langer, Christoph 
Cc: core-libs-dev@openjdk.java.net
Subject: Re: RFR: 8169772: [JAXP] XALAN: Transformation of DOM with null valued 
text node causes NPE

Hi Christoph,

That looks good to me.

Best,
Joe

On 11/22/16, 1:26 PM, Langer, Christoph wrote:
Hi Joe,

as my jtreg issues are solved now, I'm finalizing the patches.

For this one I've updated the test method: 
http://cr.openjdk.java.net/~clanger/webrevs/8169772.2/

Could you please quickly check if my new test method "testBug8169772()" looks 
as you want it? I've added some more detailed comments for the method. The 
point is that without the fix you'd encounter an NPE. Or should there be some 
special assertion around it?

If it's ok for you, I'd push tomorrow.

Thanks & Best regards
Christoph

From: Joe Wang [mailto:huizhe.w...@oracle.com]
Sent: Samstag, 19. November 2016 00:23
To: Langer, Christoph 

Cc: core-libs-dev@openjdk.java.net
Subject: Re: RFR: 8169772: [JAXP] XALAN: Transformation of DOM with null valued 
text node causes NPE



On 11/18/16, 12:17 PM, Langer, Christoph wrote:
Hi Joe,

Yep, I'll conduct the testing before pushing.

With the "Expected Result", you mean some comment for the test method, right?

Yes, some comment for the test method would do. But I see that you've added 
assertions? That would be even better. Comments help in this case since without 
the patch, the process would have thrown an NPE.

Once a bug is fixed, SQE will take the new test and verify the fix. Knowing the 
success/fail (before and after fix) conditions is helpful for them.

Thanks,
Joe




Thanks
Christoph

From: Joe Wang [mailto:huizhe.w...@oracle.com]
Sent: Freitag, 18. November 2016 20:00
To: Langer, Christoph 

Cc: core-libs-dev@openjdk.java.net
Subject: Re: RFR: 8169772: [JAXP] XALAN: Transformation of DOM with null valued 
text node causes NPE

Looks good, Christoph.

I assume you'll do an all-test run (all in jaxp/test) before pushing.  No need 
for further review, but it'd be nice to add an "Expected result" for the new 
test testBug8169772, with/without the fix (e.g. NPE).

Best regards,
Joe

On 11/18/16, 4:38 AM, Langer, Christoph wrote:
Hi Joe,

thanks for the feedback.

I've created a new version of the webrev working in your suggestions, adding 
some further formatting cleanups in the files and I also moved a small 
refactoring in TransformerTest.java to this changeset.

http://cr.openjdk.java.net/~clanger/webrevs/8169772.1/

>From my end this one is ready for pushing - waiting for your final go.

Best regards
Christoph


From: Joe Wang [mailto:huizhe.w...@oracle.com]
Sent: Freitag, 18. November 2016 07:36
To: Langer, Christoph 

Cc: core-libs-dev@openjdk.java.net
Subject: Re: RFR: 8169772: [JAXP] XALAN: Transformation of DOM with null valued 
text node causes NPE

Looks fine.

License header: in general, don't change / add Year if there's no material 
change, removing the legacy $Id field in EmptySerializer.java for example, does 
not constitute a change to the code, so just keep the original year (see below).

 The initial years for the classes:
EmptySerializer.java 2012
SerializerBase.java 2012
ToSAXHandler.java none (meaning if you make changes to this class, 
just add 2016)
ToStream.java 2006
ToUnknownStream.java 2007
XSLOutputAttributes.java none (so keep the original "DO NOT REMOVE 
OR ALTER!" block)

Thanks,
Joe

On 11/16/16, 6:22 AM, Langer, Christoph wrote:

Hi,



please review another XALAN fix.



The Serializer should be able to handle text nodes with null input. There has 
already been some discussion here: 
http://mail.openjdk.java.net/pipermail/core-libs-dev/2016-November/044567.html



Bug: https://bugs.openjdk.java.net/browse/JDK-8169772

Webrev: 
http://cr.openjdk.java.net/~clanger/webrevs/8169772.0/



The actual fix is in ToUnknownStream.java, method "public void 
characters(String chars) throws SAXException". I don't know if one should even 
directly return after chars being null or of size() 0. The rest of this change 
is cleanups and a test case.



Thanks for reviewing.



Best regards

Christoph