On 10/13/15 2:43 AM, Jaroslav Bachorik wrote:
On 12.10.2015 19:09, Daniel D. Daugherty wrote:
> http://cr.openjdk.java.net/~jbachorik/8135188/webrev.02
test/serviceability/dcmd/gc/RunFinalizationTest.java
No comments.
test/serviceability/dcmd/gc/FinalizationRunner.java
L58: o = new MyObject();
L59: o = null;
L79: o = new MyObject();
L80: o = null;
So now two different threads are initializing this static
field:
55 public static MyObject o;
and both are clearing it. Is that just a left over
from simplifying the test?
Actually, this is needed for the successful test run. On L79-80 the
instance is made eligible for finalization so we can block the regular
finalizer thread and make sure that the instance from L58-59 is
finalized due to GC.run_finalization.
I've updated the test to make the test intentions clear - added more
comments and debug output.
http://cr.openjdk.java.net/~jbachorik/8135188/webrev.03
test/serviceability/dcmd/gc/RunFinalizationTest.java
No comments.
test/serviceability/dcmd/gc/FinalizationRunner.java
L42: System.out.println("inisde the regular finalizer thread;
blocking");
Typo: 'inisde' -> 'inside'
L61: /* this instance will be used to provoke the regular
finalization
L62: so the finalizer thread can be blocked for the duration of
L63: GC.run_finalization test */
Please switch to '//' style comments for this block.
L66: /* this instance will be used to perform the
GC.run_finalization test */
Please switch to '//' style comment here.
I don't need to see a new webrev for the above changes.
Thanks for the refactor and the new comments. Your intent
is much more clear now.
Thumbs up.
Dan
Thanks,
-JB-
Dan
On 10/12/15 2:00 AM, Jaroslav Bachorik wrote:
On 9.10.2015 20:05, Martin Buchholz wrote:
On Thu, Oct 8, 2015 at 11:51 PM, Jaroslav Bachorik
<jaroslav.bacho...@oracle.com <mailto:jaroslav.bacho...@oracle.com>>
wrote:
On 8.10.2015 18:56, Martin Buchholz wrote:
Hi Jaroslav,
we all keep writing finalization code like this... welcome to
the club!
I think it would be better :
- never use currentTimeMillis to measure elapsed time; use
nanoTime instead
Ok. I suppose this would be because currentTimeMillis() is
dependent
on the OS time, right?
- why use complex Phaser when simple CountDownLatch will do?
The logic is more complex than just waiting for the
finalization to
happen. I need to make sure the finalization happened due to
GC.run_finalization command and not because of an ordinary GC
run or
JVM shutdown. I will update the test comments to make this clear.
Oh, now I see what you're doing - you need to block the regular
finalizer thread to make sure there will be objects available for the
secondary finalizer thread to process. Although Phaser works for
this,
I like using simple latches - CountDownLatch(1) - because they are
easier to understand.
CountDownLatch done = new CountDownLatch(1);
in primary finalizer thread, call done.await
in secondary finalizer thread, call done.countDown to release the
primary finalizer thread
Ok, I took a look at the test from distance and simplified it a bit.
Did a test run of 500 iterations in tight loop without failure.
http://cr.openjdk.java.net/~jbachorik/8135188/webrev.02
-JB-
