review request 7195151: Multiplatform tescase for 6929067
Kevin Walls
kevin.walls at oracle.com
Mon Oct 15 08:43:10 PDT 2012
Thanks Vladimir -
For the record, that's a minor update which:
Uses -Xinternalversion once and grep the output as required for
bitness/server.
Uses @compile rather than invoke javac directly.
( updated at http://cr.openjdk.java.net/~kevinw/7195151/002/webrev/ )
Regards
Kevin
On 15/10/12 16:34, Vladimir Kozlov wrote:
> New version is good.
>
> Thanks,
> Vladimir
>
> On Oct 12, 2012, at 7: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/20121015/e4d23ca8/attachment-0001.html
More information about the hotspot-runtime-dev
mailing list