Re: RFR: 8232613: Move Object.registerNatives into HotSpot

2019-10-22 Thread David Holmes
On 23/10/2019 7:52 am, Mandy Chung wrote: On 10/22/19 8:03 AM, Claes Redestad wrote: http://cr.openjdk.java.net/~redestad/8232613/open.03 Looks good to me. Filed and drafted a CSR: https://bugs.openjdk.java.net/browse/JDK-8232801 I think it'd be good to include the IAE message as an

Re: RFR: 8232613: Move Object.registerNatives into HotSpot

2019-10-22 Thread Mandy Chung
On 10/22/19 8:03 AM, Claes Redestad wrote: http://cr.openjdk.java.net/~redestad/8232613/open.03 Looks good to me. Filed and drafted a CSR: https://bugs.openjdk.java.net/browse/JDK-8232801 I think it'd be good to include the IAE message as an example. Is there a CSR that documents the

Re: RFR: 8232613: Move Object.registerNatives into HotSpot

2019-10-22 Thread Claes Redestad
On 2019-10-22 19:09, Lois Foltan wrote: Filed and drafted a CSR: https://bugs.openjdk.java.net/browse/JDK-8232801 Hi Claes, You have my review for webrev .03. Thanks! Thanks for adding the test case.  I agree with David that a CSR is warranted.  Only minor comment I have on it is

Re: RFR: 8232613: Move Object.registerNatives into HotSpot

2019-10-22 Thread Lois Foltan
On 10/22/2019 11:03 AM, Claes Redestad wrote: On 2019-10-22 15:55, David Holmes wrote: With your change interface I's registerNatives default method is invoked successfully.  I don't think this is a major backward compatibilty issue but we should have someone from core-libs okay the removal

Re: RFR: 8232613: Move Object.registerNatives into HotSpot

2019-10-22 Thread Claes Redestad
On 2019-10-22 15:55, David Holmes wrote: With your change interface I's registerNatives default method is invoked successfully.  I don't think this is a major backward compatibilty issue but we should have someone from core-libs okay the removal of the method from Object before committing. 

Re: RFR: 8232613: Move Object.registerNatives into HotSpot

2019-10-22 Thread David Holmes
On 22/10/2019 11:41 pm, Claes Redestad wrote: Hi Lois, to make sure noone misses it, here's the new webrev: http://cr.openjdk.java.net/~redestad/8232613/open.03 I like the code placement in this version - Coleen's suggestion was better than mine :) Comments inline.. Comments inline ...

Re: RFR: 8232613: Move Object.registerNatives into HotSpot

2019-10-22 Thread Claes Redestad
Hi Lois, to make sure noone misses it, here's the new webrev: http://cr.openjdk.java.net/~redestad/8232613/open.03 Comments inline.. On 2019-10-22 14:33, Lois Foltan wrote: On 10/22/2019 7:13 AM, Claes Redestad wrote: On 2019-10-22 13:05, Andrew Dinn wrote: On 22/10/2019 12:05, Claes

Re: RFR: 8232613: Move Object.registerNatives into HotSpot

2019-10-22 Thread David Holmes
Hi Claes, On 22/10/2019 8:38 pm, Claes Redestad wrote: Hi David, On 2019-10-22 01:08, David Holmes wrote: Hi Claes, My only nit with this is that I don't think register_native and friends belongs in the SystemDictionary class as it has nothing to do with the SD. This code seems to be all

Re: RFR: 8232613: Move Object.registerNatives into HotSpot

2019-10-22 Thread Andrew Dinn
On 22/10/2019 12:13, Claes Redestad wrote: > No wait, I missed the use in jni.cpp where the bool return is actually > used. I ignored it in systemDict since when I got an exception there > (by misspelling a name or using the wrong signature) the exception > bubbles up and crashes the VM. > >

Re: RFR: 8232613: Move Object.registerNatives into HotSpot

2019-10-22 Thread Lois Foltan
On 10/22/2019 7:13 AM, Claes Redestad wrote: On 2019-10-22 13:05, Andrew Dinn wrote: On 22/10/2019 12:05, Claes Redestad wrote: On 2019-10-22 12:44, Andrew Dinn wrote: Why not leave it void? I guess: http://cr.openjdk.java.net/~redestad/8232613/open.02/ Ship it ;-) No wait, I missed

Re: RFR: 8232613: Move Object.registerNatives into HotSpot

2019-10-22 Thread Claes Redestad
On 2019-10-22 13:05, Andrew Dinn wrote: On 22/10/2019 12:05, Claes Redestad wrote: On 2019-10-22 12:44, Andrew Dinn wrote: Why not leave it void? I guess: http://cr.openjdk.java.net/~redestad/8232613/open.02/ Ship it ;-) No wait, I missed the use in jni.cpp where the bool return is

Re: RFR: 8232613: Move Object.registerNatives into HotSpot

2019-10-22 Thread Andrew Dinn
On 22/10/2019 12:05, Claes Redestad wrote: > On 2019-10-22 12:44, Andrew Dinn wrote: >> Why not leave it void? > > I guess: > > http://cr.openjdk.java.net/~redestad/8232613/open.02/ Ship it ;-) regards, Andrew Dinn --- Senior Principal Software Engineer Red Hat UK Ltd Registered in

Re: RFR: 8232613: Move Object.registerNatives into HotSpot

2019-10-22 Thread Claes Redestad
On 2019-10-22 12:44, Andrew Dinn wrote: On 22/10/2019 11:38, Claes Redestad wrote: how about this: http://cr.openjdk.java.net/~redestad/8232613/open.01/ Sure that works. It's a bit more obvious now though that you are pointlessly returning a boolean from Method::register_native (this was

Re: RFR: 8232613: Move Object.registerNatives into HotSpot

2019-10-22 Thread Andrew Dinn
On 22/10/2019 11:38, Claes Redestad wrote: > how about this: > > http://cr.openjdk.java.net/~redestad/8232613/open.01/ Sure that works. It's a bit more obvious now though that you are pointlessly returning a boolean from Method::register_native (this was also there in the previous version). Why

Re: RFR: 8232613: Move Object.registerNatives into HotSpot

2019-10-22 Thread Claes Redestad
Hi David, On 2019-10-22 01:08, David Holmes wrote: Hi Claes, My only nit with this is that I don't think register_native and friends belongs in the SystemDictionary class as it has nothing to do with the SD. This code seems to be all about Methods so that seems like the place to put this.

Re: RFR: 8232613: Move Object.registerNatives into HotSpot

2019-10-22 Thread David Holmes
Hi Andrew, On 22/10/2019 6:11 pm, Andrew Dinn wrote: Hi Claes/David, On 22/10/2019 00:08, David Holmes wrote: My only nit with this is that I don't think register_native and friends belongs in the SystemDictionary class as it has nothing to do with the SD. This code seems to be all about

Re: RFR: 8232613: Move Object.registerNatives into HotSpot

2019-10-22 Thread Andrew Dinn
Hi Claes/David, On 22/10/2019 00:08, David Holmes wrote: > My only nit with this is that I don't think register_native and friends > belongs in the SystemDictionary class as it has nothing to do with the > SD. This code seems to be all about Methods so that seems like the place > to put this.

Re: RFR: 8232613: Move Object.registerNatives into HotSpot

2019-10-21 Thread David Holmes
Hi Claes, My only nit with this is that I don't think register_native and friends belongs in the SystemDictionary class as it has nothing to do with the SD. This code seems to be all about Methods so that seems like the place to put this. Thanks, David On 22/10/2019 12:55 am, Claes

RFR: 8232613: Move Object.registerNatives into HotSpot

2019-10-21 Thread Claes Redestad
Hi, Object.java currently registers various native functions via the registerNatives facility. private static native void registerNatives(); static { registerNatives(); } Not costly in and off itself, but this has the side effect that these two methods are taken into