RFR (M) 8005012: Add WB APIs to better support NMT testing

[Zhengyu Gu]

Please see comments inline.

On 1/11/2013 1:48 AM, David Holmes wrote:
> On 10/01/2013 8:52 PM, Christian Törnqvist wrote:
>> Hi David,
>>
>>> This method blocks while holding the query_lock - is that intended?
>>
>> Yes, this will prevent NMT from shutting down and freeing the data 
>> structures we're using.
>
> Ok. Of course this screams deadlock potential to me :)
>
No, I don't think that there is deadlock potential.  _query_lock is used 
to serialize NMT queries, there is no NMT related lock  can be acquired 
before it.

>>> Aside: the MemSnapshot::wait method is troubling me. What are you 
>>> waiting upon ie what state condition? The notification is done in the
>>> destructor but how can the destructor be called if someone is calling
>>> wait() upon the object's lock ??? It means you are destroying an 
>>> object that is still in use, including the lock that is being waited 
>>> upon!!!
>>
Yes, have notify_all in destructor is a really bad idea, but you do need 
notify_all before delete memSnapshot, since 
MemTracker::wbtest_wait_for_data_merge() can still wait on snapshot ... 
How about in MemTracker::final_shutdown()

    _snapshot->notify_all_waiters();
   delete _snapshot;

Thanks,

-Zhengyu

>> This was obviously incorrect, thanks for catching this. I've removed 
>> it and done some small other cleanups, a new webrev can be found at 
>> http://cr.openjdk.java.net/~ehelin/8005012/webrev.01/
>
> I don't see memSnapshot in the new webrev - was this all new stuff 
> that is now gone?
>
> David
>
>
>> Thanks,
>> Christian
>>
>> -----Original Message-----
>> From: David Holmes
>> Sent: den 19 december 2012 00:21
>> To: Christian Törnqvist
>> Cc: hotspot-runtime-dev at openjdk.java.net; Zhengyu Gu
>> Subject: Re: RFR (M) 8005012: Add WB APIs to better support NMT testing
>>
>> On 19/12/2012 12:10 AM, Christian Törnqvist wrote:
>>> Hi everyone,
>>>
>>> This change is about adding three new WB APIs to better support NMT
>>> testing. The changes are:
>>>
>>> ·A Test memory type, used by WB API's NMTAllocTest and
>>> NMTFreeTestMemory
>>>
>>> ·Added a WaitForDataMerge WB API to be able to block until the current
>>> generation has been merged and is visible, most of this work was done
>>> by Zhengyu Gu.
>>
>> This method blocks while holding the query_lock - is that intended?
>>
>> Aside: the MemSnapshot::wait method is troubling me. What are you 
>> waiting upon ie what state condition? The notification is done in the 
>> destructor but how can the destructor be called if someone is calling
>> wait() upon the object's lock ??? It means you are destroying an 
>> object that is still in use, including the lock that is being waited 
>> upon!!!
>>
>> David
>> -----
>>
>>>
>>> Webrev can be found at:
>>>
>>> http://cr.openjdk.java.net/~ehelin/8005012/webrev.00/
>>>
>>> Thanks,
>>>
>>> Christian
>>>