RFC: JamVM: there is something odd going on with the langtools tests
Dr Andrew John Hughes
ahughes at redhat.com
Thu Mar 3 06:20:56 PST 2011
On 14:21 Thu 03 Mar , Xerxes Rånby wrote:
> On 2011-03-03 02:35, Dr Andrew John Hughes wrote:
> > On 23:54 Wed 02 Mar , Xerxes Rånby wrote:
> >> On 2011-02-28 01:44, Xerxes Rånby wrote:
> >>> On 2011-02-28 00:37, Xerxes Rånby wrote:
> >>>> On 2011-02-28 00:01, Mark Wielaard wrote:
> >>>> > BTW. If you look at the buildbot test results there is something
> >>>> > odd going on with the langtools tests. There are some failures that
> >>>> > we should take a look at. But the odd thing is that they seem to
> >>>> > just abort after T6397044. I can reproduce this locally.
> >>>> > The logs stop at T6397044, but I have a (passing) T6397286.jtr
> >>>> > file. Which seems to mean the actual failure/crash/abort is in
> >>>> > that one or in processing the results from it. I haven't debugged
> >>>> > it further.
> >>>> >
> >>>> Yes this is odd.
> >>>>
> >>>> I have attached a patch for icedtea6 that disables a lot more
> >>>> Unrecognized options. I belive this will help us to locate tests that
> >>>> take-down JamVM in a unclean way. By finding these failing tests we will
> >>>> have a easier time to fix remaining bugs in JamVM.
> >>> Updated the patch once more.
> >>>> Ok for TRUNK ?
> >> FYI: I have committed this patch with a little sorting to icedtea6 head.
> >> http://icedtea.classpath.org/hg/icedtea6/rev/6c809c02b993
> >>
> > You posted this as RFC. Why did you not then wait for a review?
> First of all, thank you Andrew for reviewing:
>
> My brain are wired something like this:
> Code can be reviewed either before it is committed or after. We expect major changes to be reviewed before being committed, but smaller changes can be reviewed after commit. The developer responsible for a code change is also responsible for making all necessary review-related changes.
>
> I still request for comments but I felt the change to be small enough to be first committed and then reviewed after.
>
Is that a quote from somewhere?
The policy with IcedTea is that review is REQUIRED before commits to release branches and IcedTea-Web, and may be
requested for IcedTea6.
Post-commit review doesn't make sense to me. It effectively puts the onus on the reviewer to fix the committer's
mistakes.
I was about to post my review and then was confused as to why you had committed. Please don't do this again.
> > This seems like the wrong approach. Surely it should default to ignoring an
> > unrecognised option instead of having this whitelist of HotSpot options.
> > I'm pretty sure CACAO does that rather than this mess.
> Good point, I have now checked what CACAO do:
> CACAO contains a small unimplemented whitelist for all the options that are required to build OpenJDK like: -XX:MaxPermSize= and -XX:PermSize= .
> For the remaining unknown -XX options CACAO are simply printing a line to System.err for each unknown -XX option that it encounter.
>
> Still CACAO do not pass most jtreg tests since CACAO stop on any unknown encountered -X option, prints its usage, and then return with exit code 1.
> This are the main reason why CACAO make check-hotspot currently fails most of the tests: Test results: passed: 29; failed: 78
>
> example: CACAO currently fail hotspot/test/compiler/7009359 when it execute the command-line:
> command: main -Xbatch -XX:+IgnoreUnrecognizedVMOptions -XX:+OptimizeStringConcat -XX:CompileCommand=exclude,Test7009359,mainTest7009359
> ----------System.out:(34/1763)----------
> Unknown option: -Xbatch
> Usage: cacao [-options] classname [arguments]
> (to run a class file)
> ...
> ----------System.err:(3/163)----------
> Unknown -XX option: -XX:+IgnoreUnrecognizedVMOptions
> Unknown -XX option: -XX:+OptimizeStringConcat
> Unknown -XX option: -XX:CompileCommand=exclude,Test7009359,main
> result: Failed. Unexpected exit from test [exit code: 1]
>
> I will try post a fix for CACAO in a follow up patch to make CACAO pass all jtreg tests that uses unknown -X options like -Xbatch.
>
>
> In contrast Zero currently only uses a white-list, zero currently accepts all -XX compiler options that Shark support and stop when it encounters anything unknown.
> Both Zero and Shark fails the hotspot/test/compiler/6795161 due to a unsupported -XX:+DoEscapeAnalysisTest option.
> ----------messages:(3/219)----------
> command: main -server -Xcomp -XX:CompileOnly=Test -XX:+DoEscapeAnalysisTest
> reason: User specified action: run main/othervm -server -Xcomp -XX:CompileOnly=Test -XX:+DoEscapeAnalysis Test
> elapsed time (seconds): 0.008
> ----------System.out:(0/0)----------
> ----------System.err:(2/86)----------
> Unrecognized VM option '+DoEscapeAnalysis'
> Could not create the Java virtual machine.
> result: Failed. Unexpected exit from test [exit code: 1]
>
> I will try post a fix for Zero/Shark in a follow up patch to make Zero and Shark continue after they have reported a unknown option.
>
Zero+Shark aren't directly comparable as they presumably leverage some of HotSpot's functionality for most options
and the options make some sense in that context. They are largely non-sensical for CACAO and JamVM.
>
> I have attached a new patch for JamVM that removes the "white-list" patch and makes JamVM print a line to System.err for each unknown option that it encounter and then continue.
>
I think this is the right approach, as also taken by autoconf.
> Cheers
> Xerxex
> Index: icedtea6/Makefile.am
> ===================================================================
> --- icedtea6.orig/Makefile.am 2011-03-03 13:37:46.089904748 +0100
> +++ icedtea6/Makefile.am 2011-03-03 13:37:55.645926119 +0100
> @@ -358,7 +358,7 @@
>
> if BUILD_JAMVM
> ICEDTEA_PATCHES += \
> - patches/jamvm/ignore-more-XX-options.patch
> + patches/jamvm/ignore-unknown-options.patch
> endif
>
> if ENABLE_PULSE_JAVA
> Index: icedtea6/patches/jamvm/ignore-unknown-options.patch
> ===================================================================
> --- /dev/null 1970-01-01 00:00:00.000000000 +0000
> +++ icedtea6/patches/jamvm/ignore-unknown-options.patch 2011-03-03 13:49:56.985399520 +0100
> @@ -0,0 +1,13 @@
> +Index: jamvm/jamvm/src/jni.c
> +===================================================================
> +--- jamvm.orig/jamvm/src/jni.c 2011-02-27 04:35:37.000000000 +0100
> ++++ jamvm/jamvm/src/jni.c 2011-03-03 13:48:31.897900208 +0100
> +@@ -1642,7 +1642,7 @@
> + /* Ignore */
> + } else if(!vm_args->ignoreUnrecognized) {
> + optError(args, "Unrecognised option: %s\n", string);
> +- goto error;
> ++ /* Ignore */
> + }
> + }
> +
--
Andrew :)
Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)
Support Free Java!
Contribute to GNU Classpath and IcedTea
http://www.gnu.org/software/classpath
http://icedtea.classpath.org
PGP Key: F5862A37 (https://keys.indymedia.org/)
Fingerprint = EA30 D855 D50F 90CD F54D 0698 0713 C3ED F586 2A37
More information about the distro-pkg-dev
mailing list