Re: Mauve patch

Re: Mauve patch

[Sascha Brawer]

Archie Cobbs <archie@dellroad.org> wrote on Mon, 5 Apr 2004 22:30:34 -0500:

>Is this the right place to send Mauve patches? If not please point me
>(or the patch) in the right direction.

There's a list mauve-discuss@sources.redhat.com  (presumably with mostly
the same subscribers as commit-classpath@gnu.org, although there seem to
be also a few non-Classpath people there).

>In JC, phantom references are enqueued on the next finalizer run
>after the one that finalizes the object, and finalizer runs only
>happen after a GC cycle, therefore the patch below is required to
>make JC pass the test; ie, the test is being too strict.


>Index: gnu/testlet/java/lang/ref/PhantomReference/phantom.java
>===================================================================
>RCS file: /cvs/mauve/mauve/gnu/testlet/java/lang/ref/PhantomReference/
>phantom.java,v
>retrieving revision 1.1
>diff -u -r1.1 phantom.java
>--- gnu/testlet/java/lang/ref/PhantomReference/phantom.java	27 Sep
>2001 15:44:09 -0000	1.1
>+++ gnu/testlet/java/lang/ref/PhantomReference/phantom.java	6 Apr
>2004 03:28:19 -0000
>@@ -70,6 +70,8 @@
> 
>     PhantomReference wr = try1 (q, harness);
>     System.gc ();
>+    Thread.yield();
>+    System.gc ();
> 
>     Reference r = null;
>     try

Does this really guarantee that the finalizer has run? Couldn't this also
lead to any other thread, such as some VM-internal thread, without
running the finalizer? If so, you might want to call Object.notify in the
finalizer and Object.wait at the above code location.

-- Sascha

Sascha Brawer, brawer@dandelis.ch, http://www.dandelis.ch/people/brawer/ 

Re: Mauve patch

[Ingo Prötel]

Sascha Brawer wrote:
> Archie Cobbs <archie@dellroad.org> wrote on Mon, 5 Apr 2004 22:30:34 -0500:
> 
> 
>>Is this the right place to send Mauve patches? If not please point me
>>(or the patch) in the right direction.
> 
> 
> There's a list mauve-discuss@sources.redhat.com  (presumably with mostly
> the same subscribers as commit-classpath@gnu.org, although there seem to
> be also a few non-Classpath people there).
> 
> 
>>In JC, phantom references are enqueued on the next finalizer run
>>after the one that finalizes the object, and finalizer runs only
>>happen after a GC cycle, therefore the patch below is required to
>>make JC pass the test; ie, the test is being too strict.
> 
> 
> 
>>Index: gnu/testlet/java/lang/ref/PhantomReference/phantom.java
>>===================================================================
>>RCS file: /cvs/mauve/mauve/gnu/testlet/java/lang/ref/PhantomReference/
>>phantom.java,v
>>retrieving revision 1.1
>>diff -u -r1.1 phantom.java
>>--- gnu/testlet/java/lang/ref/PhantomReference/phantom.java	27 Sep
>>2001 15:44:09 -0000	1.1
>>+++ gnu/testlet/java/lang/ref/PhantomReference/phantom.java	6 Apr
>>2004 03:28:19 -0000
>>@@ -70,6 +70,8 @@
>>
>>    PhantomReference wr = try1 (q, harness);
>>    System.gc ();
>>+    Thread.yield();
>>+    System.gc ();
>>
>>    Reference r = null;
>>    try
> 
> 
> Does this really guarantee that the finalizer has run? Couldn't this also
> lead to any other thread, such as some VM-internal thread, without
> running the finalizer? If so, you might want to call Object.notify in the
> finalizer and Object.wait at the above code location.
> 
I'm not sure a wait/notify will help. It might also just hang the VM since no garbage collection 
might occur. Probably the only real solution would be to force an OutOfMemoryError. Before the VM 
may throw such an error it has to try to clean up memory. But probably not all VMs survive such a 
test ;-)

ingo


> -- Sascha
> 
> Sascha Brawer, brawer@dandelis.ch, http://www.dandelis.ch/people/brawer/ 
> 
> 
> 

-- 
Ingo Prötel                                          proetel@aicas.com
aicas GmbH                                        http://www.aicas.com
Haid-und-Neu-Str. 18                        phone   +49 721 663 968-32
76131 Karlsruhe                             fax     +49 721 663 968-93
Germany

Re: Mauve patch

[Archie Cobbs]

Sascha Brawer wrote:
> >     PhantomReference wr = try1 (q, harness);
> >     System.gc ();
> >+    Thread.yield();
> >+    System.gc ();
> > 
> >     Reference r = null;
> >     try
> 
> Does this really guarantee that the finalizer has run? Couldn't this also
> lead to any other thread, such as some VM-internal thread, without
> running the finalizer? If so, you might want to call Object.notify in the
> finalizer and Object.wait at the above code location.

This patch doesn't guarantee anything, and in general it's impossible
to make this test "correct" because the spec allows finalization and
reference enqueuing to happen after arbitrarily long delays.

For example, a JVM that *never* finalizes is still within the spec
(it would probably throw OutOfMemoryError's more readily though).

This patch simply makes the test "correct" for JC (and possibly
some other VM(s) out there). Since there's no way to *ensure* the
finalizer and reference enqueing thread(s) have run, we just try
to give them every opportunity to do so before declaring that their
work should be done.

-Archie

__________________________________________________________________________
Archie Cobbs      *        CTO, Awarix        *      http://www.awarix.com

Re: Mauve patch

[Tom Tromey]

>>>>> "Archie" == Archie Cobbs <archie@dellroad.org> writes:

Archie> This patch doesn't guarantee anything, and in general it's impossible
Archie> to make this test "correct" because the spec allows finalization and
Archie> reference enqueuing to happen after arbitrarily long delays.

Yeah, these tests are basically bogus.  I thought I had removed them,
but I guess I forgot.

Archie> This patch simply makes the test "correct" for JC (and possibly
Archie> some other VM(s) out there). Since there's no way to *ensure* the
Archie> finalizer and reference enqueing thread(s) have run, we just try
Archie> to give them every opportunity to do so before declaring that their
Archie> work should be done.

I think the patch is fine to go in.  It certainly doesn't make the
situation any worse.

Perhaps it is better to just remove the test.  Or make a new
"unportable" section of Mauve, since some things seemingly can't be
tested.

Tom

Re: Mauve patch

[Stephen Crawley]

Hi Tom,

> Perhaps it is better to just remove the test. 

That would be my preferred option.

> Or make a new "unportable" section of Mauve, since some things 
> seemingly can't be tested.

I think it would be better for each JVM projects to manage its own
non-portable testcases.   In Kissme for example, there is a small
regression testsuite (called "HitMe") that tests some of Kissme's low
level and VM specific features.  

The HitMe harness assumes that less of the VM infrastructure is working
that the Mauve harness does.  There is even a version that avoids using
java.lang.io.* and uses the rc returned using System.exit() to signal
test success / failure.  Some HitMe tests require special Kissme
commandline arguments.  The makefile understands this ...

-- Steve

Re: Mauve patch

[Archie Cobbs]

Stephen Crawley wrote:
> > Or make a new "unportable" section of Mauve, since some things 
> > seemingly can't be tested.

That's a reasonable idea.. i.e., a "don't panic if these tests fail"
section.

> I think it would be better for each JVM projects to manage its own
> non-portable testcases.   In Kissme for example, there is a small

Would these be kept in the Mauve source or the JVM source? If the
latter, it would be nice for us to define a standard way to tell
Mauve, "oh yeah, also run these JVM-specific tests and by the way
they're in this other directory..." or whatever.

-Archie

__________________________________________________________________________
Archie Cobbs      *        CTO, Awarix        *      http://www.awarix.com

Re: Mauve patch

[Mark Wielaard]

[-- Attachment #1.1: Type: text/plain, Size: 1158 bytes --]

Hi,

On Tue, 2004-04-06 at 15:46, Archie Cobbs wrote:
> This patch doesn't guarantee anything, and in general it's impossible
> to make this test "correct" because the spec allows finalization and
> reference enqueuing to happen after arbitrarily long delays.
> 
> For example, a JVM that *never* finalizes is still within the spec
> (it would probably throw OutOfMemoryError's more readily though).
> 
> This patch simply makes the test "correct" for JC (and possibly
> some other VM(s) out there). Since there's no way to *ensure* the
> finalizer and reference enqueing thread(s) have run, we just try
> to give them every opportunity to do so before declaring that their
> work should be done.

I committed it to mauve as follows since it doesn't break things for
others and might actually help.

2004-04-15  Archie Cobbs  <archie@dellroad.org>

       * gnu/testlet/java/lang/ref/PhantomReference/phantom.java: Give the
       runtime some more hints (Thread.yield/System.gc) that it should
       really garbage collect.

But a "non-portable" section in Mauve is also a good idea. It is just
more work :)

Cheers,

Mark


[-- Attachment #1.2: phantom.patch --]
[-- Type: text/x-patch, Size: 728 bytes --]

Index: gnu/testlet/java/lang/ref/PhantomReference/phantom.java
===================================================================
RCS file: /cvs/mauve/mauve/gnu/testlet/java/lang/ref/PhantomReference/phantom.java,v
retrieving revision 1.1
diff -u -r1.1 phantom.java
--- gnu/testlet/java/lang/ref/PhantomReference/phantom.java	27 Sep 2001 15:44:09 -0000	1.1
+++ gnu/testlet/java/lang/ref/PhantomReference/phantom.java	15 Apr 2004 20:18:34 -0000
@@ -54,6 +54,9 @@
     phantom twt = new phantom ();
     PhantomReference wr = genRef (q, twt);
 
+    // Give the runtime some hints that it should really garbage collect.
+    System.gc ();
+    System.yield ();
     System.gc ();
 
     Reference r = q.poll ();

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]