Hi Mandy
Thank you for sponsoring this for me. Change as you suggested
http://cr.openjdk.java.net/~tyan/JDK-8029346/webrev.01/
<http://cr.openjdk.java.net/%7Etyan/JDK-8029346/webrev.01/>
Thank you
Tristan
On 12/19/2013 10:42 AM, Mandy Chung wrote:
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
