Re: RFR: 8156500: deadlock provoked by new stress test com/sun/jdi/OomDebugTest.java

2016-08-24 Thread Kim Barrett
> On Aug 24, 2016, at 9:26 PM, David Holmes wrote: > > Hi Kim, > > Sorry this fell off the radar … That’s okay. There was a long gap while I was working on other things. > On 24/08/2016 7:09 AM, Kim Barrett wrote: >> Mandy asked for some additional comments to make

Re: RFR: 8156500: deadlock provoked by new stress test com/sun/jdi/OomDebugTest.java

2016-08-24 Thread David Holmes
Hi Kim, Sorry this fell off the radar ... On 24/08/2016 7:09 AM, Kim Barrett wrote: Mandy asked for some additional comments to make life easier for future readers. I've also stopped pretending the description for waitForReferenceProcessing is javadoc, since this function is private and only

Re: RFR: 8156500: deadlock provoked by new stress test com/sun/jdi/OomDebugTest.java

2016-08-24 Thread Kim Barrett
> On Aug 24, 2016, at 3:50 AM, Per Liden wrote: > > Still looks good. > > cheers, > Per Thanks. > > On 2016-08-23 23:09, Kim Barrett wrote: >> Mandy asked for some additional comments to make life easier for >> future readers. I've also stopped pretending the

Re: RFR: 8156500: deadlock provoked by new stress test com/sun/jdi/OomDebugTest.java

2016-08-24 Thread Per Liden
Still looks good. cheers, Per On 2016-08-23 23:09, Kim Barrett wrote: Mandy asked for some additional comments to make life easier for future readers. I've also stopped pretending the description for waitForReferenceProcessing is javadoc, since this function is private and only accessible via

Re: RFR: 8156500: deadlock provoked by new stress test com/sun/jdi/OomDebugTest.java

2016-08-23 Thread Kim Barrett
> On Aug 23, 2016, at 5:31 PM, Mandy Chung wrote: > > >> On Aug 23, 2016, at 2:09 PM, Kim Barrett wrote: >> >> New webrevs: >> full: http://cr.openjdk.java.net/~kbarrett/8156500/jdk.05/ >> incr:

Re: RFR: 8156500: deadlock provoked by new stress test com/sun/jdi/OomDebugTest.java

2016-08-23 Thread Mandy Chung
> On Aug 23, 2016, at 2:09 PM, Kim Barrett wrote: > > New webrevs: > full: http://cr.openjdk.java.net/~kbarrett/8156500/jdk.05/ > incr: http://cr.openjdk.java.net/~kbarrett/8156500/jdk.05.inc/ > > Unchanged hotspot webrevs: > full:

Re: RFR: 8156500: deadlock provoked by new stress test com/sun/jdi/OomDebugTest.java

2016-08-23 Thread Kim Barrett
Mandy asked for some additional comments to make life easier for future readers. I've also stopped pretending the description for waitForReferenceProcessing is javadoc, since this function is private and only accessible via SharedSecrets or reflection that avoids access control. Changes only in

Re: RFR: 8156500: deadlock provoked by new stress test com/sun/jdi/OomDebugTest.java

2016-08-19 Thread Kim Barrett
> On Aug 19, 2016, at 7:54 PM, Mandy Chung wrote: > > >> On Aug 19, 2016, at 4:09 PM, Kim Barrett wrote: >> >>> On Aug 13, 2016, at 4:07 PM, Mandy Chung wrote: >>> >>> On Aug 13, 2016, at 1:20 AM, Peter Levart

Re: RFR: 8156500: deadlock provoked by new stress test com/sun/jdi/OomDebugTest.java

2016-08-19 Thread Mandy Chung
> On Aug 19, 2016, at 4:09 PM, Kim Barrett wrote: > >> On Aug 13, 2016, at 4:07 PM, Mandy Chung wrote: >> >> >>> On Aug 13, 2016, at 1:20 AM, Peter Levart wrote: >>> >>> Hi Mandy, >>> >>> On 08/13/2016 01:55 AM, Mandy

Re: RFR: 8156500: deadlock provoked by new stress test com/sun/jdi/OomDebugTest.java

2016-08-19 Thread Kim Barrett
> On Aug 13, 2016, at 4:07 PM, Mandy Chung wrote: > > >> On Aug 13, 2016, at 1:20 AM, Peter Levart wrote: >> >> Hi Mandy, >> >> On 08/13/2016 01:55 AM, Mandy Chung wrote: On Aug 8, 2016, at 6:25 PM, Kim Barrett

Re: RFR: 8156500: deadlock provoked by new stress test com/sun/jdi/OomDebugTest.java

2016-08-17 Thread Peter Levart
Hi Kim, On 08/15/2016 05:15 AM, Kim Barrett wrote: >I have a feeling that these pauses are now unnecessary. Will try to check with some experiments… I found that the DirectBufferAllocTest will sometimes fail if the pauses are taken out. I think what’s going on is that the multiple threads

Re: RFR: 8156500: deadlock provoked by new stress test com/sun/jdi/OomDebugTest.java

2016-08-14 Thread Kim Barrett
> On Aug 13, 2016, at 4:20 AM, Peter Levart wrote: > > Hi Mandy, > > On 08/13/2016 01:55 AM, Mandy Chung wrote: >>> On Aug 8, 2016, at 6:25 PM, Kim Barrett >>> wrote: >>> >>> full: >>> http://cr.openjdk.java.net/~kbarrett/8156500/jdk.04/ >>>

Re: RFR: 8156500: deadlock provoked by new stress test com/sun/jdi/OomDebugTest.java

2016-08-13 Thread Mandy Chung
> On Aug 13, 2016, at 1:20 AM, Peter Levart wrote: > > Hi Mandy, > > On 08/13/2016 01:55 AM, Mandy Chung wrote: >>> On Aug 8, 2016, at 6:25 PM, Kim Barrett >>> wrote: >>> >>> full: >>> http://cr.openjdk.java.net/~kbarrett/8156500/jdk.04/ >>>

Re: RFR: 8156500: deadlock provoked by new stress test com/sun/jdi/OomDebugTest.java

2016-08-13 Thread Peter Levart
Hi Mandy, On 08/13/2016 01:55 AM, Mandy Chung wrote: On Aug 8, 2016, at 6:25 PM, Kim Barrett wrote: full: http://cr.openjdk.java.net/~kbarrett/8156500/jdk.04/ http://cr.openjdk.java.net/~kbarrett/8156500/hotspot.04/ This looks very good. Have you considered

Re: RFR: 8156500: deadlock provoked by new stress test com/sun/jdi/OomDebugTest.java

2016-08-12 Thread Mandy Chung
> On Aug 8, 2016, at 6:25 PM, Kim Barrett wrote: > > full: http://cr.openjdk.java.net/~kbarrett/8156500/jdk.04/ > http://cr.openjdk.java.net/~kbarrett/8156500/hotspot.04/ This looks very good. Have you considered having JVM_WaitForReferencePendingList method to

Re: RFR: 8156500: deadlock provoked by new stress test com/sun/jdi/OomDebugTest.java

2016-08-09 Thread Kim Barrett
> On Aug 9, 2016, at 3:52 AM, Per Liden wrote: > > Hi Kim, > > On 2016-08-09 03:25, Kim Barrett wrote: >>> On Aug 8, 2016, at 8:16 AM, Per Liden wrote: >>> I have one suggestion though, regarding CheckReferencePendingList(). While >>> reviewing I

Re: RFR: 8156500: deadlock provoked by new stress test com/sun/jdi/OomDebugTest.java

2016-08-09 Thread Per Liden
Hi Kim, On 2016-08-09 03:25, Kim Barrett wrote: On Aug 8, 2016, at 8:16 AM, Per Liden wrote: Hi Kim, On 2016-08-01 20:47, Kim Barrett wrote: This updated webrev addresses concerns about the performance of the VM API used by the reference processor thread in the

Re: RFR: 8156500: deadlock provoked by new stress test com/sun/jdi/OomDebugTest.java

2016-08-08 Thread Kim Barrett
> On Aug 8, 2016, at 8:16 AM, Per Liden wrote: > > Hi Kim, > > On 2016-08-01 20:47, Kim Barrett wrote: >> This updated webrev addresses concerns about the performance of the VM >> API used by the reference processor thread in the original webrev. >> >> New webrevs: >>

Re: RFR: 8156500: deadlock provoked by new stress test com/sun/jdi/OomDebugTest.java

2016-08-08 Thread Per Liden
Hi Kim, On 2016-08-01 20:47, Kim Barrett wrote: This updated webrev addresses concerns about the performance of the VM API used by the reference processor thread in the original webrev. New webrevs: full: http://cr.openjdk.java.net/~kbarrett/8156500/jdk.03/

Re: RFR: 8156500: deadlock provoked by new stress test com/sun/jdi/OomDebugTest.java

2016-08-03 Thread Kim Barrett
> On Aug 3, 2016, at 3:41 AM, David Holmes wrote: > > On 2/08/2016 4:46 AM, Kim Barrett wrote: >> This updated webrev addresses the concerns around initialization order >> for Throwable and OutOfMemoryError. It doesn't address the VM API >> used by the reference

Re: RFR: 8156500: deadlock provoked by new stress test com/sun/jdi/OomDebugTest.java

2016-08-03 Thread David Holmes
On 2/08/2016 4:46 AM, Kim Barrett wrote: This updated webrev addresses the concerns around initialization order for Throwable and OutOfMemoryError. It doesn't address the VM API used by the reference processor thread; that will be in a later webrev. New webrevs: full:

Re: RFR: 8156500: deadlock provoked by new stress test com/sun/jdi/OomDebugTest.java

2016-08-02 Thread Kim Barrett
> On Aug 1, 2016, at 2:47 PM, Kim Barrett wrote: > > This updated webrev addresses concerns about the performance of the VM > API used by the reference processor thread in the original webrev. > > New webrevs: > full: http://cr.openjdk.java.net/~kbarrett/8156500/jdk.03/

Re: RFR: 8156500: deadlock provoked by new stress test com/sun/jdi/OomDebugTest.java

2016-08-02 Thread Kim Barrett
> On Aug 2, 2016, at 8:55 AM, Peter Levart wrote: > > Hi Kim, > > This looks very good. I like the way you dealt with race between the > ReferenceHandler thread and threads waiting for it to do some cleanup > progress. I think the VM API is suitable for possible

Re: RFR: 8156500: deadlock provoked by new stress test com/sun/jdi/OomDebugTest.java

2016-08-02 Thread Peter Levart
Hi Kim, This looks very good. I like the way you dealt with race between the ReferenceHandler thread and threads waiting for it to do some cleanup progress. I think the VM API is suitable for possible further development on JDK-8149925. It's also nice that the whole pending list is obtained

Re: RFR: 8156500: deadlock provoked by new stress test com/sun/jdi/OomDebugTest.java

2016-08-01 Thread Kim Barrett
This updated webrev addresses concerns about the performance of the VM API used by the reference processor thread in the original webrev. New webrevs: full: http://cr.openjdk.java.net/~kbarrett/8156500/jdk.03/ http://cr.openjdk.java.net/~kbarrett/8156500/hotspot.03/ incr:

Re: RFR: 8156500: deadlock provoked by new stress test com/sun/jdi/OomDebugTest.java

2016-08-01 Thread Kim Barrett
This updated webrev addresses the concerns around initialization order for Throwable and OutOfMemoryError. It doesn't address the VM API used by the reference processor thread; that will be in a later webrev. New webrevs: full: http://cr.openjdk.java.net/~kbarrett/8156500/hotspot.02/ incr:

Re: RFR: 8156500: deadlock provoked by new stress test com/sun/jdi/OomDebugTest.java

2016-06-29 Thread Mandy Chung
> On Jun 29, 2016, at 6:42 PM, David Holmes wrote: > > The earlier you initialize Throwable the earlier you can try to create an > exception that has a backtrace. But there will always be a region of code > where we can't throw an OOME with a backtrace because of the

Re: RFR: 8156500: deadlock provoked by new stress test com/sun/jdi/OomDebugTest.java

2016-06-29 Thread Mandy Chung
> On Jun 28, 2016, at 10:19 PM, David Holmes wrote: > > So here is what I see has happened. > > Looking back at 9-b01, before we forced the initialization of > InterruptedException and thus Throwable we find: > > 58 Initializing 'java/lang/Throwable'

Re: RFR: 8156500: deadlock provoked by new stress test com/sun/jdi/OomDebugTest.java

2016-06-29 Thread David Holmes
On 30/06/2016 5:12 AM, Kim Barrett wrote: On Jun 29, 2016, at 2:54 PM, Kim Barrett wrote: I think the problem may have existed long before modules, but simply wasn't noticed. That is, I suspect trying the test case mentioned below with a pre-June-2014 fastdebug build

Re: RFR: 8156500: deadlock provoked by new stress test com/sun/jdi/OomDebugTest.java

2016-06-29 Thread Kim Barrett
> On Jun 29, 2016, at 2:54 PM, Kim Barrett wrote: > > I think the problem may have existed long before modules, but simply > wasn't noticed. That is, I suspect trying the test case mentioned > below with a pre-June-2014 fastdebug build (when Reference started >

Re: RFR: 8156500: deadlock provoked by new stress test com/sun/jdi/OomDebugTest.java

2016-06-29 Thread Kim Barrett
> On Jun 28, 2016, at 7:23 PM, David Holmes wrote: > > On 29/06/2016 8:43 AM, Kim Barrett wrote: >> Updated webrevs: >> >> Full: >> http://cr.openjdk.java.net/~kbarrett/8156500/jdk.01/ >> http://cr.openjdk.java.net/~kbarrett/8156500/hotspot.01/ >> >> Incremental: >>

Re: RFR: 8156500: deadlock provoked by new stress test com/sun/jdi/OomDebugTest.java

2016-06-29 Thread Mandy Chung
> On Jun 28, 2016, at 3:43 PM, Kim Barrett wrote: > > Updated webrevs: > > Full: > http://cr.openjdk.java.net/~kbarrett/8156500/jdk.01/ > http://cr.openjdk.java.net/~kbarrett/8156500/hotspot.01/ > The VM change does simplify the implementation a lot. Does/should the

Re: RFR: 8156500: deadlock provoked by new stress test com/sun/jdi/OomDebugTest.java

2016-06-29 Thread Peter Levart
Hi again, On 06/29/2016 03:30 PM, Peter Levart wrote: Just to show what I mean, here's a simple optimization that doesn't use a private pendingList of references shared among callers of tryHandlePanding(true/false), but instead uses course-grained synchronization and waiting for

Re: RFR: 8156500: deadlock provoked by new stress test com/sun/jdi/OomDebugTest.java

2016-06-29 Thread Peter Levart
Hi Kim, On 06/29/2016 01:22 PM, Peter Levart wrote: Transfering the whole list in one JNI invocation has the potential for further optimizations on the Java side (like handling the whole popped list privately without additional synchronization - if we ever find a way for java.nio.Bits to

Re: RFR: 8156500: deadlock provoked by new stress test com/sun/jdi/OomDebugTest.java

2016-06-29 Thread Peter Levart
Hi Kim, Let me chime-in although it's a bit late... I think this is a good change to finally get rid of OOME problems in this area. On 06/28/2016 07:45 PM, Kim Barrett wrote: On Jun 28, 2016, at 5:33 AM, Per Liden wrote: Patch looks good. The only thing I don't feel

Re: RFR: 8156500: deadlock provoked by new stress test com/sun/jdi/OomDebugTest.java

2016-06-28 Thread David Holmes
Hi Mandy, On 29/06/2016 12:28 PM, Mandy Chung wrote: On Jun 28, 2016, at 4:23 PM, David Holmes wrote: On 29/06/2016 8:43 AM, Kim Barrett wrote: Updated webrevs: Full: http://cr.openjdk.java.net/~kbarrett/8156500/jdk.01/

Re: RFR: 8156500: deadlock provoked by new stress test com/sun/jdi/OomDebugTest.java

2016-06-28 Thread Mandy Chung
> On Jun 28, 2016, at 4:23 PM, David Holmes wrote: > > On 29/06/2016 8:43 AM, Kim Barrett wrote: >> Updated webrevs: >> >> Full: >> http://cr.openjdk.java.net/~kbarrett/8156500/jdk.01/ >> http://cr.openjdk.java.net/~kbarrett/8156500/hotspot.01/ >> >> Incremental: >>

Re: RFR: 8156500: deadlock provoked by new stress test com/sun/jdi/OomDebugTest.java

2016-06-28 Thread David Holmes
On 29/06/2016 8:43 AM, Kim Barrett wrote: Updated webrevs: Full: http://cr.openjdk.java.net/~kbarrett/8156500/jdk.01/ http://cr.openjdk.java.net/~kbarrett/8156500/hotspot.01/ Incremental: http://cr.openjdk.java.net/~kbarrett/8156500/jdk.01.inc/ Did Reference not work? Just curious. I used to

Re: RFR: 8156500: deadlock provoked by new stress test com/sun/jdi/OomDebugTest.java

2016-06-28 Thread Kim Barrett
> On Jun 27, 2016, at 9:34 PM, Daniel D. Daugherty > wrote: > Other than the possible missing notify_all() in vmGCOperations.cpp, > this all looks great. Thanks Dan.

Re: RFR: 8156500: deadlock provoked by new stress test com/sun/jdi/OomDebugTest.java

2016-06-28 Thread Kim Barrett
Updated webrevs: Full: http://cr.openjdk.java.net/~kbarrett/8156500/jdk.01/ http://cr.openjdk.java.net/~kbarrett/8156500/hotspot.01/ Incremental: http://cr.openjdk.java.net/~kbarrett/8156500/jdk.01.inc/ http://cr.openjdk.java.net/~kbarrett/8156500/hotspot.01.inc/ Still investigating the

Re: RFR: 8156500: deadlock provoked by new stress test com/sun/jdi/OomDebugTest.java

2016-06-28 Thread Kim Barrett
> On Jun 28, 2016, at 12:04 AM, David Holmes wrote: > > Hi Kim, > > I like all the removal of the pending-list-locker related code, but can't > really comment on the details of how it was replaced and interacts with all > the GC code. The interface to the Reference

Re: RFR: 8156500: deadlock provoked by new stress test com/sun/jdi/OomDebugTest.java

2016-06-28 Thread Kim Barrett
> On Jun 28, 2016, at 5:33 AM, Per Liden wrote: > Patch looks good. The only thing I don't feel qualified to review is the > initialization order change in thread.cpp, so I'll let others comments on > that. Thanks. I’ll be following up on that area. > I like the

Re: RFR: 8156500: deadlock provoked by new stress test com/sun/jdi/OomDebugTest.java

2016-06-28 Thread Per Liden
Hi Kim, On 2016-06-24 22:09, Kim Barrett wrote: Please review this change which moves the Reference pending list and locking from the java.lang.ref.Reference class into the VM. This includes elimination of the ReferencePendingListLocker mechanism in the VM. This fixes various fragility issues

Re: RFR: 8156500: deadlock provoked by new stress test com/sun/jdi/OomDebugTest.java

2016-06-27 Thread David Holmes
Hi Kim, I like all the removal of the pending-list-locker related code, but can't really comment on the details of how it was replaced and interacts with all the GC code. The interface to the Reference class is fine, it is the GC code that pushes into the reference queue that I can't really

Re: RFR: 8156500: deadlock provoked by new stress test com/sun/jdi/OomDebugTest.java

2016-06-27 Thread Daniel D. Daugherty
> Webrev: > http://cr.openjdk.java.net/~kbarrett/8156500/jdk.00/ make/mapfiles/libjava/mapfile-vers No comments. src/java.base/share/classes/java/lang/ref/Reference.java Much cleaner (pun might have been intended :-)). src/java.base/share/native/include/jvm.h No comments.

Re: RFR: 8156500: deadlock provoked by new stress test com/sun/jdi/OomDebugTest.java

2016-06-26 Thread David Holmes
One more follow up before I actually review the code ... On 25/06/2016 6:09 AM, Kim Barrett wrote: Please review this change which moves the Reference pending list and locking from the java.lang.ref.Reference class into the VM. This includes elimination of the ReferencePendingListLocker

Re: RFR: 8156500: deadlock provoked by new stress test com/sun/jdi/OomDebugTest.java

2016-06-26 Thread David Holmes
Hi Kim, On 25/06/2016 6:09 AM, Kim Barrett wrote: Please review this change which moves the Reference pending list and locking from the java.lang.ref.Reference class into the VM. This includes elimination of the ReferencePendingListLocker mechanism in the VM. This fixes various fragility

RFR: 8156500: deadlock provoked by new stress test com/sun/jdi/OomDebugTest.java

2016-06-24 Thread Kim Barrett
Please review this change which moves the Reference pending list and locking from the java.lang.ref.Reference class into the VM. This includes elimination of the ReferencePendingListLocker mechanism in the VM. This fixes various fragility issues arising from having the management of this list