Round 3: RFR: 8013651 NMT: reserve/release sequence id's in incorrect order due to race

[David Holmes]

Hi Zhengyu,

Not a full review by any means. I understand the issue being addressed 
in basic form, and see how you have addressed it, but I'm not familiar 
enough with the NMT details to evaluate in depth.

Minor typos in memTracker.cpp comments:

shoul -> should
boundry -> boundary

I'm confused about the expectations of the constructor:

MemTracker::Tracker::Tracker(MemoryOperation op, Thread* thr)

Is thr, if not NULL, always the current thread? If so, then I don't 
think it would be allocating from a SafepointSafe state; and if not then 
it could change state immediately after you have checked it!

In memTracker.hpp for the !INCLUDE_NMT case class Tracker doesn't have 
an allocation-type as a supertype. I'm also unclear whether the methods 
that have:

return Tracker();

are going to be returning a stack-allocated object, or whether this will 
actually force use of a copy-constructor at the call site. We really 
want this to be a no-op (or as close as possible) when NMT is not 
compiled in. Perhaps a static singleton instance of Tracker could be 
used instead?

Thanks,
David

On 5/06/2013 12:26 AM, Zhengyu Gu wrote:
> Round 3:
>
> Based on Coleen and Stefan's comment, reverted most of NMT tracking
> calls to original to reduce code changes.
>
> http://cr.openjdk.java.net/~zgu/8013651/webrev.03/
> <http://cr.openjdk.java.net/%7Ezgu/8013651/webrev.03/>
>
>
> Tests:
>    JPRT
>    vm.quick.testlist on Linux 32
>
> Thanks,
>
> -Zhengyu
>
>
> On 5/22/2013 10:28 AM, Zhengyu Gu wrote:
>> Based on the discussion with Karen, Coleen and Harold, following
>> changes are made:
>>
>> 1) Renamed NMTTrackOp to NMTTracker, avoid the confusion with VM
>> operations.
>> 2) Used NMTTracker's dtor to discard the tracking operation if no
>> recording is done.
>>
>> Tests:
>>  - JPRT
>>  - vm.quick.testlist on Linux 32, Solaris sparcv9 and Windows 32.
>>
>> Thanks,
>>
>> -Zhengyu
>>
>>
>> On 5/14/2013 10:01 AM, Zhengyu Gu wrote:
>>> There can be race conditions between the memory operations and the
>>> book keeping records are written. For example, thread 1 releases a
>>> virtual memory block, before it can write the release record, thread
>>> 2 reserves the same virtual memory block and writes reservation
>>> first, as result, NMT indicates the block is "released".
>>>
>>> The solution is that, for those operations that can cause the race
>>> conditions, NMT should pre-reserve sequence number for it, if the
>>> operation succeeds, NMT uses pre-reserved sequence number to write
>>> the record.
>>>
>>> The tricky part is that, a sequence number is only good for the
>>> generation it is acquired, when there are reserved sequence number,
>>> NMT has to prevent itself from entering so called "sync-point" where
>>> the generation can be advanced.
>>>
>>>
>>> Bug: http://bugs.sun.com/view_bug.do?bug_id=8013651
>>> Webrev: http://cr.openjdk.java.net/~zgu/8013651/webrev/
>>> <http://cr.openjdk.java.net/%7Ezgu/8013651/webrev/>
>>>
>>>
>>> Tests:
>>>    1) JPRT
>>>    2) vm.quick.testlist on Linux 32, Linux x64 and Solaris Sparcv9
>>>    3) Kitchensink on Linux 32, Linux x64, Solaris Sparcv9 and Windows
>>> x64
>>>    4) NMT jtreg tests on Linux 32, Linux x64 and Solaris Sparcv9
>>>
>>>
>>> Thanks,
>>>
>>> -Zhengyu
>>
>