On 26/02/2013 4:42 AM, Christian Thalinger wrote:
On Feb 24, 2013, at 2:54 PM, David Holmes <david.hol...@oracle.com> wrote:
On 23/02/2013 1:55 PM, Christian Thalinger wrote:
I talked to a lot of people about this today.  What we really want is to not 
run tests when we build.  Mikael and I were looking into how we could do that 
without gamma and there is a way:

http://cr.openjdk.java.net/~twisti/8006965/

This would be the first of three fixes:

Fix 1)  The patch above removes test_gamma and uses some weirdness in the VM 
(-Dsun.java.launcher=gamma) to run it with an existing JDK; add 
test_{product,fastdebug,debug} targets

This logic is not suitable:

541 # Testing the built JVM
542 ifeq ($(JAVA_HOME),)
543   RUN_JVM=JAVA_HOME=$(JDK_IMPORT_PATH) $(JDK_IMPORT_PATH)/bin/java 
-d$(ARCH_DATA_MODEL) -server -XXaltjvm=$(MISC_DIR)/$(VM_FLAVOR) 
-Dsun.java.launcher=gamma
544 else
545   RUN_JVM=$(JAVA_HOME)/bin/java -d$(ARCH_DATA_MODEL) -server 
-XXaltjvm=$(MISC_DIR)/$(VM_FLAVOR) -Dsun.java.launcher=gamma
546 endif

I have JAVA_HOME set in my environment for use by other tools/scripts and it 
points at JDK7. The existing logic does not use my environments JAVA_HOME 
setting so neither should the revised logic!

That's not entirely correct.  test_gamma uses your JAVA_HOME setting:

This is so confusing. Our makefiles are an abomination!

So this all started because the makefile has:

JAVA_HOME=$(ABS_BOOTDIR)

which was flagged as wrong because gamma would run in the boot JDK. But now it seems the make variable JAVA_HOME is irrelevant anyway because the test_gamma script will use the JAVA_HOME environment variable.

So how did the boot JDK come back into this???

cthaling@sc14a501:/export/twisti/build/graal/build/linux_amd64_compiler2/product$
 export JAVA_HOME=/java/re/jdk/8/latest/binaries/linux-x64
cthaling@sc14a501:/export/twisti/build/graal/build/linux_amd64_compiler2/product$
 ./test_gamma
Using java runtime at: /java/re/jdk/8/latest/binaries/linux-x64/jre
<snip>
cthaling@sc14a501:/export/twisti/build/graal/build/linux_amd64_compiler2/product$
 export JAVA_HOME=/foo
cthaling@sc14a501:/export/twisti/build/graal/build/linux_amd64_compiler2/product$
 ./test_gamma
JAVA_HOME must point to a 64-bit OpenJDK.

And here comes this little snippet into play:

-MAKE_ARGS += JAVA_HOME=$(ABS_BOOTDIR)
+MAKE_ARGS += BOOTDIR=$(ABS_BOOTDIR)

Only setting JAVA_HOME to ABS_BOOTDIR make test_gamma work even if you have a 
JAVA_HOME set.

I still don't get this. What has BOOTDIR got to do with JAVA_HOME? Where is this BOOTDIR value being used? There is no use of it in http://cr.openjdk.java.net/~twisti/8006965/8006965.patch and I don't see it pre-existing ??

Thanks,
David

I have added this logic so that users can control what JDK is used when running 
the test.  In fact they should use ALT_JDK_IMPORT_PATH if they want to control 
that.


I also don't see why the above sets JAVA_HOME at #543 - what will read that 
environment variable?

It's the odd logic in os::jvm_path guarded by 
Arguments::created_by_gamma_launcher().  A clean-up of that logic would be part 
of Fix 3.


I still have concerns over what JDK_IMPORT_PATH will point to for different JDK 
builders.

It's either JDK_IMPORT_PATH or JDK_IMAGE_DIR.  Since most people don't want to 
export their libjvm into a JDK we have to use JDK_IMPORT_PATH.  We could add 
some logic that checks if JDK_IMAGE_DIR exists and use that one.

-- Chris


And this addition still makes no sense to me:

MAKE_ARGS += BOOTDIR=$(ABS_BOOTDIR)

Why do you need to add BOOTDIR to the MAKE_ARGS?

David


Fix 2)  Remove gamma and all the ugly code that comes with it (copies of the 
jdk launcher in hotspot and other pieces); make the hotspot script work like 
the test targets in Fix 1

Fix 3)  Remove the -Dsun.java.launcher=gamma and possibly replace the existing 
-Dsun.boot.library.path weirdness by explicit command line options like 
-Xbootlibrarypath:{/p,/a}

-- Chris

On Feb 22, 2013, at 3:21 PM, Christian Thalinger 
<christian.thalin...@oracle.com> wrote:


On Feb 22, 2013, at 12:58 AM, Staffan Larsen <staffan.lar...@oracle.com> wrote:

I'm not sure what the correct solution is, but when you do find out, the 
jdkpath.sh target should also be updated.

How many are actually using the hotspot script?  Would people be very 
sentimental if we would remove the gamma launcher altogether?

Taking to people here it seems that most are copying their libjvm into a JDK 
and use java anyway.

-- Chris


Thanks,
/Staffan

On 22 feb 2013, at 03:40, Christian Thalinger <christian.thalin...@oracle.com> 
wrote:

http://cr.openjdk.java.net/~twisti/8006965

8006965: test_gamma should run with import JDK
Reviewed-by:

Right now test_gamma runs with the boot JDK which is JDK n-1 (where
JDK n is the version we are actually compiling for).  This setup is
unsupported and thus should not be done during HotSpot builds.

The fix is to always use JDK_IMPORT_PATH instead of JAVA_HOME when
running test_gamma.

make/bsd/makefiles/buildtree.make
make/defs.make
make/linux/makefiles/buildtree.make
make/solaris/makefiles/buildtree.make





Reply via email to