Re: RFR: JDK-8197594: String#repeat

2018-02-28 Thread Andrej Golovnin
Hi Jim,

src/java.base/share/classes/java/lang/String.java

2972  * This method may be used to create space padding for
2973  * formatting text or zero padding for formatting numbers.
2974  * @param   count number of times to repeat
2975  * @return  A string composed of this string repeated
2976  *  {@code count} times or the empty string if count
2977  *  or length is zero.
2978  * @throws  IllegalArgumentException if the {@code count} is
2979  *  negative.

I don't know whether the JavaDoc style guide requires it or not, but I
would put an empty line before @param, @return and @ throws to
improved readability.

2983 throw new IllegalArgumentException("count is
negative, " + count);

"count is negative, " -> "count is negative: "

test/jdk/java/lang/String/StringRepeat.java

102 static void VERIFY(String result, String string, int repeat) {

Why is the method name in uppercase?

Best regards,
Andrej Golovnin

On Wed, Feb 28, 2018 at 5:31 PM, Jim Laskey  wrote:
> Introduction of a new instance method String::repeat to allow an efficient 
> and concise approach for generating repeated character sequences as strings.
>
> Performance information in JBS.
>
> Thank you.
>
> Cheers,
>
> — Jim
>
>
> JBS: https://bugs.openjdk.java.net/browse/JDK-8197594 
> 
> CSR: https://bugs.openjdk.java.net/browse/JDK-8198296
>  
> Webrev: http://cr.openjdk.java.net/~jlaskey/8197594/webrev-02/index.html 
> 
> JavaDoc: http://cr.openjdk.java.net/~jlaskey/8197594/String.html 
> 
>
>
>


Re: Reactive Streams utility API

2018-02-28 Thread James Roper
Hi all,

We've put together a simple proposal for this. Please read the README for
an introduction to this proposal.

https://github.com/lightbend/reactive-streams-utils

Regards,

James

On 22 February 2018 at 11:47, James Roper  wrote:

> Hi all,
>
> This is an email to give people a heads up that we'd like to look at
> creating an API, in the same vein as the JDK8 Streams API, for building
> reactive streams (a la JDK9 juc.Flow). Our goals for this are:
>
> * To fill a gap in the JDK where if a developer wants to do even the
> simplest of things with a JDK9 juc.Flow, such as map or filter, they need
> to bring in a third party library that implements that.
> * To produce an API that can build Publishers, Subscribers, Processors,
> and complete graphs, for the purposes of consuming APIs that use reactive
> streams (for example, JDK9 Http Client).
> * To produce an API that aligns closely with ju.stream.Stream, using it
> for inspiration for naming, scope, general API shape, and other aspects.
> The purpose of this goal is to ensure familiarity of Java developers with
> the new API, and to limit the number of concepts Java developers need to
> understand to do the different types of streaming offered by the JDK.
> * To produce an API that can be implemented by multiple providers
> (including an RI in the JDK itself), using the ServiceLoader mechanism to
> provide and load a default implementation (while allowing custom
> implementations to be manually provided). There are a lot of concerns that
> each different streams implementation provides and implements, beyond
> streaming, for example monitoring/tracing, concurrency modelling, buffering
> strategies, performance aspects of the streams handling including fusing,
> and context (eg thread local) propagation. This will allow libraries to use
> and provide contracts based on this API without depending on a particular
> implementation, and allows developers to select the implementation that
> meets their needs.
>
> Non goals:
>
> * To produce a kitchen sink of utilities for working with reactive
> streams. There already exist a number of reactive streams implementations
> that seek to meet this goal (eg, Akka Streams, Reactor, RxJava), and once
> you go past the basics (map, filter, collect), and start dealing with
> things like fan in/out, cycles, restarting, etc, the different approaches
> to solving this start to vary greatly. The JDK should provide enough to be
> useful for typical every day streaming use cases, with developers being
> able to select a third party library for anything more advanced.
>
> We will update this list when we have something ready for public review.
> This probably won't be far off. Our hope is that we can propose this as a
> JEP.
>
> Regards,
>
> James
>
> --
> *James Roper*
> *Senior Octonaut*
>
> Lightbend  – Build reactive apps!
> Twitter: @jroper 
>



-- 
*James Roper*
*Senior Octonaut*

Lightbend  – Build reactive apps!
Twitter: @jroper 


Re: RFR: JDK-8197594: String#repeat

2018-02-28 Thread James Laskey
Thanks Stuart. RE apinote, I was trying to follow the pattern of other older 
method comments (Roman-style.) Comments/Javadoc in most of these older classes 
are a mix of styles. Question: if you update/clean-up a method in an older 
class, should you bring the comment/Javadoc up-to-date as well?

Cheers,

— Jim

Sent from my iPhone

> On Feb 28, 2018, at 6:21 PM, Stuart Marks  wrote:
> 
> Hi Jim,
> 
> Implementation looks good. I'd suggest a couple small editorial changes to 
> the spec:
> 
> 2966 /**
> 2967  * Returns a string whose value is the concatenation of this
> 2968  * string repeated {@code count} times.
> 2969  * 
> 2970  * If count or length is zero then the empty string is returned.
> 
> I went looking for a 'length' parameter, until I realized it means the length 
> of *this string*. So maybe clarify that. (And also line 2977.)
> 
> 2971  * 
> 2972  * This method may be used to create space padding for
> 2973  * formatting text or zero padding for formatting numbers.
> 
> I don't think having this text is necessary, but if you want it, I'd suggest 
> putting it into an @apiNote.
> 
> 2974  * @param   count number of times to repeat
> 2975  * @return  A string composed of this string repeated
> 2976  *  {@code count} times or the empty string if count
> 2977  *  or length is zero.
> 
> Thanks,
> 
> s'marks
> 
> 
>> On 2/28/18 8:31 AM, Jim Laskey wrote:
>> Introduction of a new instance method String::repeat to allow an efficient 
>> and concise approach for generating repeated character sequences as strings.
>> Performance information in JBS.
>> Thank you.
>> Cheers,
>> — Jim
>> JBS: https://bugs.openjdk.java.net/browse/JDK-8197594 
>> 
>> CSR: https://bugs.openjdk.java.net/browse/JDK-8198296
>>  
>> Webrev: http://cr.openjdk.java.net/~jlaskey/8197594/webrev-02/index.html 
>> 
>> JavaDoc: http://cr.openjdk.java.net/~jlaskey/8197594/String.html 
>> 



Re: RFR: JDK-8197594: String#repeat

2018-02-28 Thread James Laskey
Thanks Paul. 

Sent from my iPhone

> On Feb 28, 2018, at 10:13 PM, Paul Sandoz  wrote:
> 
> Hi Jim,
> 
> Looks good. I like the power of 2 copying.
> 
> 
> 2978  * @throws  IllegalArgumentException if the {@code count} is
> 2979  *  negative.
> 2980  */
> 2981 public String repeat(int count) {
> 
> Missing @since11 on the method.
> 
> 
> Like Stuart suggests, turn the explanatory text into an api note, perhaps 
> with a small code sample.
> 
> Thanks,
> Paul.
> 
> 
>> On Feb 28, 2018, at 8:31 AM, Jim Laskey  wrote:
>> 
>> Introduction of a new instance method String::repeat to allow an efficient 
>> and concise approach for generating repeated character sequences as strings.
>> 
>> Performance information in JBS.
>> 
>> Thank you.
>> 
>> Cheers,
>> 
>> — Jim
>> 
>> 
>> JBS: https://bugs.openjdk.java.net/browse/JDK-8197594 
>> 
>> CSR: https://bugs.openjdk.java.net/browse/JDK-8198296
>> 
>> Webrev: http://cr.openjdk.java.net/~jlaskey/8197594/webrev-02/index.html 
>> 
>> JavaDoc: http://cr.openjdk.java.net/~jlaskey/8197594/String.html 
>> 
>> 
>> 
>> 
> 



Re: RFR: JDK-8197594: String#repeat

2018-02-28 Thread Paul Sandoz
Hi Jim,

Looks good. I like the power of 2 copying.


2978  * @throws  IllegalArgumentException if the {@code count} is
2979  *  negative.
2980  */
2981 public String repeat(int count) {

Missing @since11 on the method.


Like Stuart suggests, turn the explanatory text into an api note, perhaps with 
a small code sample.

Thanks,
Paul.
 

> On Feb 28, 2018, at 8:31 AM, Jim Laskey  wrote:
> 
> Introduction of a new instance method String::repeat to allow an efficient 
> and concise approach for generating repeated character sequences as strings.
> 
> Performance information in JBS.
> 
> Thank you.
> 
> Cheers,
> 
> — Jim
> 
> 
> JBS: https://bugs.openjdk.java.net/browse/JDK-8197594 
> 
> CSR: https://bugs.openjdk.java.net/browse/JDK-8198296
> 
> Webrev: http://cr.openjdk.java.net/~jlaskey/8197594/webrev-02/index.html 
> 
> JavaDoc: http://cr.openjdk.java.net/~jlaskey/8197594/String.html 
> 
> 
> 
> 



[JDK 11] RFR 8198821: fix test methods access for test java/text/Normalizer/NormalizerAPITest.java

2018-02-28 Thread Chris Yin
Please review the minor change for test 
java/text/Normalizer/NormalizerAPITest.java, thanks

Added public access modifier to all “Test_" methods so they can be recognized 
as test method correctly by util class

bug: https://bugs.openjdk.java.net/browse/JDK-8198821 

webrev: http://cr.openjdk.java.net/~xiaofeya/8198821/webrev.00/ 


Regards,
Chris

Re: RFR JDK-8197533 move javax.transaction.xa into its own module

2018-02-28 Thread Paul Sandoz


> On Feb 28, 2018, at 1:22 PM, Lance Andersen  wrote:
> 
>> 
>> On Feb 28, 2018, at 2:20 PM, Lance Andersen > > wrote:
>> 
>> Hi Paul,
>> 
>> Thank you for the review.
>>> On Feb 28, 2018, at 1:40 PM, Paul Sandoz >> > wrote:
>>> 
>>> Compatible module refactoring in action!
>>> 
>>> Looks good, one comment:
>>> 
>>> test/jdk/javax/transaction/xa/testng/JavaSqlModuleDriver.java
>>> 
>>> This is not a valid Java source file can you merge the jtreg meta data into 
>>> XAExceptionTests instead?
>> 
>> As we discussed offline, I will change the above file and Driver.java.  I 
>> naively assumed this was OK as the change  to add Driver.java was made after 
>> I had originally added these tests in 2015.
> 
> http://cr.openjdk.java.net/~lancea/8197533/webrev.01/ 
>  has the updated tests

+1, i second Joe’s request to update package-info.java while we are 
opportunistically cleaning this area up.

Paul.

Re: RFR 7183985: Class.getAnnotation() throws an ArrayStoreException when the annotation class not present

2018-02-28 Thread Martin Buchholz
I was surprised to learn that junit has a magic assertEquals specifically
for Object[]
assertEquals(Object[], Object[])
but that is obviously a bad idea and junit eventually deprecated it.
https://junit.org/junit4/javadoc/4.12/org/junit/Assert.html#assertEquals(java.lang.Object[],
java.lang.Object[])
Follow their lead and rename your method to assertArrayEquals

 124 static void assertEquals(Object[] actual, Object[] expected) {


Re: RFR: Here are some URLClassPath patches

2018-02-28 Thread Martin Buchholz
OK, that was weird...
All I did for testing was follow

// This section should be uncommented if 8026517 is fixed.


but ...  8026517 is marked fixed!

So switching to ArrayDeque accidentally fixed 8026517 for real?!

8198810: URLClassLoader does not specify behavior when URL array contains
null
http://cr.openjdk.java.net/~martin/webrevs/jdk/URLClassLoader-NPE/
https://bugs.openjdk.java.net/browse/JDK-8198810


Re: RFR: JDK-8197594: String#repeat

2018-02-28 Thread Stuart Marks

Hi Jim,

Implementation looks good. I'd suggest a couple small editorial changes to the 
spec:

2966 /**
2967  * Returns a string whose value is the concatenation of this
2968  * string repeated {@code count} times.
2969  * 
2970  * If count or length is zero then the empty string is returned.

I went looking for a 'length' parameter, until I realized it means the length of 
*this string*. So maybe clarify that. (And also line 2977.)


2971  * 
2972  * This method may be used to create space padding for
2973  * formatting text or zero padding for formatting numbers.

I don't think having this text is necessary, but if you want it, I'd suggest 
putting it into an @apiNote.


2974  * @param   count number of times to repeat
2975  * @return  A string composed of this string repeated
2976  *  {@code count} times or the empty string if count
2977  *  or length is zero.

Thanks,

s'marks


On 2/28/18 8:31 AM, Jim Laskey wrote:

Introduction of a new instance method String::repeat to allow an efficient and 
concise approach for generating repeated character sequences as strings.

Performance information in JBS.

Thank you.

Cheers,

— Jim


JBS: https://bugs.openjdk.java.net/browse/JDK-8197594 

CSR: https://bugs.openjdk.java.net/browse/JDK-8198296
  
Webrev: http://cr.openjdk.java.net/~jlaskey/8197594/webrev-02/index.html 

JavaDoc: http://cr.openjdk.java.net/~jlaskey/8197594/String.html 






Re: RFR JDK-8197533 move javax.transaction.xa into its own module

2018-02-28 Thread joe darcy

Hi Lance,

I'd prefer if 
src/java.sql/share/classes/javax/transaction/xa/package.html was 
replaced by a package-info.java files as opposed to another package.html 
file in the new module.


Thanks,

-Joe


On 2/28/2018 1:22 PM, Lance Andersen wrote:

On Feb 28, 2018, at 2:20 PM, Lance Andersen  wrote:

Hi Paul,

Thank you for the review.

On Feb 28, 2018, at 1:40 PM, Paul Sandoz mailto:paul.san...@oracle.com>> wrote:

Compatible module refactoring in action!

Looks good, one comment:

test/jdk/javax/transaction/xa/testng/JavaSqlModuleDriver.java

This is not a valid Java source file can you merge the jtreg meta data into 
XAExceptionTests instead?

As we discussed offline, I will change the above file and Driver.java.  I 
naively assumed this was OK as the change  to add Driver.java was made after I 
had originally added these tests in 2015.

http://cr.openjdk.java.net/~lancea/8197533/webrev.01/ has the updated tests
Best
Lance

Best
Lance

Paul.


On Feb 28, 2018, at 10:25 AM, Lance Andersen mailto:lance.ander...@oracle.com>>> wrote:

Hi all,

This RFR request moves the javax.transaction.xa package out of the java.sql 
module and into its own module java.transaction.xa.  One of the motivators for 
this change is due to the fact JSR 907 1.3 MR indicated that the 
javax.transaction.xa package will be subsumed by Java SE.

There should be no compatibility issues with this change. Any module that 
`requires java.sql` will continue to have access to the public classes in the 
javax.transaction.xa package at both compile-time and run-time.


The CSR has been approved

The webrev can be found at: 
http://cr.openjdk.java.net/~lancea/8197533/webrev.00/

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


 

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

  
   

  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 JDK-8197533 move javax.transaction.xa into its own module

2018-02-28 Thread Magnus Ihse Bursie


On 2018-02-28 19:25, Lance Andersen wrote:

Hi all,

This RFR request moves the javax.transaction.xa package out of the java.sql 
module and into its own module java.transaction.xa.  One of the motivators for 
this change is due to the fact JSR 907 1.3 MR indicated that the 
javax.transaction.xa package will be subsumed by Java SE.

There should be no compatibility issues with this change. Any module that 
`requires java.sql` will continue to have access to the public classes in the 
javax.transaction.xa package at both compile-time and run-time.


The CSR has been approved


The webrev can be found at: 
http://cr.openjdk.java.net/~lancea/8197533/webrev.00/

Build changes look good.

/Magnus


Best
Lance
  
   

  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 JDK-8197533 move javax.transaction.xa into its own module

2018-02-28 Thread Lance Andersen

> On Feb 28, 2018, at 2:20 PM, Lance Andersen  wrote:
> 
> Hi Paul,
> 
> Thank you for the review.
>> On Feb 28, 2018, at 1:40 PM, Paul Sandoz > > wrote:
>> 
>> Compatible module refactoring in action!
>> 
>> Looks good, one comment:
>> 
>> test/jdk/javax/transaction/xa/testng/JavaSqlModuleDriver.java
>> 
>> This is not a valid Java source file can you merge the jtreg meta data into 
>> XAExceptionTests instead?
> 
> As we discussed offline, I will change the above file and Driver.java.  I 
> naively assumed this was OK as the change  to add Driver.java was made after 
> I had originally added these tests in 2015.

http://cr.openjdk.java.net/~lancea/8197533/webrev.01/ has the updated tests
Best
Lance
> 
> Best
> Lance
>> 
>> Paul.
>> 
>>> On Feb 28, 2018, at 10:25 AM, Lance Andersen >> >> >> wrote:
>>> 
>>> Hi all,
>>> 
>>> This RFR request moves the javax.transaction.xa package out of the java.sql 
>>> module and into its own module java.transaction.xa.  One of the motivators 
>>> for this change is due to the fact JSR 907 1.3 MR indicated that the 
>>> javax.transaction.xa package will be subsumed by Java SE.
>>> 
>>> There should be no compatibility issues with this change. Any module that 
>>> `requires java.sql` will continue to have access to the public classes in 
>>> the javax.transaction.xa package at both compile-time and run-time. 
>>> 
>>> 
>>> The CSR has been approved
>>> 
>>> The webrev can be found at: 
>>> http://cr.openjdk.java.net/~lancea/8197533/webrev.00/
>>> 
>>> Best
>>> Lance
>>> >>  
>>> >> >>
>>> >>  
>>> >> >> 
>>> >>  
>>> >> >>
>>> >>  
>>> >> >>Lance Andersen| 
>>> Principal Member of Technical Staff | +1.781.442.2037
>>> Oracle Java Engineering 
>>> 1 Network Drive 
>>> Burlington, MA 01803
>>> lance.ander...@oracle.com  
>>> > 
>>> >> >> >>
> 
> 
>  
> 
> Lance Andersen| 
> Principal Member of Technical Staff | +1.781.442.2037
> Oracle Java Engineering 
> 1 Network Drive 
> Burlington, MA 01803
> lance.ander...@oracle.com 

 
  

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





RFR 8196298 Add null Reader and Writer

2018-02-28 Thread Patrick Reinhart
Hi every boy that reviewed

I tried to incorporate all feedback I received so far and updated the webrev 
accordingly:

http://cr.openjdk.java.net/~reinhapa/reviews/8196298/webrev.01

-Patrick


Re: RFR JDK-8197533 move javax.transaction.xa into its own module

2018-02-28 Thread Lance Andersen

> On Feb 28, 2018, at 2:43 PM, Alan Bateman  wrote:
> 
> On 28/02/2018 18:25, Lance Andersen wrote:
>> Hi all,
>> 
>> This RFR request moves the javax.transaction.xa package out of the java.sql 
>> module and into its own module java.transaction.xa.  One of the motivators 
>> for this change is due to the fact JSR 907 1.3 MR indicated that the 
>> javax.transaction.xa package will be subsumed by Java SE.
>> 
>> There should be no compatibility issues with this change. Any module that 
>> `requires java.sql` will continue to have access to the public classes in 
>> the javax.transaction.xa package at both compile-time and run-time.
>> 
>> 
> I skipped the tests but everything else looks good. It is a compatible change 
> as you noted.
Thank you Alan
> 
> Is there any XA text from the original JTA spec that should be added to the 
> module description as part of this? Another way to ask this is whether the 
> JTA 1.3 drops any text dealing with the XA part.
Still waiting to see what changes are made to the PDF spec, that is still 
needing to be completed. So I think for now, we go with what we have and can 
circle back if needed.

Best
Lance
> 
> -Alan

 
  

 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 JDK-8197533 move javax.transaction.xa into its own module

2018-02-28 Thread Alan Bateman

On 28/02/2018 18:25, Lance Andersen wrote:

Hi all,

This RFR request moves the javax.transaction.xa package out of the java.sql 
module and into its own module java.transaction.xa.  One of the motivators for 
this change is due to the fact JSR 907 1.3 MR indicated that the 
javax.transaction.xa package will be subsumed by Java SE.

There should be no compatibility issues with this change. Any module that 
`requires java.sql` will continue to have access to the public classes in the 
javax.transaction.xa package at both compile-time and run-time.


I skipped the tests but everything else looks good. It is a compatible 
change as you noted.


Is there any XA text from the original JTA spec that should be added to 
the module description as part of this? Another way to ask this is 
whether the JTA 1.3 drops any text dealing with the XA part.


-Alan


Re: RFR JDK-8197533 move javax.transaction.xa into its own module

2018-02-28 Thread Lance Andersen
Hi Paul,

Thank you for the review.
> On Feb 28, 2018, at 1:40 PM, Paul Sandoz  wrote:
> 
> Compatible module refactoring in action!
> 
> Looks good, one comment:
> 
> test/jdk/javax/transaction/xa/testng/JavaSqlModuleDriver.java
> 
> This is not a valid Java source file can you merge the jtreg meta data into 
> XAExceptionTests instead?

As we discussed offline, I will change the above file and Driver.java.  I 
naively assumed this was OK as the change  to add Driver.java was made after I 
had originally added these tests in 2015.

Best
Lance
> 
> Paul.
> 
>> On Feb 28, 2018, at 10:25 AM, Lance Andersen > > wrote:
>> 
>> Hi all,
>> 
>> This RFR request moves the javax.transaction.xa package out of the java.sql 
>> module and into its own module java.transaction.xa.  One of the motivators 
>> for this change is due to the fact JSR 907 1.3 MR indicated that the 
>> javax.transaction.xa package will be subsumed by Java SE.
>> 
>> There should be no compatibility issues with this change. Any module that 
>> `requires java.sql` will continue to have access to the public classes in 
>> the javax.transaction.xa package at both compile-time and run-time. 
>> 
>> 
>> The CSR has been approved
>> 
>> The webrev can be found at: 
>> http://cr.openjdk.java.net/~lancea/8197533/webrev.00/
>> 
>> Best
>> Lance
>> > >
>> > > 
>> > >
>> > >Lance Andersen| 
>> Principal Member of Technical Staff | +1.781.442.2037
>> Oracle Java Engineering 
>> 1 Network Drive 
>> Burlington, MA 01803
>> lance.ander...@oracle.com  
>> >

 
  

 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 JDK-8197533 move javax.transaction.xa into its own module

2018-02-28 Thread Paul Sandoz
Compatible module refactoring in action!

Looks good, one comment:

test/jdk/javax/transaction/xa/testng/JavaSqlModuleDriver.java

This is not a valid Java source file can you merge the jtreg meta data into 
XAExceptionTests instead?

Paul.

> On Feb 28, 2018, at 10:25 AM, Lance Andersen  
> wrote:
> 
> Hi all,
> 
> This RFR request moves the javax.transaction.xa package out of the java.sql 
> module and into its own module java.transaction.xa.  One of the motivators 
> for this change is due to the fact JSR 907 1.3 MR indicated that the 
> javax.transaction.xa package will be subsumed by Java SE.
> 
> There should be no compatibility issues with this change. Any module that 
> `requires java.sql` will continue to have access to the public classes in the 
> javax.transaction.xa package at both compile-time and run-time. 
> 
> 
> The CSR has been approved
> 
> The webrev can be found at: 
> http://cr.openjdk.java.net/~lancea/8197533/webrev.00/
> 
> Best
> Lance
> 
>  
> 
> Lance Andersen| 
> Principal Member of Technical Staff | +1.781.442.2037
> Oracle Java Engineering 
> 1 Network Drive 
> Burlington, MA 01803
> lance.ander...@oracle.com 
> 
> 
> 



RFR JDK-8197533 move javax.transaction.xa into its own module

2018-02-28 Thread Lance Andersen
Hi all,

This RFR request moves the javax.transaction.xa package out of the java.sql 
module and into its own module java.transaction.xa.  One of the motivators for 
this change is due to the fact JSR 907 1.3 MR indicated that the 
javax.transaction.xa package will be subsumed by Java SE.

There should be no compatibility issues with this change. Any module that 
`requires java.sql` will continue to have access to the public classes in the 
javax.transaction.xa package at both compile-time and run-time. 

   
The CSR has been approved

The webrev can be found at: 
http://cr.openjdk.java.net/~lancea/8197533/webrev.00/

Best
Lance
 
  

 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: 8193660 Check SOURCE line in "release" file for closedjdk

2018-02-28 Thread Randy Crihfield


On 02/28/18 11:58 AM, Alan Bateman wrote:

On 28/02/2018 16:57, Randy Crihfield wrote:


poke poke can someone please review?  Thanks

The `release` is a properties file so I assume you can simplify the 
test by using Properties.load and then testing if the "SOURCE" key is 
present and whether it has the expected value.


-Alan.


I could though I actually already wrote the test as requested initially 
and read the file as desired at that time, which is in the workspace.


The current incantation is to expand the test to also check a closed 
build and streamline, rather than write two tests.


I would like to get approval as it is, but if I need to go back and 
change existing behavior I will.


Randy


Re: RFR: Here are some URLClassPath patches

2018-02-28 Thread Alan Bateman



On 28/02/2018 18:04, Martin Buchholz wrote:

:

Too bad the URL[] argument is not consistently the last one, else one 
could  convert them all to varargs. Varargs-friendliness seems like an 
independent change.

Yes, completely separate issue and one that is already tracked:
https://bugs.openjdk.java.net/browse/JDK-8068425




I'm not really a java.net  guy (yet?) - I invite you 
to take over this sub-task of "improving URLClassLoader's constructors".




Re: RFR: Here are some URLClassPath patches

2018-02-28 Thread Martin Buchholz
On Wed, Feb 28, 2018 at 6:17 AM, Alan Bateman 
wrote:

> On 28/02/2018 03:02, Martin Buchholz wrote:
>
> I should probably do more testing than usual when touching classloading...
>
> We have a jdi test that does this (hg blame finds duke as only author):
>
> public static ClassLoader classLoaderValue;
> {
> try {
> urls[0] = new URL("hi there");
> } catch (java.net.MalformedURLException ee) {
> }
> classLoaderValue = new URLClassLoader(urls);
> }
>
> :
>
> Can we get jdi team to fix their dodgy tests?
>
> I checked pre OpenJDK history but I'm none the wiser. I assume the
> original author of this test assumed that urls[0] would be set to
> placeholder URL, it's just the MalformedURLException (which is must catch
> because it's a checked exception) masked the test bug.
>

Yes, it's likely this is a classic example of "impossible exception
swallowing" that is no longer considered acceptable.  Of course converting
to an unchecked exception like AssertionError would have been correct and
would have caught the mistake.


> java.net.URLClassLoader and its implementation in URLClassPath
> implementation haven't had a lot of TLC which probably explains why the
> null issue didn't surface before. We also need to change the one-arg
> constructor to use varargs to avoid needing to create an explicit array for
> the common case that you want to create it with a single URL.
>

Too bad the URL[] argument is not consistently the last one, else one
could  convert them all to varargs.  Varargs-friendliness seems like an
independent change.

I'm not really a java.net guy (yet?) - I invite you to take over this
sub-task of "improving URLClassLoader's constructors".


Re: RFR: 8193660 Check SOURCE line in "release" file for closedjdk

2018-02-28 Thread Alan Bateman

On 28/02/2018 16:57, Randy Crihfield wrote:


poke poke can someone please review?  Thanks

The `release` is a properties file so I assume you can simplify the test 
by using Properties.load and then testing if the "SOURCE" key is present 
and whether it has the expected value.


-Alan.


Re: RFR: 8193660 Check SOURCE line in "release" file for closedjdk

2018-02-28 Thread Randy Crihfield


poke poke can someone please review?  Thanks

Randy

On 02/15/18 12:25 PM, Randy Crihfield wrote:

I need to revise the resource file test created for JDK-8192837

The new bug is https://bugs.openjdk.java.net/browse/JDK-8193660

The webrev is located at 
http://cr.openjdk.java.net/~shurailine/8193660/webrev.00/ 



Any comments/suggestions are welcome, also I will need a sponsor for 
it at the end…


Randy




RFR: JDK-8197594: String#repeat

2018-02-28 Thread Jim Laskey
Introduction of a new instance method String::repeat to allow an efficient and 
concise approach for generating repeated character sequences as strings.

Performance information in JBS.

Thank you.

Cheers,

— Jim


JBS: https://bugs.openjdk.java.net/browse/JDK-8197594 

CSR: https://bugs.openjdk.java.net/browse/JDK-8198296
 
Webrev: http://cr.openjdk.java.net/~jlaskey/8197594/webrev-02/index.html 

JavaDoc: http://cr.openjdk.java.net/~jlaskey/8197594/String.html 






Re: RFR (JDK11) 8137326: Methods for comparing CharSequence, StringBuilder, and StringBuffer

2018-02-28 Thread Roger Riggs

+1; looks good

On 2/27/2018 3:15 PM, Xueming Shen wrote:

+1

On 2/27/18, 11:37 AM, Joe Wang wrote:

Hi Sherman and all,

Thanks for the further reviews!

Here's the latest webrev with boundary checks in 
StringLatin1/StringUTF16:


JBS: https://bugs.openjdk.java.net/browse/JDK-8137326
webrev: http://cr.openjdk.java.net/~joehw/jdk11/8137326/webrev/

Best,
Joe

On 2/22/2018 6:07 PM, Joe Wang wrote:



On 2/22/18, 12:51 PM, Xueming Shen wrote:

On 2/22/18, 12:04 PM, Joe Wang wrote:

Hi Sherman,

Thanks for reviewing the change.

Taking a local copy of the count field, but the boundary check 
would be almost immediately done against the field itself. Are you 
worrying about the count field may be out of sync with the byte 
array? I would think that's unlikely to happen. Whether it's 
StringBuilder or StringBuffer, it's not advisable/practical to use 
in multiple threads. In a valid usage, the count is always 
consistent with the byte array.




Hi Joe,

It might not be a "valid usage" but it did happen and when it 
happens it might just crash the
vm without those boundary checks. It's especially true for those 
intrinsics methods with explicit
comments "intrinsic performs no bounds checks". In this case, the 
StringUTF16.getChar() is being
called in new public method StringUTF16.compareTo(byte[], byte[], 
int, int) without appropriate
boundary check. In the "old" code the "index" is guaranteed to be 
within [0, len) in
StringUTF16.compareTo(byte[], byte[]), so it's safe. A real case 
for such scenario can be found in

JDK-8158168 [1], for example.


Thanks for the pointer! The email thread helps a lot. I've updated 
the webrev with a boundary check in ASB (AbstractStringBuilder line 
106, 107), and then a note to StringUTF16.compareTo (StringUTF16 
line 280). Hopefully this is sufficient. Didn't want to add any 
check in StringUTF16 since that may affect the original two-arg method.


JBS: https://bugs.openjdk.java.net/browse/JDK-8137326
webrev: http://cr.openjdk.java.net/~joehw/jdk11/8137326/webrev/

-Joe



-Sherman


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








RFR 8198697: Simplify platform encoding initialization tweak

2018-02-28 Thread Roger Riggs

Hi,

In an effort to untangle some of the issues with property initialization 
I was looking

at the platform encoding initialization and found a simplification.

Currently, the initialization occurs as a side effect of the first call 
to JNU_NewStringPlatform and
involves a upcall to get sun.jnu.encoding from the system properties.  
The value is cached for later use.


The native System.initProperties determines the platform specific 
encoding via java_props_md.c and does an upcall to set the 
sun.jnu.encoding system property, taking care to do it before the first 
string that needs platform encoding.


The change directly initializes the platform encoding fast path before 
it is needed to encode platform strings.
And moves the setting sun.jnu.encoding system property after the command 
line arguments are inserted
keeping it from being overridden by -D on the command line that can only 
confuse confusion

with code that later reads the property.

Please review and comment.

webrev:
   http://cr.openjdk.java.net/~rriggs/webrev-simplify-jnu-8198697/

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

Thanks, Roger






Re: RFR: 8198831: Lazy initialization of ValueConversions MethodHandles

2018-02-28 Thread Claes Redestad



On 2018-02-28 14:48, Aleksey Shipilev wrote:

On 02/28/2018 02:16 PM, Claes Redestad wrote:

this patch makes the lookup of various MH in sun.invoke.util.ValueConversions 
lazy, realizing a tiny
startup optimization to various apps.

Webrev: http://cr.openjdk.java.net/~redestad/8198831/jdk.00/

Looks good to me.


Thanks, Aleksey!

/Claes


Re: RFR: Here are some URLClassPath patches

2018-02-28 Thread Alan Bateman

On 28/02/2018 03:02, Martin Buchholz wrote:

I should probably do more testing than usual when touching classloading...

We have a jdi test that does this (hg blame finds duke as only author):

    public static ClassLoader classLoaderValue;
    {
        try {
            urls[0] = new URL("hi there");
        } catch (java.net.MalformedURLException ee) {
        }
        classLoaderValue = new URLClassLoader(urls);
    }

:

Can we get jdi team to fix their dodgy tests?
I checked pre OpenJDK history but I'm none the wiser. I assume the 
original author of this test assumed that urls[0] would be set to 
placeholder URL, it's just the MalformedURLException (which is must 
catch because it's a checked exception) masked the test bug.


java.net.URLClassLoader and its implementation in URLClassPath 
implementation haven't had a lot of TLC which probably explains why the 
null issue didn't surface before. We also need to change the one-arg 
constructor to use varargs to avoid needing to create an explicit array 
for the common case that you want to create it with a single URL.


-Alan


Re: RFR: 8198831: Lazy initialization of ValueConversions MethodHandles

2018-02-28 Thread Aleksey Shipilev
On 02/28/2018 02:16 PM, Claes Redestad wrote:
> this patch makes the lookup of various MH in sun.invoke.util.ValueConversions 
> lazy, realizing a tiny
> startup optimization to various apps.
> 
> Webrev: http://cr.openjdk.java.net/~redestad/8198831/jdk.00/

Looks good to me.

-Aleksey



RFR: 8198831: Lazy initialization of ValueConversions MethodHandles

2018-02-28 Thread Claes Redestad

Hi,

this patch makes the lookup of various MH in 
sun.invoke.util.ValueConversions lazy, realizing a tiny startup 
optimization to various apps.


Webrev: http://cr.openjdk.java.net/~redestad/8198831/jdk.00/
Bug:    https://bugs.openjdk.java.net/browse/JDK-8198831

Thanks!

/Claes


sun.rmi.transport.tcp.responseTimeout ignored

2018-02-28 Thread Marcello Lorenzi
 Hi All,
We started a new Java standalone application with OpenJDK Runtime
Environment (build 1.8.0_161-b14). This application has a network problem
to connect to a RMI service and the application threads remained in hang
status. We tried to apply the parameter sun.rmi.transport.tcp.responseTimeout
but the threads on JVM side remained in stuck and the connection to the RMI
server was in SYN_SENT status. Could it be related with bug
https://bugs.openjdk.java.net/browse/JDK-8151212?

Thanks,
Marcello


Re: [JDK 11] Problem list tools/jimage/JImageExtractTest.java for macosx-all

2018-02-28 Thread Amy Lu

On 28/02/2018 6:23 PM, Alan Bateman wrote:



On 28/02/2018 08:53, Amy Lu wrote:
Please review the patch to problem list 
tools/jimage/JImageExtractTest.java for macosx-all.


This test fails frequently (observed at Mac) and should be problem 
listed before JDK-8198819 fixed.


bug: https://bugs.openjdk.java.net/browse/JDK-8198820
webrev: http://cr.openjdk.java.net/~amlu/8198820/webrev.00/
This looks okay, also no objection to excluding it on all platforms as 
it is seems to fail periodically on Solaris too.


Thank you Alan!

Changed to problem list for generic-all and pushed.

Thanks,
Amy



-Alan




Re: [JDK 11] Problem list tools/jimage/JImageExtractTest.java for macosx-all

2018-02-28 Thread Alan Bateman



On 28/02/2018 08:53, Amy Lu wrote:
Please review the patch to problem list 
tools/jimage/JImageExtractTest.java for macosx-all.


This test fails frequently (observed at Mac) and should be problem 
listed before JDK-8198819 fixed.


bug: https://bugs.openjdk.java.net/browse/JDK-8198820
webrev: http://cr.openjdk.java.net/~amlu/8198820/webrev.00/
This looks okay, also no objection to excluding it on all platforms as 
it is seems to fail periodically on Solaris too.


-Alan


[JDK 11] Problem list tools/jimage/JImageExtractTest.java for macosx-all

2018-02-28 Thread Amy Lu
Please review the patch to problem list 
tools/jimage/JImageExtractTest.java for macosx-all.


This test fails frequently (observed at Mac) and should be problem 
listed before JDK-8198819 fixed.


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

Thanks,
Amy


--- old/test/jdk/ProblemList.txt2018-02-28 16:48:06.0 +0800
+++ new/test/jdk/ProblemList.txt2018-02-28 16:48:06.0 +0800
@@ -467,7 +467,7 @@
 
 tools/launcher/FXLauncherTest.java  8068049 linux-all,macosx-all
 
-tools/jimage/JImageExtractTest.java 8198405 windows-all

+tools/jimage/JImageExtractTest.java 
8198405,8198819 windows-all,macosx-all
 tools/jimage/JImageListTest.java8198405 
windows-all