Re: RFR: JDK-8145099 Better error message when SA can't attach to a process

2015-12-10 Thread Thomas Stüfe
Hi Dmitry, > Or just don't care of threads and use plain strerror - it widely used in > hotspot and it doesn't crash on a race. > > Unfortunately, it does: https://bugs.openjdk.java.net/browse/JDK-8133249 Regards, Thomas > >

Re: RFR: 8145153: Convert TraceMonitorInflation to Unified Logging

2015-12-10 Thread David Holmes
Hi Rachel, On 11/12/2015 8:21 AM, Rachel Protacio wrote: Hello! Please review another product flag logging change. This one's pretty small. Converts TraceMonitorInflation to -Xlog:monitorinflation=debug Conversion looks fine. internally, with an alias for the old option. I've again patched

RFR: 8145153: Convert TraceMonitorInflation to Unified Logging

2015-12-10 Thread Rachel Protacio
Hello! Please review another product flag logging change. This one's pretty small. Converts TraceMonitorInflation to -Xlog:monitorinflation=debug internally, with an alias for the old option. I've again patched in Max's alias table code in the arguments.cpp and .hpp files since my previous ch

Re: RFR: JDK-8145099 Better error message when SA can't attach to a process

2015-12-10 Thread Dmitry Samersoff
Staffan, 1. strerror_r comes in two versions - returning string (GNU) and returning int (XSI) To make developers live more interesting GNU version doesn't guarantee to fill buf with appropriate string, so you can't just void return value. Default version tends to be XSI now days and I'm not sur

Re: [RFR] (M) 8143608: Don't 64-bit align start of InstanceKlass vtable, itable, and nonstatic_oopmap on 32-bit systems

2015-12-10 Thread Chris Plummer
Hi Mikael, See comments inline below: On 12/9/15 8:48 AM, Mikael Gerdin wrote: On 2015-12-08 20:14, Chris Plummer wrote: [Adding serviceability-dev@openjdk.java.net] Hi Mikael, Thanks for pointing this out. I'll look into it some more. Are there any tests that should be failing as a result o

Re: RFR: JDK-5108778 Too many instances of java.lang.Boolean created in Java application(hotspot)

2015-12-10 Thread Staffan Larsen
> On 10 dec. 2015, at 17:58, Sebastian Sickelmann > wrote: > > Thanks for the reviews so far. > > I am not sure how the serviceability thing need to be handled. Should > there be a review-result of a serviceability-member? > I think nether david or staffan are members of the serviceability-gro

Re: RFR: JDK-5108778 Too many instances of java.lang.Boolean created in Java application(hotspot)

2015-12-10 Thread Sebastian Sickelmann
Thanks for the reviews so far. I am not sure how the serviceability thing need to be handled. Should there be a review-result of a serviceability-member? I think nether david or staffan are members of the serviceability-group. If it is not the case here would be the changeset so far. As I am not

Re: RFR: JDK-8145099 Better error message when SA can't attach to a process

2015-12-10 Thread Staffan Larsen
Thanks! > On 10 dec. 2015, at 15:26, Jaroslav Bachorik > wrote: > > On 10.12.2015 15:22, Staffan Larsen wrote: >> Hi Thommas, >> >>> On 10 dec. 2015, at 14:49, Thomas Stüfe >> > wrote: >>> >>> Hi Stefan, >>> >>> Disclaimer: not a "R"eviewer. >>> >>> http://cr

Re: RFR: JDK-8145099 Better error message when SA can't attach to a process

2015-12-10 Thread Staffan Larsen
Thanks! > On 10 dec. 2015, at 15:56, Thomas Stüfe wrote: > > Hi Staffan, > > this looks fine now, thanks! > > ..Thomas > > On Thu, Dec 10, 2015 at 3:42 PM, Staffan Larsen > wrote: > Hi Thomas, > >> On 10 dec. 2015, at 15:27, Thomas Stüfe >

Re: RFR: JDK-8145099 Better error message when SA can't attach to a process

2015-12-10 Thread Thomas Stüfe
Hi Staffan, this looks fine now, thanks! ..Thomas On Thu, Dec 10, 2015 at 3:42 PM, Staffan Larsen wrote: > Hi Thomas, > > On 10 dec. 2015, at 15:27, Thomas Stüfe wrote: > > Hi Staffan, > > thanks! > > It also just occurred to me that strerror is not considered threadsafe. > Maybe strerror_r()

Re: RFR: JDK-5108778 Too many instances of java.lang.Boolean created in Java application(hotspot)

2015-12-10 Thread Staffan Larsen
Looks good! Thanks, /Staffan > On 10 dec. 2015, at 15:44, Coleen Phillimore > wrote: > > > Adding serviceability-dev. They work on this code. > Coleen > > > On 12/10/15 12:38 AM, Sebastian Sickelmann wrote: >> @Adding hotspot-runtime-dev >> >> Hi, >> >> a want to restart a discussion/rev

Re: RFR: JDK-5108778 Too many instances of java.lang.Boolean created in Java application(hotspot)

2015-12-10 Thread Coleen Phillimore
Adding serviceability-dev. They work on this code. Coleen On 12/10/15 12:38 AM, Sebastian Sickelmann wrote: @Adding hotspot-runtime-dev Hi, a want to restart a discussion/review-process for on old "bug" JDK-5108778. I created a webrev which is based on the jdk9/dev repo: http://cr.openjdk.

Re: RFR: JDK-8145099 Better error message when SA can't attach to a process

2015-12-10 Thread Staffan Larsen
Hi Thomas, > On 10 dec. 2015, at 15:27, Thomas Stüfe wrote: > > Hi Staffan, > > thanks! > > It also just occurred to me that strerror is not considered threadsafe. Maybe > strerror_r() would a better option (depending on the version you use you > would have to check the return value for NUL

Re: RFR: JDK-8145099 Better error message when SA can't attach to a process

2015-12-10 Thread Thomas Stüfe
Hi Staffan, thanks! It also just occurred to me that strerror is not considered threadsafe. Maybe strerror_r() would a better option (depending on the version you use you would have to check the return value for NULL, though). sorry for this piecemeal review. Kind Regards, Thomas On Thu, Dec

Re: RFR: JDK-8145099 Better error message when SA can't attach to a process

2015-12-10 Thread Jaroslav Bachorik
On 10.12.2015 15:22, Staffan Larsen wrote: Hi Thommas, On 10 dec. 2015, at 14:49, Thomas Stüfe mailto:thomas.stu...@gmail.com>> wrote: Hi Stefan, Disclaimer: not a "R"eviewer. http://cr.openjdk.java.net/~sla/JDK-8145099/webrev.01/agent/src/os/linux/LinuxDebuggerLocal.c.udiff.html I am not s

Re: RFR: JDK-8145099 Better error message when SA can't attach to a process

2015-12-10 Thread Staffan Larsen
Hi Thommas, > On 10 dec. 2015, at 14:49, Thomas Stüfe wrote: > > Hi Stefan, > > Disclaimer: not a "R"eviewer. > > http://cr.openjdk.java.net/~sla/JDK-8145099/webrev.01/agent/src/os/linux/LinuxDebuggerLocal.c.udiff.html > >

Re: RFR: JDK-8145099 Better error message when SA can't attach to a process

2015-12-10 Thread Thomas Stüfe
Hi Stefan, Disclaimer: not a "R"eviewer. http://cr.openjdk.java.net/~sla/JDK-8145099/webrev.01/agent/src/os/linux/LinuxDebuggerLocal.c.udiff.html I am not sure why you pass sizeof(err_buf) - 1 instead of sizeof(err_buf) to Pgrab and sizeof(msg) - 1 instead of sizeof(msg) to snprintf? snprintf wi

Re: Potential infinite waiting at JMXConnection#createConnection

2015-12-10 Thread KUBOTA Yuji
Hi Shanliang and all, Sorry my reply is too late. But, finally, I reproduced this issue by following test program! :) Could you please review test program and my patch ? The test program includes some files, but I do not have an account of openjdk, so I push it on icedtea server as below. http:/

RFR: JDK-8145099 Better error message when SA can't attach to a process

2015-12-10 Thread Staffan Larsen
Please review this patch to add a better error message to SA when it fails to attach to a process on linux. Currently the error says "Can't attach to the process”. After this change the message will look something like: "Can't attach to the process: ptrace(PTRACE_ATTACH, ..) failed for 28417: Op

Re: [ding] jmx-dev RFR 8142398: ManagementAgent.status diagnostic command only outputs the specifically set properties

2015-12-10 Thread Staffan Larsen
Looks good! Thanks, /Staffan > On 10 dec. 2015, at 12:07, Jaroslav Bachorik > wrote: > > On 11.11.2015 16:52, Jaroslav Bachorik wrote: >> Please, review the following change > > Pretty please - it's been sitting here unnoticed for almost 30 days despite > regular pings. Shouldn't be that har

[ding] Re: jmx-dev RFR 8142398: ManagementAgent.status diagnostic command only outputs the specifically set properties

2015-12-10 Thread Jaroslav Bachorik
On 11.11.2015 16:52, Jaroslav Bachorik wrote: Please, review the following change Pretty please - it's been sitting here unnoticed for almost 30 days despite regular pings. Shouldn't be that hard ... -JB- Issue : https://bugs.openjdk.java.net/browse/JDK-8142398 Webrev: http://cr.openjdk.j