RE: Patch for adding SO_REUSEPORT socket option

[Lu, Yingqi]

Yes, we have an open discussion there as well. From now on, we will focus on 
net-dev.

Thank you!
Yingqi

-Original Message-
From: David Holmes [mailto:david.hol...@oracle.com] 
Sent: Thursday, November 19, 2015 6:10 PM
To: Lu, Yingqi ; Kharbas, Kishor 
; core-libs-dev@openjdk.java.net
Subject: Re: Patch for adding SO_REUSEPORT socket option

Hi,

I think this should be moved to the net-dev mailing list.

Thanks,
David

On 20/11/2015 6:37 AM, Lu, Yingqi wrote:
> Hello All,
>
> Currently, I think the only platform that does not have SO_REUSEPORT is 
> Windows. API allows this possibility by throwing SocketException with message 
> "Invalid option". Older kernel that does not support the feature throws 
> SocketException with message "Protocol not available".
>
> Here is the first version of the patch (we implement SO_REUSEPORT as a 
> standard socket option): 
> http://cr.openjdk.java.net/~mcberg/jdk/6432031/webrev.01/
>
> Here is the second version of the patch (we implement SO_REUSEPORT as an 
> extended socket option): 
> http://cr.openjdk.java.net/~mcberg/jdk/6432031/webrev.02/
>
> The performance improvement by enabling SO_REUSEPORT is significant (up to 
> 1.93x on Hadoop Distributed File System (HDFS)). It would be great if OpenJDK 
> can have it. Please take some time to review the patches and let us know your 
> feedback and comments!
>
> Thanks,
> Lucy
>
>
> From: Kharbas, Kishor
> Sent: Monday, November 16, 2015 10:47 AM
> To: core-libs-dev@openjdk.java.net
> Cc: Kharbas, Kishor; Lu, Yingqi; Kharbas, Kishor
> Subject: Patch for adding SO_REUSEPORT socket option
>
> Hello all,
>
> I request the community to review a patch for adding SO_REUSEPORT support. 
> There is already an existing JBS opened at 
> https://bugs.openjdk.java.net/browse/JDK-6432031
>
> Details :
>
> SO_REUSEPORT removes 1:1 assignment between listen socket and IP:PORT pair 
> and enable multiple sockets listening to the same address and port. This 
> improves the scalability and parallelism of network traffic handling. It is 
> enabled for both TCP and UDP sockets (at least for Linux). For more details, 
> please refer to https://lwn.net/Articles/542629/. Many applications, 
> especially Linux or BSD based webservers such as Apache httpd and Nginx are 
> already supporting it now. Ruby and Python have it supported as well. Other 
> Java applications such as Netty webserver have it supported via JNI function 
> since JDK has not supported it yet.
>
>
>
> By enabling the SO_REUSEPORT feature itself, up to 4X throughput and latency 
> improvement have been observed from various applications. Specific to Java 
> application with this patch, we modified Apache Hadoop Distributed File 
> System (HDFS) source code to take advantage of this feature. We observed up 
> to 1.93x performance improvements.
>
> The feature is supported since Linux Kernel 3.9. It is also supported in 
> BSDs, Solaris and Mac OS. Windows does not have it. In the current patch, we 
> only enable the feature on Linux platform since we do not have BSD, Solaris 
> and Mac OS for testing. Whether the feature is supported or not on the 
> running kernel is determined at the run time.
>
> P.S. Based on Alan Baleman's comment on JBS, we are in meanwhile working on 
> adding this option to 'java.net.ExtendedSocketOption'.
>
> Regards,
> Kishor Kharbas
>

Re: RFR(L): JDK-8046936 : JEP 270: Reserved Stack Areas for Critical Sections

[David Holmes]

On 20/11/2015 4:10 AM, Doug Lea wrote:

On 11/16/2015 08:00 PM, David Holmes wrote:

I've tried that :-) The amount of code being executed with
the ReservedStackAccess privilege tends to increase very
quickly and I was concerned about keeping the size of the
reserved area small.


How much space does each level of calling need? I know that is hard to
answer
but are we talking a few bytes, few kb, many kb?



Relatedly, could someone spell out in more detail the
worst-case behavior of StackOverflowError? Knowing this
might be helpful in some cases in which we might be able to
at least preserve minimal sanity without needing to reserve.


Not sure what you are asking for Doug - worst-case is that an action 
that is critical to complete does not complete. Best example is 
lock/unlock which have to both update AQS-state and update owner. If 
only one is done the lock internal state is inconsistent and the lock 
ceases to function.


David


-Doug

Re: Review request for JDK-8141338: Move jdk.internal.dynalink package to jdk.dynalink

[Sundararajan Athijegannathan]

+1 on all changes.

-Sundar

On 11/20/2015 12:15 AM, Attila Szegedi wrote:

Please review JDK-8141338 "Move jdk.internal.dynalink package to jdk.dynalink" for 
. This is basically the implementation 
step for integrating JEP 276. This changeset will introduce a new public API that has CCC 
approval (request 8075866), and is also the implementation step of JEP 276 which is now 
targeted for 9 and thus can be integrated.

The changes in this changeset fall into several categories:
- renaming of jdk.internal.dynalink.* package to jdk.dynalink.* package, with 
ripple effects in Nashorn classes that import from these packages
- changes to modules.xml and some build files to accommodate a new public 
module and a dependency of Nashorn on it
- new tests

  I’m sending this webrev to several lists with the following rationales:
- nashorn-dev as the primary users and expected reviewers (also, the Dynalink 
module code lives in jdk9/nashorn/src/jdk.dynalink). A lot of newly added test 
code was contributed by Sundar.
- jigsaw-dev because of modules.xml changes
- jdk9-dev for build changes (build file changes were graciously contributed by 
Erik Joelsson and Sundar)
- core-libs-dev since that’s the designated JEP 276 discussion list.

Nashorn changes: 
top-level jdk9 changes: 


Thanks,
   Attila.

Re: RFR: 8066272: pack200 must support Multi-Release Jars

[Steve Drach]

I’m not an official reviewer so here are my unofficial comments:

1.  I see no obvious flaws
2.  The name MultiVersion should be changed to MultiRelease to correspond with 
the name change in the JEP.
3.  While for this test it’s okay to have versions 7, 8, and 9, in practice we 
won’t process version 7 or 8.  Version numbers correspond to platform release 
numbers and will start at 9 since it’s the first release that can process 
MultiRelease jars.

> On Nov 16, 2015, at 2:17 PM, Kumar Srinivasan  
> wrote:
> 
> 
> John, Steve,
> 
> I have updated the webrev which fixes the version numbers
> and removes the duplicate manifest's in the jar-files.
> 
> Full webrev:
> http://cr.openjdk.java.net/~ksrini/8066272/webrev.1/
> 
> Incremental webrev:
> http://cr.openjdk.java.net/~ksrini/8066272/webrev.1/webrev.delta/index.html
> 
> Thanks
> Kumar
> 
> 
>> Hi John, Steve,
>> 
>> Please review a fix for bug:
>> https://bugs.openjdk.java.net/browse/JDK-8066272
>> 
>> The webrev:
>> http://cr.openjdk.java.net/~ksrini/8066272/webrev.0/
>> 
>> To support Multi versioned jar files all  versioned class files
>> in the META-INF directory will be excluded from the global
>> constant pool, instead these will be regarded as resources
>> and stored in the resource specific bands.
>> 
>> Thanks
>> Kumar
>> 
> 

RE: Proposed API for JEP 259: Stack-Walking API

[Timo Kinnunen]

The issue is that usually IllegalStateException implies the callee went into 
the wrong state thanks to what the user has done; as nothing happens until a 
user puts things in motion by executing some code. But this is not the case 
here. In this case the IllegalStateException only reflects an implementation 
detail that might change in the future. Any decision from a user on seeing an 
ISE here can only bring about extra dependencies on this in ways throwing an 
UnsupportedOperationException wouldn’t.




Sent from Mail for Windows 10



From: David Holmes
Sent: Friday, November 20, 2015 03:12
To: Timo Kinnunen;John Rose
Cc: OpenJDK Dev list
Subject: Re: Proposed API for JEP 259: Stack-Walking API


On 19/11/2015 11:36 PM, Timo Kinnunen wrote:
> A point in favor of UnsupportedOperationException would be: if in the
> future it becomes possible to have large parts of the JVM written in
> Java or sun.tools becomes part of the J2SE API then having an instance
> of a VirtualMachine class as the caller of public static void main and
> Thread.run could be completely true and natural. Changing getCallerClass
> to support this and not throw an UOE feels less of a breaking change
> than it no longer throwing an IllegalStateException. Especially since no
> state that the caller can affect has actually changed!

@throws IllegalStateException if called from a context where this is no 
Java caller present

If you change the VM implementation so there is a Java caller then you 
don't throw ISE any more. I don't see any issue.

Cheers,
David

> Sent from Mail  for
> Windows 10
>
>
> *From: *John Rose
> *Sent: *Thursday, November 19, 2015 05:45
> *To: *David Holmes
> *Cc: *OpenJDK Dev list
> *Subject: *Re: Proposed API for JEP 259: Stack-Walking API
>
> On Nov 18, 2015, at 6:32 PM, David Holmes  wrote:
>
>  >
>
>  >>>
>
>  >>> Looks good to me too if IllegalStateException is used instead of
>
>  >>> UnsupportedOperationException.
>
>  >>> UnsuppportedOperationException is used when the operation is not
>
>  >>> available, here, the same code can work or not depending how it is
>
>  >>> called.
>
>  >>
>
>  >> But IllegalStateException is dependent on some state. There's no state
>
>  >> involved here (in the sense "state" is characterized in Java). My 1st
>
>  >
>
>  > I agree with Remi. "state" doesn't have to mean fields - there are
> numerous existing examples in the JDK. Calling a method in a context
> that is invalid is an illegal state to me. IllegalThreadStateException
> would also work. But UnsupportedOperationException ... more of a stretch.
>
> +1
>

Re: Review request for JDK-8141338: Move jdk.internal.dynalink package to jdk.dynalink

[Mandy Chung]

I reviewed the top repo change.

modules.xml looks fine.

jdk.dynalink  should be in MAIN_MODULES since it has exported APIs.  
jdk.scripting.nashorn should be moved too.  They are not sole service 
providers.  Since you are on this file, can you move jdk.scripting.nashorn to 
MAIN_MODULES as well?

Mandy

> On Nov 19, 2015, at 3:15 PM, Attila Szegedi  wrote:
> 
> Please review JDK-8141338 "Move jdk.internal.dynalink package to 
> jdk.dynalink" for . This is 
> basically the implementation step for integrating JEP 276. This changeset 
> will introduce a new public API that has CCC approval (request 8075866), and 
> is also the implementation step of JEP 276 which is now targeted for 9 and 
> thus can be integrated.
> 
> The changes in this changeset fall into several categories:
> - renaming of jdk.internal.dynalink.* package to jdk.dynalink.* package, with 
> ripple effects in Nashorn classes that import from these packages
> - changes to modules.xml and some build files to accommodate a new public 
> module and a dependency of Nashorn on it
> - new tests
> 
> I’m sending this webrev to several lists with the following rationales:
> - nashorn-dev as the primary users and expected reviewers (also, the Dynalink 
> module code lives in jdk9/nashorn/src/jdk.dynalink). A lot of newly added 
> test code was contributed by Sundar.
> - jigsaw-dev because of modules.xml changes
> - jdk9-dev for build changes (build file changes were graciously contributed 
> by Erik Joelsson and Sundar)
> - core-libs-dev since that’s the designated JEP 276 discussion list.
> 
> Nashorn changes:  
> top-level jdk9 changes: 
> 
> 
> Thanks,
>  Attila.
> 

Re: Code Review for JEP 259: Stack-Walking API

[Mandy Chung]

Cross-posting to core-libs-dev.

Delta webrev incorporating feedback from Coleen and Brent and also a fix that 
Daniel made:
   http://cr.openjdk.java.net/~mchung/jdk9/jep259/webrev.03-delta/

Full webrev:
   http://cr.openjdk.java.net/~mchung/jdk9/jep259/webrev.03/

This also includes a couple of new tests Hamlin has contributed.

Mandy

> On Nov 19, 2015, at 1:38 PM, Coleen Phillimore  
> wrote:
> 
> 
> Hi Mandy,
> Thank you for making these changes.  I think it looks really good!
> 
> On 11/19/15 4:09 PM, Mandy Chung wrote:
>> Hi Coleen,
>> 
>> Here is the revised webrev incorporating your suggestions and a couple other 
>> minor java side change:
>>http://cr.openjdk.java.net/~mchung/jdk9/jep259/webrev.03-delta
>> 
>> I got some idea how to prepare Throwable to switch to StackWalker API to 
>> address the footprint/performance concern.  I agree with you that it would 
>> have to use mid/cpref in the short term for Throwable.  StackWalker::walk 
>> should use MemberName and should improve the footprint/performance issue in 
>> the long term.
>> 
>> Are you okay to follow that up with JDK-8141239 and Throwable continues to 
>> use the VM backtrace until JDK-8141239 is resolved?
> 
> Yes, and as we discussed, I'd like to help with this.
> 
> Thanks!
> Coleen

Re: Proposed API for JEP 259: Stack-Walking API

[David Holmes]

On 19/11/2015 11:36 PM, Timo Kinnunen wrote:

A point in favor of UnsupportedOperationException would be: if in the
future it becomes possible to have large parts of the JVM written in
Java or sun.tools becomes part of the J2SE API then having an instance
of a VirtualMachine class as the caller of public static void main and
Thread.run could be completely true and natural. Changing getCallerClass
to support this and not throw an UOE feels less of a breaking change
than it no longer throwing an IllegalStateException. Especially since no
state that the caller can affect has actually changed!


@throws IllegalStateException if called from a context where this is no 
Java caller present


If you change the VM implementation so there is a Java caller then you 
don't throw ISE any more. I don't see any issue.


Cheers,
David


Sent from Mail  for
Windows 10


*From: *John Rose
*Sent: *Thursday, November 19, 2015 05:45
*To: *David Holmes
*Cc: *OpenJDK Dev list
*Subject: *Re: Proposed API for JEP 259: Stack-Walking API

On Nov 18, 2015, at 6:32 PM, David Holmes  wrote:

 >

 >>>

 >>> Looks good to me too if IllegalStateException is used instead of

 >>> UnsupportedOperationException.

 >>> UnsuppportedOperationException is used when the operation is not

 >>> available, here, the same code can work or not depending how it is

 >>> called.

 >>

 >> But IllegalStateException is dependent on some state. There's no state

 >> involved here (in the sense "state" is characterized in Java). My 1st

 >

 > I agree with Remi. "state" doesn't have to mean fields - there are
numerous existing examples in the JDK. Calling a method in a context
that is invalid is an illegal state to me. IllegalThreadStateException
would also work. But UnsupportedOperationException ... more of a stretch.

+1

Re: Patch for adding SO_REUSEPORT socket option

[David Holmes]

Hi,

I think this should be moved to the net-dev mailing list.

Thanks,
David

On 20/11/2015 6:37 AM, Lu, Yingqi wrote:

Hello All,

Currently, I think the only platform that does not have SO_REUSEPORT is Windows. API allows this 
possibility by throwing SocketException with message "Invalid option". Older kernel that 
does not support the feature throws SocketException with message "Protocol not available".

Here is the first version of the patch (we implement SO_REUSEPORT as a standard 
socket option): http://cr.openjdk.java.net/~mcberg/jdk/6432031/webrev.01/

Here is the second version of the patch (we implement SO_REUSEPORT as an 
extended socket option): 
http://cr.openjdk.java.net/~mcberg/jdk/6432031/webrev.02/

The performance improvement by enabling SO_REUSEPORT is significant (up to 
1.93x on Hadoop Distributed File System (HDFS)). It would be great if OpenJDK 
can have it. Please take some time to review the patches and let us know your 
feedback and comments!

Thanks,
Lucy


From: Kharbas, Kishor
Sent: Monday, November 16, 2015 10:47 AM
To: core-libs-dev@openjdk.java.net
Cc: Kharbas, Kishor; Lu, Yingqi; Kharbas, Kishor
Subject: Patch for adding SO_REUSEPORT socket option

Hello all,

I request the community to review a patch for adding SO_REUSEPORT support. 
There is already an existing JBS opened at 
https://bugs.openjdk.java.net/browse/JDK-6432031

Details :

SO_REUSEPORT removes 1:1 assignment between listen socket and IP:PORT pair and 
enable multiple sockets listening to the same address and port. This improves 
the scalability and parallelism of network traffic handling. It is enabled for 
both TCP and UDP sockets (at least for Linux). For more details, please refer 
to https://lwn.net/Articles/542629/. Many applications, especially Linux or BSD 
based webservers such as Apache httpd and Nginx are already supporting it now. 
Ruby and Python have it supported as well. Other Java applications such as 
Netty webserver have it supported via JNI function since JDK has not supported 
it yet.



By enabling the SO_REUSEPORT feature itself, up to 4X throughput and latency 
improvement have been observed from various applications. Specific to Java 
application with this patch, we modified Apache Hadoop Distributed File System 
(HDFS) source code to take advantage of this feature. We observed up to 1.93x 
performance improvements.

The feature is supported since Linux Kernel 3.9. It is also supported in BSDs, 
Solaris and Mac OS. Windows does not have it. In the current patch, we only 
enable the feature on Linux platform since we do not have BSD, Solaris and Mac 
OS for testing. Whether the feature is supported or not on the running kernel 
is determined at the run time.

P.S. Based on Alan Baleman's comment on JBS, we are in meanwhile working on 
adding this option to 'java.net.ExtendedSocketOption'.

Regards,
Kishor Kharbas

Re: RFR JDK-8143330: No appropriate CCC request for two new protected methods added in JDK9b93 by JDK-8141132

[Xueming Shen]

Thanks Joe, Alan! The synopsis has been updated accordingly.

On 11/19/2015 12:43 PM, Alan Bateman wrote:



On 19/11/2015 20:39, joe darcy wrote:

Looks good Sherman; thanks,


Looks okay to me too. We should fix the synopsis on the JIRA issue or at least 
put a better message in the change-set comment.

-Alan.

Re: [RFR]: 8131334: SAAJ Plugability Layer: using java.util.ServiceLoader

[Alan Bateman]

On 19/11/2015 18:36, Daniel Fuchs wrote:


Hi Miran,

I would expect this to fail horribly - you may not have the
right to create anything under $JAVA_HOME/conf - it may even
be a read-only file system... And I would be very uneasy as this
may have side effects on tests executing later on if anything
is left over in the java home conf dir...

There may be tests executing concurrently in other agent VMs too so we 
need to avoid changing the config like this.


-Alan

RE: Patch for adding SO_REUSEPORT socket option

[Lu, Yingqi]

Hello All,

Currently, I think the only platform that does not have SO_REUSEPORT is 
Windows. API allows this possibility by throwing SocketException with message 
"Invalid option". Older kernel that does not support the feature throws 
SocketException with message "Protocol not available".

Here is the first version of the patch (we implement SO_REUSEPORT as a standard 
socket option): http://cr.openjdk.java.net/~mcberg/jdk/6432031/webrev.01/

Here is the second version of the patch (we implement SO_REUSEPORT as an 
extended socket option): 
http://cr.openjdk.java.net/~mcberg/jdk/6432031/webrev.02/

The performance improvement by enabling SO_REUSEPORT is significant (up to 
1.93x on Hadoop Distributed File System (HDFS)). It would be great if OpenJDK 
can have it. Please take some time to review the patches and let us know your 
feedback and comments!

Thanks,
Lucy


From: Kharbas, Kishor
Sent: Monday, November 16, 2015 10:47 AM
To: core-libs-dev@openjdk.java.net
Cc: Kharbas, Kishor; Lu, Yingqi; Kharbas, Kishor
Subject: Patch for adding SO_REUSEPORT socket option

Hello all,

I request the community to review a patch for adding SO_REUSEPORT support. 
There is already an existing JBS opened at 
https://bugs.openjdk.java.net/browse/JDK-6432031

Details :

SO_REUSEPORT removes 1:1 assignment between listen socket and IP:PORT pair and 
enable multiple sockets listening to the same address and port. This improves 
the scalability and parallelism of network traffic handling. It is enabled for 
both TCP and UDP sockets (at least for Linux). For more details, please refer 
to https://lwn.net/Articles/542629/. Many applications, especially Linux or BSD 
based webservers such as Apache httpd and Nginx are already supporting it now. 
Ruby and Python have it supported as well. Other Java applications such as 
Netty webserver have it supported via JNI function since JDK has not supported 
it yet.



By enabling the SO_REUSEPORT feature itself, up to 4X throughput and latency 
improvement have been observed from various applications. Specific to Java 
application with this patch, we modified Apache Hadoop Distributed File System 
(HDFS) source code to take advantage of this feature. We observed up to 1.93x 
performance improvements.

The feature is supported since Linux Kernel 3.9. It is also supported in BSDs, 
Solaris and Mac OS. Windows does not have it. In the current patch, we only 
enable the feature on Linux platform since we do not have BSD, Solaris and Mac 
OS for testing. Whether the feature is supported or not on the running kernel 
is determined at the run time.

P.S. Based on Alan Baleman's comment on JBS, we are in meanwhile working on 
adding this option to 'java.net.ExtendedSocketOption'.

Regards,
Kishor Kharbas

RFR JDK-8143330: No appropriate CCC request for two new protected methods added in JDK9b93 by JDK-8141132

[Xueming Shen]

Hi

Please help review the change for 8143330.

Issue: https://bugs.openjdk.java.net/browse/JDK-8143330
webrev: http://cr.openjdk.java.net/~sherman/8143330/webrev

Cause: two implementation methods ABS.getBytes/initBytes were
mistakenly declared as "protected" and exposed to the public. Both
should be package private.

Thanks,
Sherman

Re: RFR: 8066272: pack200 must support Multi-Release Jars

[Kumar Srinivasan]

Will do, and will also rename the file.

Thanks
Kumar


Reviewed.  There is an underlying bug in the packer when it is stressed
with certain kind of incompatible simultaneous class data, but this is a clean
enough work-around.  Please file a follow-up bug; you can adapt the
unit test simply by making the string "META-INF" a command line
parameter (property) and changing it to (say) "NOT-META-INF".

— John


On Nov 16, 2015, at 2:17 PM, Kumar Srinivasan  
wrote:


John, Steve,

I have updated the webrev which fixes the version numbers
and removes the duplicate manifest's in the jar-files.

Full webrev:
http://cr.openjdk.java.net/~ksrini/8066272/webrev.1/

Incremental webrev:
http://cr.openjdk.java.net/~ksrini/8066272/webrev.1/webrev.delta/index.html

Thanks
Kumar



Hi John, Steve,

Please review a fix for bug:
https://bugs.openjdk.java.net/browse/JDK-8066272

The webrev:
http://cr.openjdk.java.net/~ksrini/8066272/webrev.0/

To support Multi versioned jar files all  versioned class files
in the META-INF directory will be excluded from the global
constant pool, instead these will be regarded as resources
and stored in the resource specific bands.

Thanks
Kumar

Review request for JDK-8141338: Move jdk.internal.dynalink package to jdk.dynalink

[Attila Szegedi]

Please review JDK-8141338 "Move jdk.internal.dynalink package to jdk.dynalink" 
for . This is basically the 
implementation step for integrating JEP 276. This changeset will introduce a 
new public API that has CCC approval (request 8075866), and is also the 
implementation step of JEP 276 which is now targeted for 9 and thus can be 
integrated.

The changes in this changeset fall into several categories:
- renaming of jdk.internal.dynalink.* package to jdk.dynalink.* package, with 
ripple effects in Nashorn classes that import from these packages
- changes to modules.xml and some build files to accommodate a new public 
module and a dependency of Nashorn on it
- new tests

 I’m sending this webrev to several lists with the following rationales:
- nashorn-dev as the primary users and expected reviewers (also, the Dynalink 
module code lives in jdk9/nashorn/src/jdk.dynalink). A lot of newly added test 
code was contributed by Sundar.
- jigsaw-dev because of modules.xml changes
- jdk9-dev for build changes (build file changes were graciously contributed by 
Erik Joelsson and Sundar)
- core-libs-dev since that’s the designated JEP 276 discussion list.

Nashorn changes:  
top-level jdk9 changes: 


Thanks,
  Attila.

Re: RFR: 8066272: pack200 must support Multi-Release Jars

[John Rose]

Reviewed.  There is an underlying bug in the packer when it is stressed
with certain kind of incompatible simultaneous class data, but this is a clean
enough work-around.  Please file a follow-up bug; you can adapt the
unit test simply by making the string "META-INF" a command line
parameter (property) and changing it to (say) "NOT-META-INF".

— John

> On Nov 16, 2015, at 2:17 PM, Kumar Srinivasan  
> wrote:
> 
> 
> John, Steve,
> 
> I have updated the webrev which fixes the version numbers
> and removes the duplicate manifest's in the jar-files.
> 
> Full webrev:
> http://cr.openjdk.java.net/~ksrini/8066272/webrev.1/
> 
> Incremental webrev:
> http://cr.openjdk.java.net/~ksrini/8066272/webrev.1/webrev.delta/index.html
> 
> Thanks
> Kumar
> 
> 
>> Hi John, Steve,
>> 
>> Please review a fix for bug:
>> https://bugs.openjdk.java.net/browse/JDK-8066272
>> 
>> The webrev:
>> http://cr.openjdk.java.net/~ksrini/8066272/webrev.0/
>> 
>> To support Multi versioned jar files all  versioned class files
>> in the META-INF directory will be excluded from the global
>> constant pool, instead these will be regarded as resources
>> and stored in the resource specific bands.
>> 
>> Thanks
>> Kumar
>> 
> 

Re: RFR(L): 8139885: implement JEP 274: enhanced method handles

[Michael Haupt]

Hi John, Paul,

thanks for your reviews - they have helped me polish the code and documentation 
some more and add some more tests. The result is at 
http://cr.openjdk.java.net/~mhaupt/8139885/webrev.02

I noticed both of you emphasised how the Streams API helps the implementation - 
I second that. Streams "made my day" there. :-)

I'm replying below to those of your remarks I didn't address in the new webrev.

> Am 19.11.2015 um 05:25 schrieb John Rose :
> My usual practice, though, is to move argument checking and error reporting 
> code out of line, away from the main logic.  I think this is good style, and 
> it gives the JIT a little bit (a very little bit) of help.

In the most generic loop combinator, the tests are somewhat dispersed over the 
implementation because they depend on results that are not readily available at 
the beginning of the method's execution. I haven't put them all in one test 
method because I prefer early failure with clear messages over possibly hard to 
interpret failure due to inconsistencies in mid-run.

> One wishful item:  It would be nice if the javadoc examples could be 
> integrated into JavaDocExamplesTest.java.  I see that is messy since we are 
> now using TestNG instead of JUnit.  The point of JavaDocExamplesTest was to 
> make it easy to "sync" the javadoc with the examples, by having one place to 
> copy-and-paste the javadoc.

I've filed this RFE for that: https://bugs.openjdk.java.net/browse/JDK-8143343

> Am 19.11.2015 um 11:45 schrieb Paul Sandoz :
> I have a mild preference to maintain the 80 char limit for JavaDoc, to me 
> it’s easier to read. For code i don’t mind a 100 or more limit.

The formatting in the files I've touched is inconsistent; I'll stick with the 
120 character limit for both code and Javadoc.

> 3458  * The loop handle's result type is the same as the sole loop 
> variable's, i.e., the result type of {@code init}.
> 
> s/variable’s/variable ?

No; the genitive is intentional (result type = loop variable type, rather than 
result type = loop variable).

> I guess in the future there may be ample opporunity to specialize the generic 
> “loop” mechanism with LambdaForms?

Yep: https://bugs.openjdk.java.net/browse/JDK-8143211

Best,

Michael

-- 

 
Dr. Michael Haupt | Principal Member of Technical Staff
Phone: +49 331 200 7277 | Fax: +49 331 200 7561
Oracle Java Platform Group | LangTools Team | Nashorn
Oracle Deutschland B.V. & Co. KG, Schiffbauergasse 14 | 14467 Potsdam, Germany
  Oracle is committed to developing 
practices and products that help protect the environment

Code (Pre-)Review for JEP 280: Indify String Concat

[Aleksey Shipilev]

Hi,

I would like to have a code pre-review for Indify String Concat changes.
For the reference, the rationale, design constraints, etc. are outlined
in the JEP itself:
  https://bugs.openjdk.java.net/browse/JDK-8085796

The experimental notes, including the bytecode shapes, benchmark data,
and benchmark analysis, interaction with Compact Strings are here:
  http://cr.openjdk.java.net/~shade/8085796/notes.txt

Javadoc (CCC is in progress):

http://cr.openjdk.java.net/~shade/8085796/javadocs/StringConcatFactory.html


=== javac change:

Webrev:
 http://cr.openjdk.java.net/~shade/8085796/webrev.langtools.00/

A little guide:

 * We cut in at the same two points current javac does the String
concat. But instead of emitting the StringBuilder append chains, we
collect all arguments and hand them over to JDK's
java.lang.invoke.StringConcatFactory.

 * The bytecode flavor is guarded by hidden -XDstringConcat option, with
three possible values: inline, indy, indyWithConstants.
"indyWithConstants" is selected as the default bytecode flavor.

 * Since a String concat expression may contain more operands than any
Java call can manage, we sometimes have to peel the concat in several
back-to-back calls, and concat-ting the results. That peeling is
one-level, and can accommodate 200*200 = 4 arguments, well beyond
what can be achieved for a singular String concat expression (limited
either by constant pool size, or by method code size).


=== JDK change:

Webrev:
 http://cr.openjdk.java.net/~shade/8085796/webrev.jdk.00/

A little guide:

 * The entry point is java.lang.invoke.StringConcatFactory. Its methods
would be used as invokedynamic bootstrap methods, at least in javac.

 * There are multiple concatenation strategies supported by SCF. We
explored the performance characteristics of all factories, chosen one
performing good throughput- and startup-wise. All strategies are fully
functional, covered by tests, and kept as the base for the future work
and fine tuning.

 * Most strategies delegate to public StringBuilder API, which makes
them quite simple, since they are not forced to deal with actual storage
(this got complicated with Compact Strings).

 * The only strategy that builds the String directly is
MH_INLINE_SIZED_EXACT strategy, and thus it is the most complicated of
all. It uses private java.lang.StringConcatHelper class to get access to
package-private (Integer|Long).(stringSize|getChar*) methods; the access
to j.l.SCH is granted by a private lookup.

 * Some tests assume the particular code shape and/or invokedynamic
counts, needed to fix those as well.

=== Build change

(I don't copy build-dev@ to avoid spamming that list with langtools/jdk
reviews; this section is FYI)

Webrev:
 http://cr.openjdk.java.net/~shade/8085796/webrev.root.00/

 * This one is simple: we exempt java.base and interim classes from Indy
String Concat to avoid bootstrapping issues.


Thanks,
-Aleksey

Re: RFR JDK-8143330: No appropriate CCC request for two new protected methods added in JDK9b93 by JDK-8141132

[Alan Bateman]

On 19/11/2015 20:39, joe darcy wrote:

Looks good Sherman; thanks,

Looks okay to me too. We should fix the synopsis on the JIRA issue or at 
least put a better message in the change-set comment.


-Alan.

Re: RFR JDK-8143330: No appropriate CCC request for two new protected methods added in JDK9b93 by JDK-8141132

[joe darcy]

Looks good Sherman; thanks,

-Joe

On 11/19/2015 12:27 PM, Xueming Shen wrote:

Hi

Please help review the change for 8143330.

Issue: https://bugs.openjdk.java.net/browse/JDK-8143330
webrev: http://cr.openjdk.java.net/~sherman/8143330/webrev

Cause: two implementation methods ABS.getBytes/initBytes were
mistakenly declared as "protected" and exposed to the public. Both
should be package private.

Thanks,
Sherman

Re: [RFR]: 8131334: SAAJ Plugability Layer: using java.util.ServiceLoader

[Daniel Fuchs]

On 19/11/15 18:31, Miroslav Kos wrote:

Hi Lance,

I added jtreg test(s) -
http://cr.openjdk.java.net/~mkos/8131334/jdk.01/index.html
There are several scenarios being tested, all without/with SM.

The test preparation I am doing in java (based on test args), so the
test creates/deletes $JAVA_HOME/conf/jaxm.properties.


Hi Miran,

I would expect this to fail horribly - you may not have the
right to create anything under $JAVA_HOME/conf - it may even
be a read-only file system... And I would be very uneasy as this
may have side effects on tests executing later on if anything
is left over in the java home conf dir...

best regards,

-- daniel




Hopefully it's correct approach.

Thanks
Miran



On 12/11/15 15:07, Lance Andersen wrote:

Not sure exactly what you want to set, but you can set system
properties or run tests from a script or from testNG

For running a script, see http://openjdk.java.net/jtreg/vmoptions.html

Best
Lance
On Nov 12, 2015, at 8:28 AM, Miroslav Kos mailto:miroslav@oracle.com>> wrote:


I have a set of tests in standalone project (using bash scripts), but
the problem is that to test the correct behavior, configuring of jdk
is necessary (property file) - have no idea if something like this is
possible with jtreg - are there any similat tests already?

Thanks
Miran

On 12/11/15 14:01, Lance @ Oracle wrote:

Hi,

Overall it looks ok,

I would however suggest adding a couple of tests to the jdk that run
with/out a security manager as a sanity check.

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 
Sent from my iPad

On Nov 12, 2015, at 3:55 AM, Miroslav Kos 
wrote:


.. anyone?

On 10/11/15 15:31, Miroslav Kos wrote:

Ping ... Would somebody find time for this one?

Thanks
M.


On 26/10/15 13:59, Miroslav Kos wrote:

Hi everybody,

I'd like to ask you for a review for
8131334: SAAJ Plugability Layer: using java.util.ServiceLoader

It is about changes in pluggability layer - there was a
proprietary service loading implementation which was not fully
compatible with to use java.util.ServiceLoader facility. There
are similar changes in all JAX-* APIs specs - JAX-B,  JAX-WS and
SAAJ.

JBS: https://bugs.openjdk.java.net/browse/JDK-8131334
webrev: http://cr.openjdk.java.net/~mkos/8131334/jaxws.01/
specdiff: http://cr.openjdk.java.net/~mkos/8131334/specdiff.09/

Testing - there are tests in standalone project to ensure there
that the old behavior is still supported.

Thanks
Miran












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(L): JDK-8046936 : JEP 270: Reserved Stack Areas for Critical Sections

[Doug Lea]

On 11/16/2015 08:00 PM, David Holmes wrote:

I've tried that :-) The amount of code being executed with
the ReservedStackAccess privilege tends to increase very
quickly and I was concerned about keeping the size of the
reserved area small.


How much space does each level of calling need? I know that is hard to answer
but are we talking a few bytes, few kb, many kb?



Relatedly, could someone spell out in more detail the
worst-case behavior of StackOverflowError? Knowing this
might be helpful in some cases in which we might be able to
at least preserve minimal sanity without needing to reserve.

-Doug

Re: [RFR]: 8131334: SAAJ Plugability Layer: using java.util.ServiceLoader

[Miroslav Kos]

Hi Lance,

I added jtreg test(s) - 
http://cr.openjdk.java.net/~mkos/8131334/jdk.01/index.html

There are several scenarios being tested, all without/with SM.

The test preparation I am doing in java (based on test args), so the 
test creates/deletes $JAVA_HOME/conf/jaxm.properties.


Hopefully it's correct approach.

Thanks
Miran



On 12/11/15 15:07, Lance Andersen wrote:
Not sure exactly what you want to set, but you can set system 
properties or run tests from a script or from testNG


For running a script, see http://openjdk.java.net/jtreg/vmoptions.html

Best
Lance
On Nov 12, 2015, at 8:28 AM, Miroslav Kos > wrote:


I have a set of tests in standalone project (using bash scripts), but 
the problem is that to test the correct behavior, configuring of jdk 
is necessary (property file) - have no idea if something like this is 
possible with jtreg - are there any similat tests already?


Thanks
Miran

On 12/11/15 14:01, Lance @ Oracle wrote:

Hi,

Overall it looks ok,

I would however suggest adding a couple of tests to the jdk that run 
with/out a security manager as a sanity check.


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 
Sent from my iPad

On Nov 12, 2015, at 3:55 AM, Miroslav Kos  
wrote:



.. anyone?

On 10/11/15 15:31, Miroslav Kos wrote:

Ping ... Would somebody find time for this one?

Thanks
M.


On 26/10/15 13:59, Miroslav Kos wrote:

Hi everybody,

I'd like to ask you for a review for
8131334: SAAJ Plugability Layer: using java.util.ServiceLoader

It is about changes in pluggability layer - there was a 
proprietary service loading implementation which was not fully 
compatible with to use java.util.ServiceLoader facility. There 
are similar changes in all JAX-* APIs specs - JAX-B,  JAX-WS and 
SAAJ.


JBS: https://bugs.openjdk.java.net/browse/JDK-8131334
webrev: http://cr.openjdk.java.net/~mkos/8131334/jaxws.01/
specdiff: http://cr.openjdk.java.net/~mkos/8131334/specdiff.09/

Testing - there are tests in standalone project to ensure there 
that the old behavior is still supported.


Thanks
Miran











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: Proposed API for JEP 259: Stack-Walking API

[Timo Kinnunen]

A point in favor of UnsupportedOperationException would be: if in the future it 
becomes possible to have large parts of the JVM written in Java or sun.tools 
becomes part of the J2SE API then having an instance of a VirtualMachine class 
as the caller of public static void main and Thread.run could be completely 
true and natural. Changing getCallerClass to support this and not throw an UOE 
feels less of a breaking change than it no longer throwing an 
IllegalStateException. Especially since no state that the caller can affect has 
actually changed!



Sent from Mail for Windows 10



From: John Rose
Sent: Thursday, November 19, 2015 05:45
To: David Holmes
Cc: OpenJDK Dev list
Subject: Re: Proposed API for JEP 259: Stack-Walking API


On Nov 18, 2015, at 6:32 PM, David Holmes  wrote:
> 
>>> 
>>> Looks good to me too if IllegalStateException is used instead of
>>> UnsupportedOperationException.
>>> UnsuppportedOperationException is used when the operation is not
>>> available, here, the same code can work or not depending how it is
>>> called.
>> 
>> But IllegalStateException is dependent on some state. There's no state
>> involved here (in the sense "state" is characterized in Java). My 1st
> 
> I agree with Remi. "state" doesn't have to mean fields - there are numerous 
> existing examples in the JDK. Calling a method in a context that is invalid 
> is an illegal state to me. IllegalThreadStateException would also work. But 
> UnsupportedOperationException ... more of a stretch.

+1

Re: RFR: JDK-8057804: AnnotatedType interfaces provide no way to get annotations on owner type

[Claes Redestad]

Hi Joel,

looks good. I see a CCC has been filed.

Nits:
In AnnotatedTypeFactory.java, newly introduced 
EMPTY_TYPE_ANNOTATION_ARRAY could be used to replace a few occurrences 
of new TypeAnnotation[0] in the code above).


In TypeAnnotation.java:
New method popLocation(byte tag) seems to be unused, and popLocation() 
is only used from popAllLocations(byte tag), so it might suffice to only 
keep popAllLocations:


public LocationInfo popAllLocations(byte tag) {
LocationInfo l = this;
int depth = l.depth;
while(depth > 0 && l.locations[depth - 1].tag == tag) {
depth--;
}
if (depth != l.depth) {
Location[] res = new Location[depth];
System.arraycopy(this.locations, 0, res, 0, depth);
return new LocationInfo(depth, res);
}
return l;
}

Not a reviewer, easy to ignore. :-)

/Claes

On 2015-11-17 20:15, Joel Borggrén-Franck wrote:

Hi,

When reflecting over annotated types, there is currently no way to get
the potentially annotated owner of a type. For example, given you have
an instance of '@A Outer . @B Inner' you can't traverse it to get '@A
Outer' .

This API addition fixes this. Because both parameterized and
non-generic types can have an owner, this addition goes into the base
AnnotatedType interface together with a default implementation. CCC
has been filed.

The parsing code and annotated type factory had to be fixed to deal
with navigating inside nested types.

Bug:   https://bugs.openjdk.java.net/browse/JDK-8057804
Webrev: http://cr.openjdk.java.net/~rbackman/jbf/8057804/webrev.00/


(OCA is signed and processed).

Cheers
/Joel

Re: RFR(L): 8139885: implement JEP 274: enhanced method handles

[Paul Sandoz]

HI Michael,

Nice work, not easy this area!


It may be worth considering, as a separate issue, updating the example section 
in JavaDoc to be under @apiNote.

I have a mild preference to maintain the 80 char limit for JavaDoc, to me it’s 
easier to read. For code i don’t mind a 100 or more limit.

Add @since 9 to new public methods.


MethodType
—

 470 /** Replace the last arrayLength parameter types with the component 
type of arrayType.
 471  * @param arrayType any array type
 472  * @param arrayLength the number of parameter types to change
 473  * @return the resulting type
 474  */

 475 /*non-public*/ MethodType asSpreaderType(Class arrayType, int pos, 
int arrayLength) {

 476 assert(parameterCount() >= arrayLength);

 477 int spreadPos = pos;

Might as well adjust the JavaDoc for the new pos parameter (as you have done 
for asCollectorType).


MethodHandles
—

 898  * @param spreadArgPos the position (zero-based index) in the argument 
list at which spreading should start.
 899  * @param arrayType usually {@code Object[]}, the type of the array 
argument from which to extract the spread arguments
 900  * @param arrayLength the number of arguments to spread from an 
incoming array argument
 901  * @return a new method handle which spreads an array argument at a 
given position,
 902  * before calling the original method handle
 903  * @throws NullPointerException if {@code arrayType} is a null 
reference
 904  * @throws IllegalArgumentException if {@code arrayType} is not an 
array type,
 905  * or if target does not have at least
 906  * {@code arrayLength} parameter types,
 907  * or if {@code arrayLength} is negative,
 908  * or if the resulting method handle's type would have
 909  * too many parameters

For the new asSpreader method, what are the constraints for spreadArgPos? What 
happens if < 0 or >= arrayLength for example?

Same applies to asCollector.

This is not in your patch but the assert in the following method is redundant 
(as reported by the IDE, it’s really handy sometimes apply the patch and view 
via diffs in the IDE), as DirectMethodHandle is not a subtype of 
BoundMethodHandle:

MethodHandle viewAsType(MethodType newType, boolean strict) {
// No actual conversions, just a new view of the same method.
// Note that this operation must not produce a DirectMethodHandle,
// because retyped DMHs, like any transformed MHs,
// cannot be cracked into MethodHandleInfo.
assert viewAsTypeChecks(newType, strict);
BoundMethodHandle mh = rebind();
assert(!((MethodHandle)mh instanceof DirectMethodHandle));
return mh.copyWith(newType, mh.form);
}


 958 /**
 959  * Determines if a class can be accessed from the lookup context 
defined by this {@code Lookup} object. The
 960  * static initializer of the class is not run.
 961  *
 962  * @param targetClass the class to be accessed
 963  *
 964  * @return the class that has been accessed

Has it really been accessed by “accessClass” or just checked that it can be 
accessed?


 965  *
 966  * @throws IllegalAccessException if the class is not accessible 
from the lookup class, using the allowed access
 967  * modes.
 968  * @exception SecurityException if a security manager is present 
and it
 969  *  refuses access
 970  */
 971 public Class accessClass(Class targetClass) throws 
IllegalAccessException {



Great use of streams in MethodHandles.loop and good correlation with the 
specified steps in the JavaDoc to that in the code. Really clear and well 
specified.



3458  * The loop handle's result type is the same as the sole loop 
variable's, i.e., the result type of {@code init}.

s/variable’s/variable ?


3488  * // assume MH_initZip, MH_zipPred, and MH_zipStep are handles to the 
above methods
3489  * MethodHandle loop = MethodHandles.doWhileLoop(MH_initZip, 
MH_zipPred, MH_zipStep);
3490  * List a = Arrays.asList(new String[]{"a", "b", "c", "d"});
3491  * List b = Arrays.asList(new String[]{"e", "f", "g", "h"});
3492  * List zipped = Arrays.asList(new String[]{"a", "e", "b", 
"f", "c", "g", "d", "h"});
3493  * assertEquals(zipped, (List) loop.invoke(a.iterator(), 
b.iterator()));
3494  * }

You don’t need to create an explicit array and pass that into Arrays.asList. 
Same for other related methods.



MethodHandleImpl
—

1720 static MethodHandle makeLoop(Class tloop, List> targs, 
List> tvars, List init,
1721  List step, 
List pred, List fini) {
1722 MethodHandle[] ainit = 
toArrayArgs(init).toArray(MH_ARRAY_SENTINEL);
1723 MethodHandle[] astep = 
toArrayArgs(step).toArray(MH_ARRAY_SENTINEL);
1724 MethodHandle[] apred = 
toArrayArgs(pred).toArra