Ping - Re: RFR for JDK-8029346 LowMemoryTestConcMarkSweepGC.sh fails intermittently with timeout

[Mandy Chung]

On 12/18/2013 6:00 PM, Tristan Yan wrote:
> Hi Everyone
> This is a reminder. Can anyone help to review this code change.
>
> http://cr.openjdk.java.net/~tyan/JDK-8029346/webrev.00/ 
> <http://cr.openjdk.java.net/%7Etyan/JDK-8029346/webrev.00/>
>

Thanks for the cleanup.   Looks okay.   I spot some minor cleanup if you 
don't mind fixing them.

line 48: List -> List<MemoryPoolMXBean>
line 159: a space is missing after "//" and before "Force"
line 192: List -> List<Object>
line 236: the sweep object is no longer needed.

I suggest to include the phase number in the trace to help 
troubleshooting in the future like this:

  225     System.out.println("AllocatorThread is doing task " + i + " phase " + phaser.getPhase());
  252     System.out.println("SweepThread is doing task " + i + " phase " + phaser.getPhase());

Also add a trace to print after it advances to the next phase in line 228 and 258.

Send me the new patch and I can sponsor your fix.

Mandy

> Description:
> 1. Fix the bug as below described.
> 2. Change the shell script tests to Java tests.
> 3. Using concurrent Phaser sync thread to replace old object sync
>
> Thank you.
> Tristan.
>
> On 12/17/2013 04:38 PM, Tristan Yan wrote:
>> Thanks for the suggestion. Mandy
>>
>> Hi Everyone
>> Please review code fix for JDK-8029346
>>
>> http://cr.openjdk.java.net/~tyan/JDK-8029346/webrev.00/ 
>> <http://cr.openjdk.java.net/%7Etyan/JDK-8029346/webrev.00/>
>>
>> Description:
>> 1. Fix the bug as below described.
>> 2. Change the shell script tests to Java tests.
>> 3. Using concurrent Phaser sync thread to replace old object sync
>>
>> Thank you very much.
>> Tristan
>>
>>
>> On 12/17/2013 03:20 AM, Mandy Chung wrote:
>>> Hi Tristan,
>>>
>>> Thanks for looking into this test failure.
>>>
>>> listenerInvoked is accessed by multiple threads but not volatile.  
>>> That is certainly an issue.  The test was written long ago prior to 
>>> j.u.concurrent.  Can you also replace wait_or_notify with 
>>> j.u.concurrent (I think Semaphore or Phaser should do it)?
>>>
>>> This is probably out of scope of this bug.  I suggest to remove the 
>>> LowMemoryTest*.sh tests and replace them by adding @run in 
>>> LowMemoryTest.java.
>>>
>>> Mandy
>>>
>>> On 12/12/2013 9:12 AM, Tristan Yan wrote:
>>>> Hi Everyone
>>>> I am working on bug https://bugs.openjdk.java.net/browse/JDK-8029346
>>>>
>>>> Root cause for this bug is in CMS mode; gc could happen when 
>>>> AllocatorThread is allocating spaces. in that case. objectPool 
>>>> won't be added two object arrays as it's supposed to have. Also in 
>>>> that case. no clear and gc will be invoked in SweepThread. Which 
>>>> means memory used is still bigger than threshold. According the 
>>>> java doc. Subsequent crossing of the usage threshold value does not 
>>>> cause further notification until the memory usage has returned to 
>>>> become less than the usage threshold value. That's the reason why 
>>>> next notification will never comes.
>>>>
>>>> Suggested fix is making below change in doTask, make sure in every 
>>>> loop two object arrays are added objectPool and gc will be invoked 
>>>> in SweepThread.
>>>> change
>>>> while (!listenerInvoked) {
>>>> into
>>>> while (!listenerInvoked || mpool.getUsage().getUsed() < 
>>>> mpool.getUsageThreshold()) {
>>>>
>>>> Please let me know if you have any comments or suggestions.
>>>>
>>>> Thank you
>>>>
>>>> Tristan
>>>>
>>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20131218/7e8dfd39/attachment-0001.html