Re: RFR: 8023720: setjmp/longjmp changes the process signal mask on OS X

2013-08-29 Thread Rickard Bäckman
Looks good! /R On Aug 29, 2013, at 10:26 AM, Staffan Larsen wrote: Can I have a second review for the Hotspot part of this change? Thanks, /Staffan On 27 aug 2013, at 10:48, Staffan Larsen staffan.lar...@oracle.com wrote: I have also made a fix for hotspot. I messed up the link in

Re: RFR: 8022808: Kitchensink hangs on macos

2013-08-19 Thread Rickard Bäckman
Staffan, the change looks good. (Not a Reviewer). One thing to point out though, if we would leak ports the following line would get stuck in an infinite loop (pthread_mach_thread_np calls _pthread_lookup_thread which calls _pthread_find_thread which will spin forever if the ports part of the

Re: RFR(S): 8020701: Avoid crashes in WatcherThread

2013-07-18 Thread Rickard Bäckman
/~rbackman/8020701.1/ Thanks /R On Jul 17, 2013, at 8:12 PM, Rickard Bäckman wrote: On Jul 17, 2013, at 7:03 PM, Daniel D. Daugherty wrote: On 7/17/13 5:58 AM, Rickard Bäckman wrote: Hi all, can I please have reviews for the following change? We are adding a mechanism for avoiding some

Re: RFR(S): 8020701: Avoid crashes in WatcherThread

2013-07-18 Thread Rickard Bäckman
and we don't have a thread yet. Please change loaded - first loaded src/share/vm/runtime/os.hpp No comments. src/share/vm/runtime/thread.cpp No comments. src/share/vm/runtime/thread.hpp No comments. Dan On 7/18/13 4:37 AM, Rickard Bäckman wrote: Hi, here

Re: RFR(XS): 8020547 : Event based tracing needs a UNICODE string type (hsx24)

2013-07-17 Thread Rickard Bäckman
Looks good to me. /R On Jul 17, 2013, at 12:25 PM, Markus Grönlund wrote: Hi again, Trying again, could I please have a couple of reviews for this? Many thanks Markus From: Markus Grönlund Sent: den 16 juli 2013 17:48 To: serviceability-dev@openjdk.java.net;

RFR(S): 8020107: Avoid crashes in WatcherThread

2013-07-17 Thread Rickard Bäckman
Hi all, can I please have reviews for the following change? We are adding a mechanism for avoiding some crashes in the WatcherThread using different OS-specific methods. Webrev: http://cr.openjdk.java.net/~rbackman/8020701/webrev/ Thanks /R

Re: RFR(S): 8020107: Avoid crashes in WatcherThread

2013-07-17 Thread Rickard Bäckman
Sorry everyone, wrong bugnr in the topic. It should be 8020701. /R On Jul 17, 2013, at 1:58 PM, Rickard Bäckman wrote: Hi all, can I please have reviews for the following change? We are adding a mechanism for avoiding some crashes in the WatcherThread using different OS-specific

Re: RFR(S): 8020107: Avoid crashes in WatcherThread

2013-07-17 Thread Rickard Bäckman
Thank you David. /R On Jul 17, 2013, at 2:56 PM, David Simms wrote: Looks good to me. On 17/07/2013 1:58 p.m., Rickard Bäckman wrote: Hi all, can I please have reviews for the following change? We are adding a mechanism for avoiding some crashes in the WatcherThread using

Re: RFR(S): 8020107: Avoid crashes in WatcherThread

2013-07-17 Thread Rickard Bäckman
Thank you for the review Karen! /R On Jul 17, 2013, at 5:00 PM, Karen Kinnear wrote: Code looks good. Thank you for the assertions and comments. Cleanly done, as usual for your changes Rickard. thanks, Karen On Jul 17, 2013, at 8:22 AM, Rickard Bäckman wrote: Sorry everyone, wrong

Re: RFR(S): 8020701: Avoid crashes in WatcherThread

2013-07-17 Thread Rickard Bäckman
On Jul 17, 2013, at 7:03 PM, Daniel D. Daugherty wrote: On 7/17/13 5:58 AM, Rickard Bäckman wrote: Hi all, can I please have reviews for the following change? We are adding a mechanism for avoiding some crashes in the WatcherThread using different OS-specific methods. Webrev: http

Re: RFR(S): 8016131: nsk/sysdict/vm/stress/chain tests crash the VM in 'entry_frame_is_first()'

2013-07-16 Thread Rickard Bäckman
Thank you! /R On Jul 16, 2013, at 8:24 AM, John Rose wrote: Good. — John On Jul 15, 2013, at 10:11 PM, Rickard Bäckman rickard.back...@oracle.com wrote: thanks for the suggestion. It makes sense. Updated webrev: http://cr.openjdk.java.net/~rbackman/8016131.u2/

Re: RFR(S): 8016131: nsk/sysdict/vm/stress/chain tests crash the VM in 'entry_frame_is_first()'

2013-07-16 Thread Rickard Bäckman
stack_base() method which has the assert. Thanks, Vladimir On 7/15/13 10:11 PM, Rickard Bäckman wrote: Vladimir John, thanks for the suggestion. It makes sense. Updated webrev: http://cr.openjdk.java.net/~rbackman/8016131.u2/ Thanks /R On Jul 16, 2013, at 4:26 AM, John Rose wrote

Re: RFR(S): 8016131: nsk/sysdict/vm/stress/chain tests crash the VM in 'entry_frame_is_first()'

2013-07-15 Thread Rickard Bäckman
. function entry_frame_is_first(_) belongs somewhere else. Since it doesn't really use frame::this, it is confusing as an overload beside a function that does use frame::this. Suggest placing it in JavaCallWrapper not frame. — John On Jul 10, 2013, at 4:59 AM, Rickard Bäckman

Re: RFR(S): 8016131: nsk/sysdict/vm/stress/chain tests crash the VM in 'entry_frame_is_first()'

2013-07-15 Thread Rickard Bäckman
Vladimir John, thanks for the suggestion. It makes sense. Updated webrev: http://cr.openjdk.java.net/~rbackman/8016131.u2/ Thanks /R On Jul 16, 2013, at 4:26 AM, John Rose wrote: On Jul 15, 2013, at 9:59 AM, Vladimir Kozlov vladimir.koz...@oracle.com wrote: There are several methods

Re: RFR(S): 8016131: nsk/sysdict/vm/stress/chain tests crash the VM in 'entry_frame_is_first()'

2013-07-10 Thread Rickard Bäckman
Still looking for a Reviewer. Thanks /R On Jul 8, 2013, at 5:49 PM, Rickard Bäckman wrote: Thanks Markus! /R On Jul 8, 2013, at 5:45 PM, Markus Grönlund wrote: Rickard, I think it looks good (not a Reviewer). Cheers Markus -Original Message- From: Rickard Bäckman

Re: RFR(S): 8016131: nsk/sysdict/vm/stress/chain tests crash the VM in 'entry_frame_is_first()'

2013-07-08 Thread Rickard Bäckman
Trying again, can I please get reviews on this change? Thanks /R On Jul 4, 2013, at 11:30 AM, Rickard Bäckman wrote: Hi, can I please have a couple of reviews for this change? The problem in this crash was that we were given an incorrect fp (in this case 0x0) and had a pc that matched

Re: RFR(S): 8016131: nsk/sysdict/vm/stress/chain tests crash the VM in 'entry_frame_is_first()'

2013-07-08 Thread Rickard Bäckman
Thanks Markus! /R On Jul 8, 2013, at 5:45 PM, Markus Grönlund wrote: Rickard, I think it looks good (not a Reviewer). Cheers Markus -Original Message- From: Rickard Bäckman Sent: den 4 juli 2013 11:30 To: serviceability-dev@openjdk.java.net serviceability-dev

RFR(S): 8016131: nsk/sysdict/vm/stress/chain tests crash the VM in 'entry_frame_is_first()'

2013-07-04 Thread Rickard Bäckman
Hi, can I please have a couple of reviews for this change? The problem in this crash was that we were given an incorrect fp (in this case 0x0) and had a pc that matched the C - Java entry frame. The code then dereferenced fp +- offset. This change verifies that the fp +- offset is actually on

RFR(XS): 8016444: Duplicate zombie check in safe_for_sender

2013-06-12 Thread Rickard Bäckman
Hi, can I please have a couple of reviews for this small change. Basically we managed to get a merge error in the code and have two identical checks after each other. I also got a heads up saying that we missed to update the copyright year in referenceProcessorStats.hpp so I fixed that in

RFR(XS): 4965252: JvmtiExport::post_raw_field_modification jni ref handling is odd

2013-05-15 Thread Rickard Bäckman
Hi all, can I please have this change reviewed? My interpretation is that this isn't really a bug, since the parameter sig_type is never set to [. The suggested change is to remove the check for [ in the if and add an assert. I also created a boolean to track handle creation to simplify

Re: RFR(XS): 4965252: JvmtiExport::post_raw_field_modification jni ref handling is odd

2013-05-15 Thread Rickard Bäckman
, Serguei On 5/15/13 2:37 AM, Rickard Bäckman wrote: Hi all, can I please have this change reviewed? My interpretation is that this isn't really a bug, since the parameter sig_type is never set to [. The suggested change is to remove the check for [ in the if and add an assert. I also

Re: [PATCH] EnableTracing: output from multiple threads may be mixed together

2013-05-13 Thread Rickard Bäckman
- From: Rickard Bäckman [mailto:rickard.back...@oracle.com] Sent: Thursday, May 09, 2013 4:53 PM To: 云达(Yunda) Cc: David Holmes; serviceability-dev@openjdk.java.net; hotspot-runtime-...@openjdk.java.net Subject: Re: [PATCH] EnableTracing: output from multiple threads may be mixed together

Re: [PATCH] EnableTracing: output from multiple threads may be mixed together

2013-05-09 Thread Rickard Bäckman
; + writeEventContent(); +} else { + writeEventContent(); +} + } + + void writeEventContent() { TraceStream ts(*tty); ts.print(xsl:value-of select=@label/: [); xsl:apply-templates select=value|structvalue mode=write-data/ Regards, Yunda -Original Message- From: Rickard

Re: [PATCH] EnableTracing: output from multiple threads may be mixed together

2013-05-08 Thread Rickard Bäckman
One option could be to move the content of writeEvent() to its own method (maybe writeEventContent()) and just do if (UseLockedTracing) { ttyLocker lock; writeEventContent(); } else { writeEventContent(); } I think it makes it easier to read. /R On May 8, 2013, at 2:41 AM, David Holmes

RFR(XS): 8008255: jvmtiExport.cpp::post_to_env() does not check malloc() return

2013-05-08 Thread Rickard Bäckman
Hi, please review this small change. The change adds a check to see the result of a malloc() and aborts the VM with an out of memory error. Webrev: http://cr.openjdk.java.net/~rbackman/8008255/ Bug: http://bugs.sun.com/view_bug.do?bug_id=8008255 Thanks /R

Re: RFR(XS): 8008255: jvmtiExport.cpp::post_to_env() does not check malloc() return

2013-05-08 Thread Rickard Bäckman
Thank you, Staffan /R On May 8, 2013, at 2:17 PM, Staffan Larsen wrote: Looks good. /Staffan On 8 maj 2013, at 13:08, Rickard Bäckman rickard.back...@oracle.com wrote: Hi, please review this small change. The change adds a check to see the result of a malloc() and aborts the VM

Re: RFR(XS): 8008255: jvmtiExport.cpp::post_to_env() does not check malloc() return

2013-05-08 Thread Rickard Bäckman
create a unit test that triggers the failure ? Thx Ron -Original Message- From: Staffan Larsen Sent: Wednesday, May 08, 2013 6:17 AM To: Rickard Bäckman Cc: serviceability-dev@openjdk.java.net serviceability-dev@openjdk.java.net; hotspot-runtime- d...@openjdk.java.net Subject: Re

Re: RFR(XS): 8008255: jvmtiExport.cpp::post_to_env() does not check malloc() return

2013-05-08 Thread Rickard Bäckman
Thank you David /R On May 8, 2013, at 2:44 PM, David Holmes wrote: Looks fine to me. As we discussed in the bug report there's no reasonable way to report the error so an abort is the only way out. David On 8/05/2013 9:08 PM, Rickard Bäckman wrote: Hi, please review this small

Re: RFR(S): 8005038 remove crufty '_g' support from SA

2013-05-07 Thread Rickard Bäckman
Staffan, this looks good to me (not a Reviewer). /R On May 7, 2013, at 10:02 AM, Staffan Larsen wrote: Please review this little fix to remove the _g support from the SA files. bug: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8005038 webrev:

Re: [PATCH] EnableTracing: output from multiple threads may be mixed together

2013-05-01 Thread Rickard Bäckman
Yunda, the change looks good (not a Reviewer). /R On May 2, 2013, at 5:07 AM, 云达(Yunda) wrote: Could anyone review this for me, please? Regards, Yunda From: 云达(Yunda) Sent: Friday, April 19, 2013 4:26 PM To: hotspot-runtime-...@openjdk.java.net; serviceability-dev@openjdk.java.net

Re: RFR (XS): [findbugs] sun.management.AgentConfigurationError.getParams() may expose internal representation by returning AgentConfigurationError.params

2013-04-30 Thread Rickard Bäckman
Staffan, much better! Ship it. /R On Apr 29, 2013, at 4:09 PM, Staffan Larsen wrote: You are right. I was just being lazy. Update webrev: http://cr.openjdk.java.net/~sla/8003671/webrev.01/ Thanks, /Staffan On 29 apr 2013, at 15:43, Rickard Bäckman rickard.back...@oracle.com wrote

Re: RFR(XS): 8013364 SA-JDI exceptions caused by lack of permissions on OSX should be more verbose about issue cause

2013-04-30 Thread Rickard Bäckman
Staffan, this change looks good. (Not a Reviewer). /R On Apr 29, 2013, at 4:40 PM, Staffan Larsen wrote: Hi, This is an improvement to the error message the user receives when SA attach fails on OS X. The current message is: attach: task_for_pid(13581) failed (5) Error attaching to

Re: RFR (S): 8013466 SA crashes when attaching to a process on OS X

2013-04-30 Thread Rickard Bäckman
Looks good (Not a Reviewer). /R On Apr 29, 2013, at 4:34 PM, Staffan Larsen wrote: Please review this small fix of additional NULL-checks in SA on OS X. webrev: http://cr.openjdk.java.net/~sla/8013466/webrev.00/ Thanks, /Staffan

Re: RFR (XS): [findbugs] sun.management.AgentConfigurationError.getParams() may expose internal representation by returning AgentConfigurationError.params

2013-04-29 Thread Rickard Bäckman
Actually, even better alternatives are Arrays.copyOf or array.clone(); /R On Apr 29, 2013, at 3:15 PM, Rickard Bäckman wrote: Staffan, the change looks good, however I would be happy if we actually used the arraycopy instead :) /R On Apr 29, 2013, at 2:43 PM, Staffan Larsen wrote

Re: RFR: 8013117: Thread-local trace_buffer has wrong type and name

2013-04-26 Thread Rickard Bäckman
Staffan and David, thanks for your reviews. /R On Apr 26, 2013, at 2:57 PM, Staffan Larsen wrote: Looks good. /Staffan On 26 apr 2013, at 07:31, Rickard Bäckman rickard.back...@oracle.com wrote: I've updated the webrev (empty struct option). http://cr.openjdk.java.net/~rbackman

Re: RFR: 8013117: Thread-local trace_buffer has wrong type and name

2013-04-25 Thread Rickard Bäckman
I've updated the webrev (empty struct option). http://cr.openjdk.java.net/~rbackman/8013117.u1/ Thanks /R On Apr 24, 2013, at 2:40 PM, Rickard Bäckman wrote: David, On Apr 24, 2013, at 2:25 PM, David Holmes wrote: On 24/04/2013 9:05 PM, Rickard Bäckman wrote: David, On Apr 24, 2013

RFR: 8013117: Thread-local trace_buffer has wrong type and name

2013-04-24 Thread Rickard Bäckman
Hi all, can I have a couple of reviews for this small change. The short story is that the current way the thread-local _trace_buffer is somewhat inflexible. By changing the type of the getter this structure gets more flexible for different implementations. I also think that the name is misused.

Re: RFR: 8013117: Thread-local trace_buffer has wrong type and name

2013-04-24 Thread Rickard Bäckman
wrote: Does it matter that the pointer gets initialized to NULL before, but not now? There isn't any null checks anywhere that depends on that? Regards, Nils On 04/24/2013 09:51 AM, Rickard Bäckman wrote: Hi all, can I have a couple of reviews for this small change. The short story

Re: RFR: 8009985: [parfait] Uninitialised variable at jdk/src/solaris/native/com/sun/management/UnixOperatingSystem_md.c

2013-04-24 Thread Rickard Bäckman
Peter, the change looks good to me (not a Reviewer). I know it is against the local style of the file, but I would prefer if we declared and initialized the variables at first use, and in that way reduced chances of this kind of mistakes. /R On Apr 24, 2013, at 11:31 AM, Peter Allwin wrote:

Re: RFR: 8013117: Thread-local trace_buffer has wrong type and name

2013-04-24 Thread Rickard Bäckman
David, On Apr 24, 2013, at 12:11 PM, David Holmes wrote: On 24/04/2013 7:09 PM, Rickard Bäckman wrote: Nils, no it doesn't matter. Rather intended. By initializing it to NULL we forced implementors to use a pointer that would have to be initialized at some point. Now it can be a class

Re: RFR: 8013117: Thread-local trace_buffer has wrong type and name

2013-04-24 Thread Rickard Bäckman
David, On Apr 24, 2013, at 2:25 PM, David Holmes wrote: On 24/04/2013 9:05 PM, Rickard Bäckman wrote: David, On Apr 24, 2013, at 12:11 PM, David Holmes wrote: On 24/04/2013 7:09 PM, Rickard Bäckman wrote: Nils, no it doesn't matter. Rather intended. By initializing it to NULL we

Re: RFR: 8011882: Replace spin loops as back off when suspending

2013-04-22 Thread Rickard Bäckman
David, thanks for the review. /R On Apr 22, 2013, at 8:23 AM, David Holmes wrote: Okay - seems ready to ship. :) Thanks, David On 22/04/2013 3:20 PM, Rickard Bäckman wrote: David, you are right, the only users of this mechanism are the old flatprofiler (which from what I could

Re: RFR: 8011882: Replace spin loops as back off when suspending

2013-04-21 Thread Rickard Bäckman
consumed any post that the suspended thread may have issued. Thanks /R On Apr 22, 2013, at 1:53 AM, David Holmes wrote: Hi Rickard, On 20/04/2013 12:19 AM, Rickard Bäckman wrote: David, here is an updated webrev with the changes incorporated. http://cr.openjdk.java.net/~rbackman

Re: Review request: 6729929 I18N - Taking Heap Dump failed if project path contains multibyte characters

2013-04-18 Thread Rickard Bäckman
Peter, it looks good to me. /R On Apr 19, 2013, at 4:19 AM, David Holmes wrote: Hi Peter, On 19/04/2013 12:10 AM, Peter Allwin wrote: Hi all, I'm still looking for reviews for this change... Sounds like a perfectly reasonable change. Reviewed. Thanks, David Thanks! /peter

Re: RFR: 8011882: Replace spin loops as back off when suspending

2013-04-16 Thread Rickard Bäckman
javaTimeMillis() David On 15/04/2013 10:50 PM, David Holmes wrote: On 15/04/2013 10:07 PM, Rickard Bäckman wrote: David, this is what the suggested semaphore.cpp/semaphore.hpp. Is that what you are looking for? sigh I thought so till I saw it - far uglier and complicated than I had hoped

Re: RFR: 8012210: Make TracingTime available when INCLUDE_TRACE = 0

2013-04-16 Thread Rickard Bäckman
David, the reason I introduced a new file was that we need the same changes from our closed code. Better to have them in one place. /R On Apr 16, 2013, at 11:34 AM, David Holmes wrote: Hi Rickard, On 15/04/2013 8:56 PM, Rickard Bäckman wrote: Hi all, can I have a couple of small fixes

Re: RFR: 8012210: Make TracingTime available when INCLUDE_TRACE = 0

2013-04-16 Thread Rickard Bäckman
David, thanks for the review! /R On Apr 16, 2013, at 11:42 AM, David Holmes wrote: On 16/04/2013 7:38 PM, Rickard Bäckman wrote: David, the reason I introduced a new file was that we need the same changes from our closed code. Better to have them in one place. Got it! Hard to join

Re: RFR: 8011882: Replace spin loops as back off when suspending

2013-04-15 Thread Rickard Bäckman
David, any new thoughts? Thanks /R On Apr 12, 2013, at 8:06 AM, Rickard Bäckman wrote: On Apr 12, 2013, at 7:34 AM, David Holmes wrote: On 12/04/2013 3:01 PM, Rickard Bäckman wrote: On Apr 12, 2013, at 1:04 AM, David Holmes wrote: On 11/04/2013 11:02 PM, Rickard Bäckman wrote

RFR: 8012210: Make TracingTime available when INCLUDE_TRACE = 0

2013-04-15 Thread Rickard Bäckman
Hi all, can I have a couple of small fixes for this change? The purpose of the change is to make it easier to write events without having to put them inside a #ifdef INCLUDE_TRACE. The idea is that when INCLUDE_TRACE is false the code should be no-ops. The webrev:

Re: RFR: 8011882: Replace spin loops as back off when suspending

2013-04-15 Thread Rickard Bäckman
David, this is what the suggested semaphore.cpp/semaphore.hpp. Is that what you are looking for? Webrev: http://cr.openjdk.java.net/~rbackman/webrev/ Thanks /R On Apr 15, 2013, at 8:59 AM, David Holmes wrote: On 15/04/2013 4:55 PM, Rickard Bäckman wrote: David, any new thoughts

Re: RFR: 8011882: Replace spin loops as back off when suspending

2013-04-12 Thread Rickard Bäckman
On Apr 12, 2013, at 7:34 AM, David Holmes wrote: On 12/04/2013 3:01 PM, Rickard Bäckman wrote: On Apr 12, 2013, at 1:04 AM, David Holmes wrote: On 11/04/2013 11:02 PM, Rickard Bäckman wrote: On Apr 11, 2013, at 2:39 PM, David Holmes wrote: So what did you mean about pthread_semaphore

RFR: 8011882: Replace spin loops as back off when suspending

2013-04-11 Thread Rickard Bäckman
Hi all, can I please have reviews for this change. In the current implementation do_suspend uses spin loops to wait until a thread has been suspended. I would like to change that to use semaphores to reduce CPU usage and also make it easier to have a deterministic timeout. Since we are

Re: RFR: 8011882: Replace spin loops as back off when suspending

2013-04-11 Thread Rickard Bäckman
David, On Apr 11, 2013, at 2:15 PM, David Holmes wrote: Rickard, On 11/04/2013 9:51 PM, Rickard Bäckman wrote: Hi all, can I please have reviews for this change. In the current implementation do_suspend uses spin loops to wait until a thread has been suspended. I would like to change

Re: RFR: 8011882: Replace spin loops as back off when suspending

2013-04-11 Thread Rickard Bäckman
On Apr 11, 2013, at 2:39 PM, David Holmes wrote: On 11/04/2013 10:30 PM, Rickard Bäckman wrote: David, On Apr 11, 2013, at 2:15 PM, David Holmes wrote: Rickard, On 11/04/2013 9:51 PM, Rickard Bäckman wrote: Hi all, can I please have reviews for this change. In the current

Re: RFR: 8011882: Replace spin loops as back off when suspending

2013-04-11 Thread Rickard Bäckman
On Apr 12, 2013, at 1:04 AM, David Holmes wrote: On 11/04/2013 11:02 PM, Rickard Bäckman wrote: On Apr 11, 2013, at 2:39 PM, David Holmes wrote: So what did you mean about pthread_semaphore (what is that anyway?) ?? Never mind, pthread condition variables. Ah I see. I really

Re: RFR(S): 8008357: [sampling] assert(sender_blob-is_runtime_stub() || sender_blob-is_nmethod()) failed: Impossible call chain

2013-03-26 Thread Rickard Bäckman
Still need a Reviewer… Thanks /R On Mar 25, 2013, at 12:43 PM, Staffan Larsen wrote: Looks good to me - not a Reviewer. /Staffan On 25 mar 2013, at 12:13, Rickard Bäckman rickard.back...@oracle.com wrote: Hi, trying again, can I please have two reviews on this change? Some small

Re: RFR(S): 8008357: [sampling] assert(sender_blob-is_runtime_stub() || sender_blob-is_nmethod()) failed: Impossible call chain

2013-03-25 Thread Rickard Bäckman
Hi, trying again, can I please have two reviews on this change? Some small changes after offline discussion. The webrev is available here: http://cr.openjdk.java.net/~rbackman/8008357.u1/ Thanks /R On Mar 5, 2013, at 8:07 AM, Rickard Bäckman wrote: Anyone have time to look

Re: RFR(S): 8008357: [sampling] assert(sender_blob-is_runtime_stub() || sender_blob-is_nmethod()) failed: Impossible call chain

2013-03-04 Thread Rickard Bäckman
Anyone have time to look at this? Thanks /R On Mar 1, 2013, at 10:27 AM, Rickard Bäckman wrote: Hi all, here comes another update to frame.safe_for_sender. If the PC at a place where the stack doesn't match the _frame_size we sometimes read an invalid return PC. In this case we read

RFR(S): 8008357: [sampling] assert(sender_blob-is_runtime_stub() || sender_blob-is_nmethod()) failed: Impossible call chain

2013-03-01 Thread Rickard Bäckman
Hi all, here comes another update to frame.safe_for_sender. If the PC at a place where the stack doesn't match the _frame_size we sometimes read an invalid return PC. In this case we read one that pointed into the Safepoint blob. We now pretty much guarded for all kind of blobs in

Re: RR(S): 8008807: SA: jstack crash when target has mismatched bitness (Linux)

2013-02-25 Thread Rickard Bäckman
Kevin, looks good to me. (Not a Reviewer). /R On Feb 25, 2013, at 7:21 PM, Kevin Walls wrote: Hi, This is a crash I stumbled upon: jstack -m will crash when it doesn't match the bitness of its target. On Solaris we check and give an exception message, Linux should do similarly.

RFR(XS): 8008340: [sampling] assert(upper-pc_offset() = pc_offset) failed: sanity

2013-02-18 Thread Rickard Bäckman
Hi all, I just got a small patch this time that makes sure that a pc given to a frame is actually in the code part of the codeBlob. The case that assert here was pointing into the data part of the codeBlob. This webrev is only for hs24 so far. Waiting for pending changes in hs25. webrev:

Re: RFR(S): 8008088: SA can hang the VM

2013-02-13 Thread Rickard Bäckman
a null reference access could cause a stop. David On 13/02/2013 10:28 PM, Rickard Bäckman wrote: Hi all, can I please have a couple of reviews of this change. The problem discovered was the on Linux and BSD SA uses the ptrace() method to stop the threads before inspecting the memory

Re: RFR(S): 8008102: SA on OS X does not stop the attached process

2013-02-13 Thread Rickard Bäckman
Staffan, nice work. Looks good to me. (Not a Reviewer). /R 13 feb 2013 kl. 16:05 skrev Staffan Larsen staffan.lar...@oracle.com: Please review the following change. When SA attaches to a JVM on OS X, it does not stop the process. If the JVM keeps running while SA inspects it can lead to

Re: RFR: 8006423 SA: NullPointerException in sun.jvm.hotspot.debugger.bsd.BsdThread.getContext(BsdThread.java:67)

2013-02-08 Thread Rickard Bäckman
Staffan, the change looks good to me! Thanks for fixing this problem. /R On Jan 17, 2013, at 8:48 PM, Staffan Larsen wrote: This is a request for review of a fix for SA on OS X. webrev: http://cr.openjdk.java.net/~sla/8006423/webrev.00/ bug:

Re: Request for review (XS): 8006563: Remove unused ProfileVM_lock

2013-02-01 Thread Rickard Bäckman
Thanks for the review, David. /R On Feb 1, 2013, at 7:59 AM, David Holmes wrote: On 1/02/2013 4:54 PM, Rickard Bäckman wrote: That was the idea. However, can I have Ok for checking this into hs24 while waiting? Sorry - ignore the hs25 comment - been looking at too many JDK review

Re: Request for review (XS): 8006563: Remove unused ProfileVM_lock

2013-01-31 Thread Rickard Bäckman
That was the idea. However, can I have Ok for checking this into hs24 while waiting? Thanks /R On Jan 21, 2013, at 11:33 PM, David Holmes wrote: On 22/01/2013 12:09 AM, Rickard Bäckman wrote: Yes, that code has changed. Checked in to hs24. Okay but this is a review for hs25 ;-) So I assume

Re: 8007142: Add utility classes for writing better multiprocess tests in jtreg

2013-01-30 Thread Rickard Bäckman
Katja, I think the change looks good. However we need to find an official reviewer before we can push this. Can we get an official reviewer to look on this change please? Thanks /R On Jan 30, 2013, at 3:11 PM, Yekaterina Kantserova wrote: Hi everyone, In the previous mail I've referred

Request for review (XS): 8006563: Remove unused ProfileVM_lock

2013-01-18 Thread Rickard Bäckman
Hi all, would some please review the following change? With recent changes in hs24 the ProfileVM_lock is no longer in use. This small change set removes the lock. The changes that lead to this is also on their way into hsx, but are not yet there. If that change would go into hsx without this

Re: Request for review (XS): 8006563: Remove unused ProfileVM_lock

2013-01-18 Thread Rickard Bäckman
://cr.openjdk.java.net/~rbackman/8006563.2/ (or at least currently copying…) /R On Jan 18, 2013, at 2:12 PM, Aleksey Shipilev wrote: On 01/18/2013 04:58 PM, Rickard Bäckman wrote: http://cr.openjdk.java.net/~rbackman/8006563/ Looks good to me (not a Reviewer), modulo: a) Are we sure this thing

Re: [PATCH] JDK-8005791: Remove java.beans.* imports from com.sun.jmx.mbeanserver.Introspector

2013-01-08 Thread Rickard Bäckman
Jaroslav, the change looks good! /R On Jan 7, 2013, at 2:44 PM, Jaroslav Bachorik wrote: Looking for reviewers and a sponsor. This is a simple patch to remove unused java.beans.* imports from com.sun.jmx.mbeanserver.Introspector. The actual usage of java.beans.* classes was removed from

Re: jmx-dev [PATCH] JDK-6809322: Missing notifications from javax.management.timer.Timer

2012-10-15 Thread Rickard Bäckman
Looks good! /R On Oct 15, 2012, at 2:14 PM, Jaroslav Bachorik wrote: On 10/15/2012 01:45 PM, David Holmes wrote: On 15/10/2012 8:08 PM, Jaroslav Bachorik wrote: On 10/15/2012 04:19 AM, David Holmes wrote: I think your changes now go further than needed. The original code uses a dual

Re: RFR S: 7127792 Add the ability to change an existing PeriodicTask's execution interval

2012-10-12 Thread Rickard Bäckman
Thank you Markus. /R On Oct 12, 2012, at 7:29 AM, Markus Grönlund wrote: I am ok with this change Rickard. Thanks also to David Holmes for his great feedback and help on this one. /Markus Sent from my iPhone On 12 okt 2012, at 07:06, Rickard Bäckman rickard.back...@oracle.com wrote

Re: RFR S: 7127792 Add the ability to change an existing PeriodicTask's execution interval

2012-10-11 Thread Rickard Bäckman
People, I need at least one more reviewer, thanks! /R On Oct 9, 2012, at 3:00 PM, Rickard Bäckman wrote: David, thanks for your review! /R On Oct 9, 2012, at 2:01 PM, David Holmes wrote: On 9/10/2012 9:42 PM, Rickard Bäckman wrote: David, see inline. On Oct 9, 2012, at 1:18

Re: RFR: 8000412: Clean SA code on ia64

2012-10-09 Thread Rickard Bäckman
Looks good, ship it. /R On Oct 9, 2012, at 1:27 AM, Yumin Qi wrote: Hi, All Can I have your codereview for 8000412 : Clean SA code on ia64 Summary: Clean up IA64 code in SA since SA does no t support this cpu architecture. Reviewed-by: Contributed-by:

Re: RFR S: 7127792 Add the ability to change an existing PeriodicTask's execution interval

2012-10-09 Thread Rickard Bäckman
Trying again, can I have a couple of reviews, please? /R On Oct 4, 2012, at 3:01 PM, Rickard Bäckman wrote: Hi all, can I please have a couple of reviews on the following change: http://cr.openjdk.java.net/~rbackman/7127792/ http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7127792

Re: RFR S: 7127792 Add the ability to change an existing PeriodicTask's execution interval

2012-10-09 Thread Rickard Bäckman
-notify_all(); There is only one thread that waits so notify() will suffice. Thanks, David On 9/10/2012 4:34 PM, Rickard Bäckman wrote: Trying again, can I have a couple of reviews, please? /R On Oct 4, 2012, at 3:01 PM, Rickard Bäckman wrote: Hi all, can I please have a couple

Re: RFR S: 7127792 Add the ability to change an existing PeriodicTask's execution interval

2012-10-09 Thread Rickard Bäckman
David, see inline. On Oct 9, 2012, at 1:18 PM, David Holmes wrote: On 9/10/2012 7:36 PM, Rickard Bäckman wrote: David, thanks for your reply! I've changed the code according to the suggestions, I've also changed the types in PeriodicTask from being a size_t to being a jint (see updated

Re: RFR S: 7127792 Add the ability to change an existing PeriodicTask's execution interval

2012-10-09 Thread Rickard Bäckman
David, thanks for your review! /R On Oct 9, 2012, at 2:01 PM, David Holmes wrote: On 9/10/2012 9:42 PM, Rickard Bäckman wrote: David, see inline. On Oct 9, 2012, at 1:18 PM, David Holmes wrote: On 9/10/2012 7:36 PM, Rickard Bäckman wrote: David, thanks for your reply! I've

RFR S: 7127792 Add the ability to change an existing PeriodicTask's execution interval

2012-10-04 Thread Rickard Bäckman
Hi all, can I please have a couple of reviews on the following change: http://cr.openjdk.java.net/~rbackman/7127792/ http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7127792 In short the purpose is to enable tasks to change the interval they are executed in. We would also like to be able to

Re: Review request: 7197350: NPG: jvmtiHeapReferenceCallback receives incorrect reference_kind for system class roots

2012-09-11 Thread Rickard Bäckman
Looks good. /R On Sep 10, 2012, at 6:39 PM, Stefan Karlsson wrote: http://cr.openjdk.java.net/~stefank/7197350/webrev/ 7197350: NPG: jvmtiHeapReferenceCallback receives incorrect reference_kind for system class roots Summary: Fix the iteration over the system classes and report the

Re: RFR(S): 6963102: sun/tools/jstatd/jstatdDefaults.sh fails

2012-09-05 Thread Rickard Bäckman
Looks good. /R On Sep 5, 2012, at 1:51 PM, Staffan Larsen wrote: Please review the patch below to make the jstatd tests a little bit more resilient to different outputs of the jps command. The jps command can output a couple of different error messages when information is missing about

Re: RFR: 7093328: JVMTI: jvmtiPrimitiveFieldCallback always report 0's for static primitives

2012-08-30 Thread Rickard Bäckman
fix so IMHO you could get away with a context diff in the suggested fix of the bug report. Sorry, I pasted the wrong url, the public one is available here: http://cr.openjdk.java.net/~rbackman/7093328/ Thanks for the review Dan! /R On 8/29/12 1:24 AM, Rickard Bäckman wrote: Hi all, can I

Re: RFR: 7093328: JVMTI: jvmtiPrimitiveFieldCallback always report 0's for static primitives

2012-08-30 Thread Rickard Bäckman
Thanks Serguei. /R On Aug 29, 2012, at 6:46 PM, serguei.spit...@oracle.com wrote: Hi Richard, I see you've already got enough reviewers. The fix looks good. Nice finding! Thanks, Serguei On 8/29/12 12:24 AM, Rickard Bäckman wrote: Hi all, can I get a couple of reviews

RFR: 7093328: JVMTI: jvmtiPrimitiveFieldCallback always report 0's for static primitives

2012-08-29 Thread Rickard Bäckman
Hi all, can I get a couple of reviews for the following bug fix: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7093328 webrev: http://rbackman.se.oracle.com/~rbackman/7093328/ Thanks /R

Re: RFR: 7093328: JVMTI: jvmtiPrimitiveFieldCallback always report 0's for static primitives

2012-08-29 Thread Rickard Bäckman
. Makes sense to me… /R On Aug 29, 2012, at 10:46 AM, Rickard Bäckman wrote: David, I'll try to find the sources and see if I can find any obvious difference. I've considered adding a regression test, however I couldn't come up with an idea that wouldn't involve native libraries. I've

Re: Review request for: 7191786 retransformClasses() does not pass in LocalVariableTypeTable of a method

2012-08-24 Thread Rickard Bäckman
Serguei, looks good! /R On Aug 22, 2012, at 8:34 PM, serguei.spit...@oracle.com wrote: Dmitry, Thank you for the review! Please, find new webrev version here: http://cr.openjdk.java.net/~sspitsyn/webrevs/2012/7191786-JVMTI-LVTT.1/ I also changed the line: 196 if

Re: RFR(M): 7178703: Fix handling of quoted arguments and better error messages in dcmd

2012-06-26 Thread Rickard Bäckman
Looks good. /R On 06/25/2012 03:49 PM, Staffan Larsen wrote: Here is an updated webrev. The last one didn't compile on Solaris. http://cr.openjdk.java.net/~sla/7178703/webrev.02/ Thanks, /Staffan On 21 jun 2012, at 13:30, Staffan Larsen wrote: Please review the following fix to the

Re: RFR(S): 7178846: IterateThroughHeap: heap_iteration_callback passes a negative size for big array

2012-06-25 Thread Rickard Bäckman
Staffan, the change looks good. /R On 06/25/2012 10:05 AM, Staffan Larsen wrote: Please review the following fix. Bug: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7178846 Webrev: http://cr.openjdk.java.net/~sla/7178846/webrev.01/ Class CallbackWrapper in jvmtiTagMap.cpp has a missing

Re: JEP 158

2012-06-20 Thread Rickard Bäckman
On 06/20/2012 02:07 PM, Kirk Pepperdine wrote: Hi Staffan, Thanks for the feedback! It is going to be hard to find the right balance between a system that provides great power and one that is simple to use and to implement. In general, I will lean more towards making it simple, but I want

Re: RFR: 7143353: hprof not working with invokedynamic

2012-05-24 Thread Rickard Bäckman
Looks good. /R On 05/24/2012 09:46 AM, Nils Loodin wrote: when the 'demo' java_crw_demo (which is used by hprof) tries to mirror the constant pool of a class, a few of the constants introduced by invokedynamic ins't recognized. Fairly simple fix. For my simple repro, it seems to work with

RFR: CR7171422: Change 7161732 breaks SA on Windows

2012-05-24 Thread Rickard Bäckman
Hi, my refactoring broke the SA on Windows, the only platform that didn't have a declaration of the thread_id type in vmStructs. This change adds the declares the field as an unsigned integer. Tested by building in JPRT and connecting with jsadebugd.exe on Win64. Bug:

Re: RFR: CR7171422: Change 7161732 breaks SA on Windows

2012-05-24 Thread Rickard Bäckman
Thank you for the quick review David. /R On 05/24/2012 02:28 PM, David Holmes wrote: OK. David On 24/05/2012 10:16 PM, Rickard Bäckman wrote: Hi, my refactoring broke the SA on Windows, the only platform that didn't have a declaration of the thread_id type in vmStructs. This change adds

Re: RFR: 7161732: Improve handling of thread_id in OSThread

2012-05-14 Thread Rickard Bäckman
Hi, this has been out on review for a while, without any OK or objections. So I'll ask again, can I have a review or two on this small change? Thanks /R On 05/03/2012 11:39 AM, Rickard Bäckman wrote: Hi all, I've made a refactoring of thread_id in OSThread, which reduces the amount

Re: RFR: 7161732: Improve handling of thread_id in OSThread

2012-05-14 Thread Rickard Bäckman
Thank you, David! /R On 05/14/2012 01:29 PM, David Holmes wrote: On 14/05/2012 6:55 PM, Rickard Bäckman wrote: this has been out on review for a while, without any OK or objections. So I'll ask again, can I have a review or two on this small change? Sorry Rickard I got distracted from my

Re: RFR: 7161732: Improve handling of thread_id in OSThread

2012-05-07 Thread Rickard Bäckman
On 05/07/2012 10:55 AM, David Holmes wrote: On 7/05/2012 5:21 PM, Rickard Bäckman wrote: David, I can't find that code in any of the modified files (only as blue text in osThread_bsd.hpp, but that is removed code). Sorry the frames view of the webrev confused me. It is blue

RFR: 7161732: Improve handling of thread_id in OSThread

2012-05-03 Thread Rickard Bäckman
Hi all, I've made a refactoring of thread_id in OSThread, which reduces the amount of duplicated code. Please review this change at: http://cr.openjdk.java.net/~rbackman/7161732/webrev/ Thanks /R

RFR: 7161732 - Improve handling of thread_id in OSThread

2012-04-18 Thread Rickard Bäckman
Hi all, can I have some reviews for my attempt at making OSThread a bit more generic, reducing the code duplication. Webrev: http://cr.openjdk.java.net/~rbackman/7161732/webrev/ Thanks /R

Re: RFR: 7161732 - Improve handling of thread_id in OSThread

2012-04-18 Thread Rickard Bäckman
Hi, after some internal discussions it is clear that there are a couple of problems with this change. Please ignore it for now. Thanks /R On Apr 18, 2012, at 8:16 AM, Rickard Bäckman wrote: Hi all, can I have some reviews for my attempt at making OSThread a bit more generic, reducing

Re: RFR: 7160570: Intrinsification support for tracing framework

2012-04-16 Thread Rickard Bäckman
{ return _thread_id; } Otherwise it looks good. Thanks for catching that one, I'll change it and submit. /R tom On Apr 13, 2012, at 6:49 AM, Rickard Bäckman wrote: Thank you Staffan, still need one Reviewer, any takers? Thanks /R On 04/13/2012 11:21 AM, Staffan Larsen wrote: Looks good

Re: RFR: 7160570: Intrinsification support for tracing framework

2012-04-13 Thread Rickard Bäckman
Thank you Staffan, still need one Reviewer, any takers? Thanks /R On 04/13/2012 11:21 AM, Staffan Larsen wrote: Looks good. /Staffan On 11 apr 2012, at 14:55, Rickard Bäckman wrote: Hi all, can I get reviews for the following change where we add support for intrinsics for a couple

  1   2   >