Jigsaw Enhancement RFR: 8159145 Add JVMTI function GetModuleByPackageName

[Christian Tornqvist]

Hi Serguei,

 

>Will check if this works well in this case.

>The unique error messages are already printed in the native code.

 

Yes, they’re printed on stdout/stderr but the test will fail with a “Test failed” exception and our failure matching system wouldn’t be able to tell why it failed. It’d be a lot nicer if it’d just throw an exception when one of the checks in the native code failed.

 

Thanks,

Christian

 

From: serguei.spitsyn at oracle.com [mailto:serguei.spitsyn at oracle.com] 
Sent: Tuesday, June 21, 2016 10:59 AM
To: Christian Tornqvist <christian.tornqvist at oracle.com>; 'Alan Bateman' <Alan.Bateman at oracle.com>; serviceability-dev at openjdk.java.net; 'Alexander Kulyakhtin' <alexander.kulyakhtin at oracle.com>; hotspot-dev at openjdk.java.net
Subject: Re: Jigsaw Enhancement RFR: 8159145 Add JVMTI function GetModuleByPackageName

 

Hi Christian,

Thank you for looking at it!


On 6/21/16 04:56, Christian Tornqvist wrote:

Hi Serguei,

 

I’ve only looked at the test code so far, here are some comments:

 

* You’re not allowed to call System.exit in a jtreg test. If something fails, you need to throw an exception.


Ok, an exception is Ok with me.




* Instead of a run() method you could simply collapse all this into the main method like this:

 

assertEQ(check(), 0);


Will check if this works for me.







 

* Would it make sense to throw exceptions with useful error messages from the JNI code instead of returning failed status codes? Our error matching system would work a lot better with unique error messages.


Will check if this works well in this case.
The unique error messages are already printed in the native code. 

Thanks,
Serguei




 

Thanks,

Christian

 

From: serguei.spitsyn at oracle.com <mailto:serguei.spitsyn at oracle.com>  [mailto:serguei.spitsyn at oracle.com] 
Sent: Tuesday, June 21, 2016 6:49 AM
To: Alan Bateman  <mailto:Alan.Bateman at oracle.com> <Alan.Bateman at oracle.com>; serviceability-dev at openjdk.java.net <mailto:serviceability-dev at openjdk.java.net> ; Christian Törnqvist  <mailto:Christian.Tornqvist at oracle.com> <Christian.Tornqvist at oracle.com>; Alexander Kulyakhtin  <mailto:alexander.kulyakhtin at oracle.com> <alexander.kulyakhtin at oracle.com>; hotspot-dev at openjdk.java.net <mailto:hotspot-dev at openjdk.java.net>  Developers  <mailto:hotspot-dev at openjdk.java.net> <hotspot-dev at openjdk.java.net>
Subject: Re: Jigsaw Enhancement RFR: 8159145 Add JVMTI function GetModuleByPackageName

 

On 6/21/16 03:23, Alan Bateman wrote:

 

 

On 21/06/2016 10:54, serguei.spitsyn at oracle.com <mailto:serguei.spitsyn at oracle.com>  wrote:


Please, review the Jigsaw fix for the enhancement:
  https://bugs.openjdk.java.net/browse/JDK-8159145


The Hotspot webrev:
  http://javaweb.sfbay.sun.com/java/svc/ss45998/webrevs/2016/hotspot/8159145-jigsaw-jvmti-pkg.1/

The Jdk webrev:
  http://javaweb.sfbay.sun.com/java/svc/ss45998/webrevs/2016/hotspot/8159145-jigsaw-jvmti-pkg.jdk1/

Serguei - can you put the patches on cr.openjdk.java.net?


Sorry.
My initial plan was to write an nsk.jvmti test and review it on a confidential mailing list.
It is why I put the webrevs on the non-public server.
Forgot to switch the server when the test was converted into the jtreg format.

The public webrevs are:

Hotspot:
  http://cr.openjdk.java.net/~sspitsyn/webrevs/2016/hotspot/8159145-jigsaw-jvmti-pkg.1/ <http://cr.openjdk.java.net/%7Esspitsyn/webrevs/2016/hotspot/8159145-jigsaw-jvmti-pkg.1/> 

Jdk:
  http://cr.openjdk.java.net/~sspitsyn/webrevs/2016/hotspot/8159145-jigsaw-jvmti-pkg.jdk1/ <http://cr.openjdk.java.net/%7Esspitsyn/webrevs/2016/hotspot/8159145-jigsaw-jvmti-pkg.jdk1/> 


Thanks,
Serguei







-Alan