Re: Request for review, 6766644: Redefinition of compiled method fails with assertion "Can not load classes with the Compiler thread"

2011-02-01 Thread David Holmes
My apologies to Keith, I was completely missing the fact that the QueueNode class has to hold a copy of the event object due to the limited lifetime of the event objects themselves. Sorry. David David Holmes said the following on 02/02/11 10:30: Keith McGuigan said the following on 02/02/11

Re: Request for review, 6766644: Redefinition of compiled method fails with assertion "Can not load classes with the Compiler thread"

2011-02-01 Thread David Holmes
Keith McGuigan said the following on 02/02/11 10:26: On Feb 1, 2011, at 6:38 PM, David Holmes wrote: The point is to avoid the need to dynamically allocate the QueueNodes. If the elements to be queued are self-linking then you don't need the extra wrapper objects. Take a look at VM_Operations a

Re: Request for review, 6766644: Redefinition of compiled method fails with assertion "Can not load classes with the Compiler thread"

2011-02-01 Thread Keith McGuigan
On Feb 1, 2011, at 6:38 PM, David Holmes wrote: The point is to avoid the need to dynamically allocate the QueueNodes. If the elements to be queued are self-linking then you don't need the extra wrapper objects. Take a look at VM_Operations as an example of self-linking objects. It also m

Re: Request for review, 6766644: Redefinition of compiled method fails with assertion "Can not load classes with the Compiler thread"

2011-02-01 Thread David Holmes
Keith McGuigan said the following on 02/01/11 23:11: On Jan 31, 2011, at 9:20 PM, David Holmes wrote: As per your other email NULL check is pointless as "new" will never return null but will instead call vm_exit_out_of_memory to abort the VM - which is exactly my point. I don't believe that a f

Re: Request for review, 6766644: Redefinition of compiled method fails with assertion "Can not load classes with the Compiler thread"

2011-02-01 Thread Keith McGuigan
On Jan 31, 2011, at 9:20 PM, David Holmes wrote: Where we have: 961 void JvmtiDeferredEventQueue::enqueue(const JvmtiDeferredEvent& event) { ... 967 QueueNode* node = new QueueNode(event); and 1007 void JvmtiDeferredEventQueue::add_pending_event( 1008 const JvmtiDeferredEvent& eve

Re: Request for review, 6766644: Redefinition of compiled method fails with assertion "Can not load classes with the Compiler thread"

2011-01-31 Thread David Holmes
Hi Keith, I've looked at the new webrev - thanks. Keith McGuigan said the following on 02/01/11 00:32: On Jan 30, 2011, at 6:58 PM, David Holmes wrote: Since Service_lock is a 'special' lock (see mutex.hpp), no locks can be acquired when holding the service-lock. I believe the locking code

Re: Request for review, 6766644: Redefinition of compiled method fails with assertion "Can not load classes with the Compiler thread"

2011-01-31 Thread Keith McGuigan
Here's a webrev with the changes I said I'd make, for re-review: http://cr.openjdk.java.net/~kamg/6766644/webrev.03/ (there was no need for a NULL check for those 'new QueueNode()' statements as the allocation code called by CHeapObj already does out- of-memory checking). -- - Keith On J

Re: Request for review, 6766644: Redefinition of compiled method fails with assertion "Can not load classes with the Compiler thread"

2011-01-31 Thread Keith McGuigan
Hi David, On Jan 30, 2011, at 6:58 PM, David Holmes wrote: This is a pretty major change in the event architecture and I have a few comments and concerns - the main concern being "deadlock" as it's not obvious exactly what locks can be held when the service_lock will be acquired, nor is

Re: Request for review, 6766644: Redefinition of compiled method fails with assertion "Can not load classes with the Compiler thread"

2011-01-30 Thread David Holmes
Hi Keith, I've been waiting for the dust to settle a little on this before commenting. I don't know the semantics of the events enough to comment on whether this deferred event processing can impact things, but I can comment on the general approach. This is a pretty major change in the event

Re: Request for review, 6766644: Redefinition of compiled method fails with assertion "Can not load classes with the Compiler thread"

2011-01-28 Thread Keith McGuigan
Ok, here's my next attempt: http://cr.openjdk.java.net/~kamg/6766644/webrev.02 This enqueues the compiled-method-unloaded events so that they the posting of load/unload will be in order. Other changes include locking the nmethod until after the compiled-method-load event is posted, and a

Re: Request for review, 6766644: Redefinition of compiled method fails with assertion "Can not load classes with the Compiler thread"

2011-01-26 Thread John Pampuch
Regarding multiple versions of compiled code... It is not the case for the real-time JVM, if I recall correctly.  Depending on the context in which code is running (scoped memory vs. regular heap, and a few other factors) I think there can be as many

Re: Request for review, 6766644: Redefinition of compiled method fails with assertion "Can not load classes with the Compiler thread"

2011-01-26 Thread Daniel D. Daugherty
On 1/26/2011 2:52 PM, Tom Rodriguez wrote: On Jan 26, 2011, at 1:39 PM, Daniel D. Daugherty wrote: It also looks like there must be order between the load and unload events: CompiledMethodLoad for foo CompiledMethodUnload for foo CompiledMethodLoad for foo (again)

Re: Request for review, 6766644: Redefinition of compiled method fails with assertion "Can not load classes with the Compiler thread"

2011-01-26 Thread Tom Rodriguez
On Jan 26, 2011, at 1:39 PM, Daniel D. Daugherty wrote: > Sorry for being late to this thread... > > > On 1/25/2011 12:00 PM, Tom Rodriguez wrote: >> On Jan 25, 2011, at 8:49 AM, Keith McGuigan wrote: >> >> >> >>> Hello, >>> >>> This code modifies the way that JVMTI compiled-method-load e

Re: Request for review, 6766644: Redefinition of compiled method fails with assertion "Can not load classes with the Compiler thread"

2011-01-26 Thread Daniel D. Daugherty
On 1/25/2011 12:44 PM, Kelly O'Hair wrote: On Jan 25, 2011, at 11:00 AM, Tom Rodriguez wrote: On Jan 25, 2011, at 8:49 AM, Keith McGuigan wrote: Hello, This code modifies the way that JVMTI compiled-method-load events are posted.  Previousl

Re: Request for review, 6766644: Redefinition of compiled method fails with assertion "Can not load classes with the Compiler thread"

2011-01-26 Thread Daniel D. Daugherty
Sorry for being late to this thread... On 1/25/2011 12:00 PM, Tom Rodriguez wrote: On Jan 25, 2011, at 8:49 AM, Keith McGuigan wrote: Hello, This code modifies the way that JVMTI compiled-method-load events are posted. Previously, they were posted directly from the compiler th

Re: Request for review, 6766644: Redefinition of compiled method fails with assertion "Can not load classes with the Compiler thread"

2011-01-25 Thread Karen Kinnear
Currently to maintain the proper ordering of load and unload events, when you post a compiled_method_load event, if there are pending compiled_method_unload events, then we "lock" the nmethod to ensure that the nmethod is not flushed or unloaded while posting the event. What if we did that until

Re: Request for review, 6766644: Redefinition of compiled method fails with assertion "Can not load classes with the Compiler thread"

2011-01-25 Thread Kelly O'Hair
On Jan 25, 2011, at 11:00 AM, Tom Rodriguez wrote: On Jan 25, 2011, at 8:49 AM, Keith McGuigan wrote: Hello, This code modifies the way that JVMTI compiled-method-load events are posted. Previously, they were posted directly from the compiler thread, which could cause issues if the JV

Re: Request for review, 6766644: Redefinition of compiled method fails with assertion "Can not load classes with the Compiler thread"

2011-01-25 Thread Tom Rodriguez
On Jan 25, 2011, at 8:49 AM, Keith McGuigan wrote: > > Hello, > > This code modifies the way that JVMTI compiled-method-load events are posted. > Previously, they were posted directly from the compiler thread, which could > cause issues if the JVMTI event handling code made calls to Redefine

Request for review, 6766644: Redefinition of compiled method fails with assertion "Can not load classes with the Compiler thread"

2011-01-25 Thread Keith McGuigan
Hello, This code modifies the way that JVMTI compiled-method-load events are posted. Previously, they were posted directly from the compiler thread, which could cause issues if the JVMTI event handling code made calls to RedefineClasses, since the compiler thread is unable to load class