Re: RFR(M): JDK-8041498 Remove npt library
Dmitry, src/share/demo/jvmti/hprof/hprof_init.c L743-744: Indentation looks wrong src/share/back/utf_util.h L1: Missing copyright header. L14: UTF_ERROR and UTF_ASSERT are only used in utf_util.c, so they can be defined in that file instead of the header file. src/share/back/utf_util.c L297: Wouldn’t it make sense to cache the result of getCodepage() in a static variable so we don’t need to call this code for each UTF conversion? L492: skeep - skip L492: This TODO should be fixed. I also have the same worry here as for the windows code: that we should cache data so that we don’t run this unnecessarily. In the original code we did: /* Set the locale from the environment */ (void)setlocale(LC_ALL, ”); but that is missing from the new code. I don’t know if it matters? Thanks, /Staffan On 23 apr 2014, at 17:03, Dmitry Samersoff dmitry.samers...@oracle.com wrote: Hi Everybody, Please review the fix that removes npt library from jdk. CR link: https://bugs.openjdk.java.net/browse/JDK-8041498 Webrev: http://cr.openjdk.java.net/~dsamersoff/JDK-8041498/webrev.01/ Testing: nsk.jdwp I plan to submit a separate aurora run when I get all concerns. Fix details: * Today libnpt is a set of UTF related function and only few of them is actually used. The only users of npt is JDWP and hprof. So it's not necessary to keep it as a separate library. * Relevant code moved from npt to src/share/back/utf_util.c platfrom specific part is under #ifdef _WINDOWS initialization step is removed. * hprof changed to don't do any conversion. The only place where hrpof care about encoding is an output filename. Conversion from platform to UTF8 assume that filesystem does support UTF8 but command line doesn't. This assumption is obviously not correct in all cases and therefore handling of non-ascii filenames is incomplete. We should file a separate RFE if we need to handle no-ascii filenames by hprof. -Dmitry -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * I would love to change the world, but they won't give me the sources.
Re: RFR(M): JDK-8041498 Remove npt library
On 23/04/2014 16:03, Dmitry Samersoff wrote: Hi Everybody, Please review the fix that removes npt library from jdk. No objection to removing the npt library but what is the motivation? -Alan.
Re: RFR(M): JDK-8041498 Remove npt library
Alan, No objection to removing the npt library but what is the motivation? It's the part of the test stabilization. Next step is revisit UTF8 usage in JDWP and fix related tests when necessary. -Dmitry On 2014-04-24 14:43, Alan Bateman wrote: On 23/04/2014 16:03, Dmitry Samersoff wrote: Hi Everybody, Please review the fix that removes npt library from jdk. No objection to removing the npt library but what is the motivation? -Alan. -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * I would love to change the world, but they won't give me the sources.
Re: RFR(M): JDK-8041498 Remove npt library
On 24/04/2014 12:34, Dmitry Samersoff wrote: Alan, No objection to removing the npt library but what is the motivation? It's the part of the test stabilization. Next step is revisit UTF8 usage in JDWP and fix related tests when necessary. Could you clarify this a bit more? It's not initially obvious how this is related to tests. -Alan
Re: RFR(M): JDK-8041498 Remove npt library
Alan, On 2014-04-24 15:39, Alan Bateman wrote: On 24/04/2014 12:34, Dmitry Samersoff wrote: Alan, No objection to removing the npt library but what is the motivation? It's the part of the test stabilization. Next step is revisit UTF8 usage in JDWP and fix related tests when necessary. Could you clarify this a bit more? It's not initially obvious how this is related to tests. We have couple (about two hundreds) of intermittent JDWP test failures that have to be investigated. Some of them might be related to NPT initialization and deinitialization code (no strong evidence, just a speculation for today). Besides that there are couple of places in NPT-related code that should be re-evaluated and probably fixed. So I have an impression that if I clean it up today it saves me debugging efforts later. -Dmitry -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * I would love to change the world, but they won't give me the sources.
RFR 8031195: Support default and static interface methods in JDI, JDWP and JDB
Please, review the following patch: Issue : https://bugs.openjdk.java.net/browse/JDK-8031195 Webrev: http://cr.openjdk.java.net/~jbachorik/8031195/webrev.04 With JDK8 it became possible to have methods with implementation in interfaces - static and default interface methods. However the JDI and JDWP were not updated to reflect these capabilities so it is not currently possible to invoke a static or default interface method programatically from the debugger. This patch adds support for static and default interface methods to JDI, JDWP and JDB. Thanks, -JB-
Re: RFR 8031195: Support default and static interface methods in JDI, JDWP and JDB
Jaroslav, It looks like you are adding APIs here so you'll need to bump at least the minor version numbers here. Also, I think you need a CCC request for this work. Dan On 4/24/14 6:52 AM, Jaroslav Bachorik wrote: Please, review the following patch: Issue : https://bugs.openjdk.java.net/browse/JDK-8031195 Webrev: http://cr.openjdk.java.net/~jbachorik/8031195/webrev.04 With JDK8 it became possible to have methods with implementation in interfaces - static and default interface methods. However the JDI and JDWP were not updated to reflect these capabilities so it is not currently possible to invoke a static or default interface method programatically from the debugger. This patch adds support for static and default interface methods to JDI, JDWP and JDB. Thanks, -JB-
Re: RFR 8031195: Support default and static interface methods in JDI, JDWP and JDB
I can see version numbers bumped in com/sun/tools/jdi/VirtualMachineManagerImpl.java as well as in src/share/back/VirtualMachineImpl.c. Are there other places that need fixing? A CCC has been filed and approved. Thanks, /Staffan On 24 apr 2014, at 15:57, Daniel D. Daugherty daniel.daughe...@oracle.com wrote: Jaroslav, It looks like you are adding APIs here so you'll need to bump at least the minor version numbers here. Also, I think you need a CCC request for this work. Dan On 4/24/14 6:52 AM, Jaroslav Bachorik wrote: Please, review the following patch: Issue : https://bugs.openjdk.java.net/browse/JDK-8031195 Webrev: http://cr.openjdk.java.net/~jbachorik/8031195/webrev.04 With JDK8 it became possible to have methods with implementation in interfaces - static and default interface methods. However the JDI and JDWP were not updated to reflect these capabilities so it is not currently possible to invoke a static or default interface method programatically from the debugger. This patch adds support for static and default interface methods to JDI, JDWP and JDB. Thanks, -JB-
Re: RFR 8031195: Support default and static interface methods in JDI, JDWP and JDB
On 4/24/14 8:09 AM, Staffan Larsen wrote: I can see version numbers bumped in com/sun/tools/jdi/VirtualMachineManagerImpl.java as well as in src/share/back/VirtualMachineImpl.c. Are there other places that need fixing? No. I redid my searches of the patch and I see them now. This is what I get for trying to do a quick search and post a quick comment without having reviewed the whole webrev. I don't normally do things that way and I apologize for the noise. A CCC has been filed and approved. I would have mentioned that a CCC has been approved and probably included a line apologizing that the CCC process is still not accessible external to Oracle. Dan Thanks, /Staffan On 24 apr 2014, at 15:57, Daniel D. Daugherty daniel.daughe...@oracle.com wrote: Jaroslav, It looks like you are adding APIs here so you'll need to bump at least the minor version numbers here. Also, I think you need a CCC request for this work. Dan On 4/24/14 6:52 AM, Jaroslav Bachorik wrote: Please, review the following patch: Issue : https://bugs.openjdk.java.net/browse/JDK-8031195 Webrev: http://cr.openjdk.java.net/~jbachorik/8031195/webrev.04 With JDK8 it became possible to have methods with implementation in interfaces - static and default interface methods. However the JDI and JDWP were not updated to reflect these capabilities so it is not currently possible to invoke a static or default interface method programatically from the debugger. This patch adds support for static and default interface methods to JDI, JDWP and JDB. Thanks, -JB-
Re: RFR 8031195: Support default and static interface methods in JDI, JDWP and JDB
On 24.4.2014 16:16, Daniel D. Daugherty wrote: On 4/24/14 8:09 AM, Staffan Larsen wrote: I can see version numbers bumped in com/sun/tools/jdi/VirtualMachineManagerImpl.java as well as in src/share/back/VirtualMachineImpl.c. Are there other places that need fixing? No. I redid my searches of the patch and I see them now. This is what I get for trying to do a quick search and post a quick comment without having reviewed the whole webrev. I don't normally do things that way and I apologize for the noise. A CCC has been filed and approved. I would have mentioned that a CCC has been approved and probably included a line apologizing that the CCC process is still not accessible external to Oracle. Sorry, the CCC has already been approved. And I apologize that the CCC process is still not an open one but I'm afraid it is beyond my powers to make it so. -JB- Dan Thanks, /Staffan On 24 apr 2014, at 15:57, Daniel D. Daugherty daniel.daughe...@oracle.com wrote: Jaroslav, It looks like you are adding APIs here so you'll need to bump at least the minor version numbers here. Also, I think you need a CCC request for this work. Dan On 4/24/14 6:52 AM, Jaroslav Bachorik wrote: Please, review the following patch: Issue : https://bugs.openjdk.java.net/browse/JDK-8031195 Webrev: http://cr.openjdk.java.net/~jbachorik/8031195/webrev.04 With JDK8 it became possible to have methods with implementation in interfaces - static and default interface methods. However the JDI and JDWP were not updated to reflect these capabilities so it is not currently possible to invoke a static or default interface method programatically from the debugger. This patch adds support for static and default interface methods to JDI, JDWP and JDB. Thanks, -JB-
RFR(XXS): Event Based tracing framework trace_id's to be reassigned for CDS klasses
Greetings, Kindly asking for reviews for the following very small fix: Bug: https://bugs.openjdk.java.net/browse/JDK-8041723 Webrev: http://cr.openjdk.java.net/~mgronlun/8041723/webrev01/ Description: The Event Based tracing framework assigns a unique traceid to Klass:es for tracking purposes. Normally, a new Klass is assigned it's traceid inside the Klass constructor. For Klass:es coming into the system via the ClassDataSharing (CDS) mechanism, the old traceid for the Klass will be stale, hence a new traceid needs to be (re)assigned to the Klass. Thank you Markus
Re: RFR(XXS): Event Based tracing framework trace_id's to be reassigned for CDS klasses
Hi Markus, On 2014-04-24 17:42, Markus Grönlund wrote: Greetings, Kindly asking for reviews for the following very small fix: Bug: https://bugs.openjdk.java.net/browse/JDK-8041723 Webrev: http://cr.openjdk.java.net/~mgronlun/8041723/webrev01/ http://cr.openjdk.java.net/%7Emgronlun/8041723/webrev01/ Klass::restore_unshareable_info() might be called multiple times for a given Klass. This can happen if OutOfMemoryErrors is thrown when the Klass is loaded, and we later retry to load the Klass. Is it OK to call TRACE_INIT_ID(this) multiple times for the same Klass? thanks, StefanK Description: The Event Based tracing framework assigns a unique traceid to Klass:es for tracking purposes. Normally, a new Klass is assigned it's traceid inside the Klass constructor. For Klass:es coming into the system via the ClassDataSharing (CDS) mechanism, the old traceid for the Klass will be stale, hence a new traceid needs to be (re)assigned to the Klass. Thank you Markus
Re: RFR 8040748: [TESTBUG] Exclude failing (serviceability) jtreg tests using @ignore
I've now added the keyword quarantine as well as fixed an issue with placement of the @ignore tag (it's not allowed to come before @library). New webrev at http://cr.openjdk.java.net/~miauno/8040748/webrev.01/ Local testing: % jtreg -jdk $JAVA_HOME -k:quarantine -ignore:quiet -verbose:summary /localhome/ws/jdk9-dev/jdk/test/ Test results: no tests selected Report written to /localhome/temp/run/JTreport/html/report.html Results written to /localhome/temp/run/JTwork Thanks, Mikael On 2014-04-22 17:12, Stefan Särne wrote: Hi Mikael, I think we should use a key word to group quarantined tests, which can be used to run the tests as a separate batch. Recommend to add this to all tests: @key quarantine Note that you may have to add the key word as a known key word to the TEST.ROOT file as well. Best regards, Stefan -Original Message- From: Mikael Auno Sent: den 16 april 2014 19:51 To: serviceability-dev@openjdk.java.net Subject: RFR 8040748: [TESTBUG] Exclude failing (serviceability) jtreg tests using @ignore Please, review the following fix adding the @ignore tag to a couple of serviceability tests Issue: https://bugs.openjdk.java.net/browse/JDK-8040748 Webrev: http://cr.openjdk.java.net/~miauno/8040748/webrev.00/ Thanks, Mikael
Re: RFR(M): JDK-8041498 Remove npt library
Staffan, After some whitebox testing, I decided to change conversion code to have consistent error handling and consistent behavior of UNIX and windows parts. http://cr.openjdk.java.net/~dsamersoff/JDK-8041498/webrev.02/ (besides fixed nits only utf_util.c is changed) Changes details: 1. Original code try to convert to/from ANSI_X3.4-1968 (default value returned by nl_langinfo) if no locale set and it's not correct. Modified code check result of setlocale() and don't convert if no locale is set. 2. Original code leaves output empty or uninitialized in case of conversion error. Callers never checks for -1 returned on conversion error so it can cause weird failures. Modified code returns copy of original string in case of conversion error. Also fixed nits below. On 2014-04-24 13:52, Staffan Larsen wrote: src/share/demo/jvmti/hprof/hprof_init.c L743-744: Indentation looks wrong fixed. src/share/back/utf_util.h L1: Missing copyright header. fixed. L14: UTF_ERROR and UTF_ASSERT are only used in utf_util.c, so they can be defined in that file instead of the header file. fixed src/share/back/utf_util.c L297: Wouldn’t it make sense to cache the result of getCodepage() in a static variable so we don’t need to call this code for each UTF conversion? fixed. L492: skeep - skip L492: This TODO should be fixed. I also have the same worry here as for the windows code: that we should cache data so that we don’t run this unnecessarily. fixed In the original code we did: /* Set the locale from the environment */ (void)setlocale(LC_ALL, ”); but that is missing from the new code. I don’t know if it matters? It was intentional, but finally after some whitebox testing I decided to rewrite conversion code. See above. -Dmitry Thanks, /Staffan On 23 apr 2014, at 17:03, Dmitry Samersoff dmitry.samers...@oracle.com wrote: Hi Everybody, Please review the fix that removes npt library from jdk. CR link: https://bugs.openjdk.java.net/browse/JDK-8041498 Webrev: http://cr.openjdk.java.net/~dsamersoff/JDK-8041498/webrev.01/ Testing: nsk.jdwp I plan to submit a separate aurora run when I get all concerns. Fix details: * Today libnpt is a set of UTF related function and only few of them is actually used. The only users of npt is JDWP and hprof. So it's not necessary to keep it as a separate library. * Relevant code moved from npt to src/share/back/utf_util.c platfrom specific part is under #ifdef _WINDOWS initialization step is removed. * hprof changed to don't do any conversion. The only place where hrpof care about encoding is an output filename. Conversion from platform to UTF8 assume that filesystem does support UTF8 but command line doesn't. This assumption is obviously not correct in all cases and therefore handling of non-ascii filenames is incomplete. We should file a separate RFE if we need to handle no-ascii filenames by hprof. -Dmitry -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * I would love to change the world, but they won't give me the sources. -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * I would love to change the world, but they won't give me the sources.
