RFR (S): 8006965: test_gamma should run with import JDK

Vladimir Kozlov vladimir.kozlov at oracle.com
Wed Mar 20 15:12:40 PDT 2013 

I tested it with my workflow and it produces a lot more output and I did 
not specify verbose output. It is caused by next change in 
make/solaris/makefiles/buildtree.make:

   include $(GAMMADIR)/make/scm.make
+ include $(GAMMADIR)/make/defs.make
   include $(GAMMADIR)/make/altsrc.make

Why do you need it there?

Vladimir

On 3/20/13 12:14 PM, Christian Thalinger wrote:
>
> On Mar 19, 2013, at 7:16 PM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:
>
>> Add new test_* targets to default targets to execute a test when you do 'make all_fastdebug' for example.
>
> Yesterday we thought that's a good idea but it's a problem for cross-compilations (e.g. JPRT uses all_* targets).  So I'd rather not add test_* targets to all_* targets.
>
>>
>> The next looks strange:
>>
>> + test_fastdebug: generic_test
>> + test_fastdebug: VM_FLAVOR=fastdebug
>>
>> May be it is time to let Queens go and use -help as you suggested
>
> Some data on using -help instead of Queens:
>
> - C2 compiles one method when running Queens and two methods for -help
> - C1 compiles around 44 for Queens and around 20 for -help
> - tiered compiles 73 for Queens and 33 for -help
>
> So it's about the same smoke test for C2 but half for C1.  Which is think is enough.
>
> None of both do any GC testing.
>
>>   or 'cd test; make VM_FLAVOR=$@ servertest' (as JPRT does).
>
> No, that doesn't work because you need to install the libjvm into a JDK for running that test.
>
> Here is a new webrev:
>
> http://cr.openjdk.java.net/~twisti/8006965/
>
> (PS: I don't know why udiffs don't work in this webrev.)
>
> -- Chris
>
>> Vladimir
>>
>> On 3/19/13 6:16 PM, Christian Thalinger wrote:
>>>
>>> On Mar 13, 2013, at 4:44 PM, Christian Thalinger <christian.thalinger at oracle.com> wrote:
>>>
>>>>
>>>> On Feb 26, 2013, at 6:09 PM, Christian Thalinger <christian.thalinger at oracle.com> wrote:
>>>>
>>>>>
>>>>> On Feb 25, 2013, at 11:24 PM, David Holmes <David.Holmes at oracle.com> wrote:
>>>>>
>>>>>> On 26/02/2013 4:42 AM, Christian Thalinger wrote:
>>>>>>> On Feb 24, 2013, at 2:54 PM, David Holmes <David.Holmes at 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!
>>>>>
>>>>> I couldn't agree more.
>>>>>
>>>>>>
>>>>>> 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 at sc14a501:/export/twisti/build/graal/build/linux_amd64_compiler2/product$ export JAVA_HOME=/java/re/jdk/8/latest/binaries/linux-x64
>>>>>>> cthaling at 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 at sc14a501:/export/twisti/build/graal/build/linux_amd64_compiler2/product$ export JAVA_HOME=/foo
>>>>>>> cthaling at 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?
>>>>
>>>> Ha!  I can remember.  It's used by jmvti.make (implicitly).  The logic in rules.make is:
>>>>
>>>> ifdef ALT_BOOTDIR
>>>>
>>>> COMPILE.JAVAC = $(ALT_BOOTDIR)/bin/javac
>>>>
>>>> else
>>>>
>>>> ifdef BOOTDIR
>>>>
>>>> COMPILE.JAVAC = $(BOOTDIR)/bin/javac
>>>>
>>>> else
>>>>
>>>> ifdef JAVA_HOME
>>>> else
>>>>
>>>> endif
>>>> endif
>>>> endif
>>>>
>>>> BOOTDIR is not defined in rules.make and JAVA_HOME is picked up (which is set to ABS_BOOTDIR).
>>>
>>> Back to my favorite review these days :-)  I put the BOOTDIR setting back because we need it.
>>>
>>> Any reviewers who approve or veto pushing this change?
>>>
>>> -- Chris
>>>
>>>>
>>>> This all sucks and needs to be replaced by something completely different.  Soon.
>>>>
>>>> -- Chris
>>>>
>>>>>> There is no use of it in http://cr.openjdk.java.net/~twisti/8006965/8006965.patch and I don't see it pre-existing ??
>>>>>
>>>>> I talked to John Coomes about that yesterday and we can remove that line.  ABS_BOOTDIR is only used by Windows.
>>>>>
>>>>> -- Chris
>>>>>
>>>>>>
>>>>>> 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.thalinger at oracle.com> wrote:
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On Feb 22, 2013, at 12:58 AM, Staffan Larsen <staffan.larsen at 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.thalinger at 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
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>
>>>>>
>>>>
>>>
>

More information about the hotspot-dev mailing list