review request 7195151: Multiplatform tescase for 6929067

Fri Oct 12 07:32:08 PDT 2012

Looks ok to me.

--
- Keith

On Oct 12, 2012, at 10:09 AM, Kevin Walls wrote:

> Hi,
> 
> Just a ping on this testcase change review request.  It is clearly a lesson in why compiling native code within a test script is so to be avoided, but as it exists, it'd be nice to get it working and and integrated... 8-)
> 
> Thanks
> Kevin
> 
> 
> 
> On 19/09/12 22:27, Kevin Walls wrote:
>> 
>> Hi,
>> 
>> I'd like to get a review of this testcase change we were discussing recently.
>> 
>> http://cr.openjdk.java.net/~kevinw/7195151/webrev/
>> 
>> Andrew, with apologies for the time to get around this, you could decide to be reviewer/submitter/contributor or whatever you feel is appropriate... 8-)
>> 
>> (An update since last time is that I made the 32-bit arm test drop the -m32 as it wasn't recognised on at least some test machines, and isn't necessary in that case.)
>> 
>> Many thanks
>> Kevin
>> 
>> -------- Original Message --------
>> Subject:	Re: Review request 7157734 testcase corrections (use TESTVMOPTS).
>> Date:	Fri, 24 Aug 2012 12:40:52 -0400 (EDT)
>> From:	Andrew Hughes <ahughes at redhat.com>
>> To:	Kevin Walls <kevin.walls at oracle.com>
>> CC:	Gary Collins <gary.collins at oracle.com>, hotspot-runtime-dev at openjdk.java.net
>> 
>> ----- Original Message -----
>> > On 24/08/12 14:30, Andrew Hughes wrote:
>> > > ----- Original Message -----
>> > >> Hi,
>> > >>
>> > >> Various hotpsot .sh testcase scripts do not use the env var
>> > >> TESTVMOPTS,
>> > >> which is passed by jtreg.  They therefore don't set -d64 when we
>> > >> want
>> > >> to
>> > >> test a 64-bit JVM and on Solaris at least this means we don't test
>> > >> what
>> > >> we think we're testing.  We actually run a 32-bit JVM, and likely
>> > >> not
>> > >> even the one we just built to test.
>> > >>
>> > >> What some testcases do do, is to read $HOME/JDK64BIT if it exists,
>> > >> and
>> > >> use the file contents as command-line arguments, or even just as a
>> > >> flag
>> > >> to know that we should set -d64 when we run java.  It seems best
>> > >> to
>> > >> get
>> > >> rid of this practice.  I have asked around and found nobody who
>> > >> can
>> > >> say
>> > >> that technique is still in use.
>> > >>
>> > >> http://cr.openjdk.java.net/~kevinw/7157734.1/webrev/
>> > >>
>> > >> Thanks
>> > >> Kevin
>> > >>
>> > > I've come across your changes to Test6929067.sh as they conflicted
>> > > with
>> > > our changes, which we posted here:
>> > >
>> > > http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2011-May/002163.html
>> > >
>> > > but which were never accepted.
>> > >
>> > > Have you tested this on anything but a x86 GNU/Linux box?
>> > >
>> > > This test:
>> > >
>> > > ${TESTJAVA}/bin/java ${TESTVMOPTS} -version 2>1 | grep
>> > > "64-Bit">/dev/null
>> > > if [ "$?" = "0" ]
>> > > then
>> > >    ARCH=amd64
>> > > else
>> > >    ARCH=i386
>> > > fi
>> > >
>> > > is flawed.  Something that returns "64-Bit" could also be SPARC64
>> > > on GNU/Linux
>> > > or (via Zero) PPC64, etc.
>> > >
>> > > I also don't see how:
>> > >
>> > > gcc -o invoke -I${TESTJAVA}/include -I${TESTJAVA}/include/linux
>> > > invoke.c ${TESTJAVA}/jre/lib/${ARCH}/client/libjvm.so
>> > >
>> > > will work on x86_64 as there is no client VM.
>> > >
>> > > Our fix wasn't perfect either, but, from our perspective, it's
>> > > better than this.  Can
>> > > we perhaps come up with something between the two that works for
>> > > everyone?
>> > >
>> > > Also, if this is only building on GNU/Linux, you can drop:
>> > >
>> > > /usr/openwin/lib:/usr/dt/lib
>> > >
>> > > from LD_LIBRARY_PATH.
>> > 
>> > Hi Andrew,
>> > 
>> > Yes, I'm sure we can get the best of all these changes in there...  I
>> > was mainly just trying to banish the use of BIT_FLAG and use
>> > TESTVMOPTS
>> > where it had been ignored before.  Actually yes there are a few more
>> > issues that need fixing!...
>> > 
>> 
>> Wow, thanks for doing this so quickly!
>> 
>> > 
>> > If we do "${TESTJAVA}/bin/java -d64" and check the return code, we're
>> > assuming that 64-bit is not possible when jtreg is testing 32-bit.
>> >  So
>> > that's similar to the old way this would only run the 32-bit JVM,
>> > ignoring the TESTVMOPTS from jtreg saying it wanted 64.  So I think
>> > we
>> > need to run ${TESTJAVA}/bin/java ${TESTVMOPTS} -version and see what
>> > bitness it reports.
>> > 
>> 
>> Yes, this is why I was thinking ours isn't perfect either... :-)
>> 
>> > I'm not sure we can include both client and server on the compile
>> > line.
>> > Both might be present, but TESTVMOPTS might or might not specify
>> > -server.  Or we might get server by default.  Oh, need again to parse
>> > -version output.  (That's probably why it was hardcoded to client
>> > originally, for simplicity, but that's out of date if we aren't
>> > always
>> > getting client by default.)
>> > 
>> 
>> That seems a good fix.  So we get now whatever matches the output
>> of java -version in all cases.
>> 
>> > uname -m might return x86_64, although we might be testing 32-bit
>> > i386.
>> > Another wrinkle for the ARCH switch...
>> > 
>> 
>> Ah, true.  I don't know about the other archs, I guess the same could
>> happen with SPARC and PPC?  So we probably need something similar for
>> those, and probably aarch64 at some point.
>> 
>> For example, uname -m on my PowerMac returns ppc64 but the userland
>> is 32-bit.
>> 
>> All this gets messed up if the third line doesn't come from HotSpot,
>> but it is a HotSpot test ;-)
>> 
>> > I put a combined suggestion here:
>> > http://cr.openjdk.java.net/~kevinw/0001/webrev/
>> > 
>> > Do you think that captures everything?  I think you're saying that
>> > the
>> > arm and ppc architectures fall through the switch and get used as
>> > they
>> > are in the the later paths.
>> > 
>> 
>> Yes, they would in our patch (while in yours, there were set to i386).
>> I guess if we need to handle 32-bit on 64-bit, we need some switches
>> for those too.
>> 
>> I've added a few extra cases here:
>> 
>> http://cr.openjdk.java.net/~andrew/6929067/webrev.01/
>> 
>> and also fixed a typo (585/586 :-)
>> 
>> Pavel also spotted that you may need to add /usr/lib64 to LD_LIBRARY_PATH
>> as well as /usr/lib
>> 
>> > Let me know what you think and I'll create a bug...
>> > 
>> > Thanks!
>> > Kevin
>> > 
>> > 
>> > 
>> > 
>> > 
>> > 
>> > 
>> > 
>> 
>> Thanks!
>> -- 
>> Andrew :)
>> 
>> Free Java Software Engineer
>> Red Hat, Inc. (http://www.redhat.com)
>> 
>> PGP Key: 248BDC07 (https://keys.indymedia.org/)
>> Fingerprint = EC5A 1F5E C0AD 1D15 8F1F  8F91 3B96 A578 248B DC07
>> 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/attachments/20121012/1771311e/attachment-0001.html 