Re: RFR: JDK-8055845 - Add trace event for promoted objects

2014-09-15 Thread Bengt Rutisson
Hi Staffan, psPromotionManager.inline.hpp I think the PSPromotionManager::copy_to_survivor_space() might send multiple events. If the allocation to the young gen fails we will fall through to do an old gen allocation. But we send the events before we realize that the allocation has failed,

Re: RFR: JDK-8055845 - Add trace event for promoted objects

2014-09-15 Thread Thomas Schatzl
Hi, On Fri, 2014-09-05 at 15:20 -0700, Staffan Friberg wrote: Hi, I have uploaded a new webrev here, cr.openjdk.java.net/~sfriberg/8055845/webrev.03 It contains several changes - Split event into two events (PromoteObjectInNewPLAB, PromoteObjectOutsidePLAB) - Moved events to

Review request: 8042205: javax/management/monitor/*: some test didn't get all the notifications

2014-09-15 Thread shanliang
Hi, Please review the following fix, I changed the way to check received notifications. Bug: https://bugs.openjdk.java.net/browse/JDK-8042205 Webrec: http://cr.openjdk.java.net/~sjiang/JDK-8042205/00/ http://cr.openjdk.java.net/%7Esjiang/JDK-8042205/00/ Thanks, shanliang

Re: Review request: 8042205: javax/management/monitor/*: some test didn't get all the notifications

2014-09-15 Thread Daniel Fuchs
Looks good Shanliang. The synchronization is a bit strange, with the flag being volatile and sometime modified within synchronized blocks and sometime being modified outside of any s-block, but I believe it is working (AFAIU the synchronized is mostly needed because you call notifyAll() and

Re: RFR: JDK-8055845 - Add trace event for promoted objects

2014-09-15 Thread Erik Gahlin
Metadata looks good (trace.xml). /E Staffan Friberg skrev 2014-09-06 00:20: Hi, I have uploaded a new webrev here, cr.openjdk.java.net/~sfriberg/8055845/webrev.03 It contains several changes - Split event into two events (PromoteObjectInNewPLAB, PromoteObjectOutsidePLAB) - Moved

Re: Review request: 8042205: javax/management/monitor/*: some test didn't get all the notifications

2014-09-15 Thread shanliang
Daniel Fuchs wrote: Looks good Shanliang. The synchronization is a bit strange, with the flag being volatile and sometime modified within synchronized blocks and sometime being modified outside of any s-block, but I believe it is working (AFAIU the synchronized is mostly needed because you call

Re: Review request: 8042205: javax/management/monitor/*: some test didn't get all the notifications

2014-09-15 Thread Daniel Fuchs
Looks good! -- daniel On 9/15/14 4:59 PM, shanliang wrote: Daniel Fuchs wrote: Looks good Shanliang. The synchronization is a bit strange, with the flag being volatile and sometime modified within synchronized blocks and sometime being modified outside of any s-block, but I believe it is

Re: RFR: JDK-8055845 - Add trace event for promoted objects

2014-09-15 Thread Thomas Schatzl
Hi, one more allocation that is more serious that this change misses I think: When a PLAB does not fit the current allocation region, G1 releases that one (filling it with a dummy allocation). At the moment this might be up to half a region minus one word per allocated region. We know of

Re: RFR: JDK-8055845 - Add trace event for promoted objects

2014-09-15 Thread Staffan Friberg
Fixed, now checking the allocation result before sending event. I'm checking in all four instances, even if it is only necessary to check the direct allocation. If we get a new fresh PLAB I would expect the allocation to always succeed since we have checked the size before getting the PLAB.

Re: RFR: JDK-8055845 - Add trace event for promoted objects

2014-09-15 Thread Staffan Friberg
Hi Thomas, I feel that this is outside of the scope of this event and change. It is probably a good thing to track to understand if an application wastes more memory than wanted. Perhaps open an RFE for a separate event and documentation? Similarly the TLAB/PLAB can waste memory when we get

Re: RFR: JDK-8055845 - Add trace event for promoted objects

2014-09-15 Thread Staffan Friberg
Hi, Uploaded a new webrev, with the changes from your comments here and from Bengt's email. http://cr.openjdk.java.net/~sfriberg/8055845/webrev.04 See inline for comments. On 09/15/2014 05:57 AM, Thomas Schatzl wrote: Hi, On Fri, 2014-09-05 at 15:20 -0700, Staffan Friberg wrote: Hi, I