review request 7195151: Multiplatform tescase for 6929067

Kevin Walls kevin.walls at oracle.com
Fri Oct 12 07:09:17 PDT 2012


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/df291d0a/attachment.html 


More information about the hotspot-runtime-dev mailing list