RFR (XXL): JEP 243: Java-Level JVM Compiler Interface

Christian Thalinger christian.thalinger at oracle.com
Thu Sep 24 18:57:12 UTC 2015


> On Sep 21, 2015, at 12:24 PM, Christian Thalinger <christian.thalinger at oracle.com> wrote:
> 
> 
>> On Sep 18, 2015, at 2:19 PM, Christian Thalinger <christian.thalinger at oracle.com> wrote:
>> 
>> 
>>> On Sep 18, 2015, at 7:00 AM, Christian Thalinger <christian.thalinger at oracle.com> wrote:
>>> 
>>> 
>>>> On Sep 17, 2015, at 11:00 PM, Volker Simonis <volker.simonis at gmail.com> wrote:
>>>> 
>>>> To which repository will you finally push these changes?
>>>> 
>>>> The current webrevs are against jdk9/jdk9/hotspot but I doubt that
>>>> they will be integrated there.
>>> 
>>> Good point.
>>> 
>>>> 
>>>> Unfortunately the patches don't apply to jdk9/dev/hotspot anymore
>>>> because of '8135067: Preparatory refactorings for compiler control'
>>>> and  "8134626: Misc cleanups after generation array removal":
>>>> 
>>>> 3 out of 18 hunks FAILED -- saving rejects to file
>>>> src/share/vm/compiler/compileBroker.cpp.rej
>>>> 1 out of 5 hunks FAILED -- saving rejects to file
>>>> src/share/vm/compiler/compileBroker.hpp.rej
>>>> 6 out of 40 hunks FAILED -- saving rejects to file
>>>> src/share/vm/runtime/vmStructs.cpp.rej
>>>> 
>>>> Will you update the patches and if yes against which repositories?
>>> 
>>> Yes.  Let me update the graal-jvmci-9 repository to hs-comp.
>> 
>> Done.  Now I have to create new webrevs… 
> 
> Here are new webrevs against hs-comp:
> 
> http://cr.openjdk.java.net/~twisti/8136421/webrev/ <http://cr.openjdk.java.net/~twisti/8136421/webrev/>
> http://cr.openjdk.java.net/~twisti/8136421/hotspot/webrev/ <http://cr.openjdk.java.net/~twisti/8136421/hotspot/webrev/>

I have refreshed these webrevs.  They contain all the changes we discussed here plus a bunch of new tests that SQE finished.

> 
>> 
>>> 
>>>> 
>>>> If I want to work on the ppc64 implementation, which repository should I use?
>>> 
>>> graal-jvmci-9.  Are you working on this for fun or do you want to have that integrated with this JEP?
>>> 
>>>> 
>>>> Thank you and best regards,
>>>> Volker
>>>> 
>>>> 
>>>> On Fri, Sep 18, 2015 at 10:20 AM, Volker Simonis
>>>> <volker.simonis at gmail.com> wrote:
>>>>> For the top-level change I always get a strange error when importing it:
>>>>> 
>>>>> patch failed, unable to continue (try -v)
>>>>> patch failed, rejects left in working dir
>>>>> errors during apply, please fix and refresh 8062493_jvmci_top_0911.v1.patch
>>>>> 
>>>>> It is because of the strange path syntax of the last hunk in the patch file:
>>>>> 
>>>>> --- old/./modules.xml 2015-09-16 15:11:14.000000000 -0700
>>>>> +++ new/./modules.xml 2015-09-16 15:11:14.000000000 -0700
>>>>> @@ -254,6 +254,14 @@
>>>>>    <to>jdk.jfr</to>
>>>>>  </export>
>>>>>  <export>
>>>>> +      <name>jdk.internal.jvmci.hotspot</name>
>>>>> +      <to>jdk.jfr</to>
>>>>> +    </export>
>>>>> +    <export>
>>>>> +      <name>jdk.internal.jvmci.hotspot.events</name>
>>>>> +      <to>jdk.jfr</to>
>>>>> +    </export>
>>>>> +    <export>
>>>>>    <name>sun.misc</name>
>>>>>    <to>java.corba</to>
>>>>>    <to>java.desktop</to>
>>>>> 
>>>>> 
>>>>> Notice the strange syntax "old/./modules.xml" and "new/./modules.xml".
>>>>> If I remove the redundant './' from the path, everything works fine.
>>>>> For some reason only the diffs for modules.xml has this strange path
>>>>> syntax in the patch.
>>>>> 
>>>>> Regards,
>>>>> Volker
>>>>> 
>>>>> 
>>>>> On Thu, Sep 17, 2015 at 10:32 PM, Christian Thalinger
>>>>> <christian.thalinger at oracle.com> wrote:
>>>>>> Since there are no objections I’m going to push this…
>>>>>> 
>>>>>> http://hg.openjdk.java.net/graal/graal-jvmci-9/hotspot/rev/6a6766a8cbbf
>>>>>> 
>>>>>>> On Sep 16, 2015, at 3:05 PM, Christian Thalinger <christian.thalinger at oracle.com> wrote:
>>>>>>> 
>>>>>>> I would like to add this change:
>>>>>>> 
>>>>>>> diff -r 2134e08cc132 src/share/vm/utilities/vmError.cpp
>>>>>>> --- a/src/share/vm/utilities/vmError.cpp      Wed Sep 16 14:28:33 2015 -1000
>>>>>>> +++ b/src/share/vm/utilities/vmError.cpp      Wed Sep 16 15:04:02 2015 -1000
>>>>>>> @@ -517,6 +517,10 @@ void VMError::report(outputStream* st) {
>>>>>>>                Abstract_VM_Version::vm_release(),
>>>>>>>                Abstract_VM_Version::vm_info_string(),
>>>>>>>                TieredCompilation ? ", tiered" : "",
>>>>>>> +#if INCLUDE_JVMCI
>>>>>>> +                   EnableJVMCI ? ", jvmci" : "",
>>>>>>> +                   UseJVMCICompiler ? ", jvmci compiler" : "",
>>>>>>> +#endif
>>>>>>>                UseCompressedOops ? ", compressed oops" : "",
>>>>>>>                gc_mode(),
>>>>>>>                Abstract_VM_Version::vm_platform_string()
>>>>>>> 
>>>>>>> It would be useful to see in the crash logs if this experimental feature was turned on.
>>>>>>> 
>>>>>>>> On Sep 16, 2015, at 12:43 PM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:
>>>>>>>> 
>>>>>>>> I updated top and hotspot webrev with latest (build) changes.
>>>>>>>> 
>>>>>>>> Vladimir
>>>>>>>> 
>>>>>>>> On 9/16/15 2:28 PM, Christian Thalinger wrote:
>>>>>>>>> 
>>>>>>>>>> On Sep 16, 2015, at 6:52 AM, Christian Thalinger <christian.thalinger at oracle.com> wrote:
>>>>>>>>>> 
>>>>>>>>>> 
>>>>>>>>>>> On Sep 16, 2015, at 2:57 AM, Magnus Ihse Bursie <magnus.ihse.bursie at oracle.com> wrote:
>>>>>>>>>>> 
>>>>>>>>>>> On 2015-09-14 09:24, Christian Thalinger wrote:
>>>>>>>>>>>> The JEP itself can be found here:
>>>>>>>>>>>> 
>>>>>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8062493 <https://bugs.openjdk.java.net/browse/JDK-8062493>
>>>>>>>>>>>> 
>>>>>>>>>>>> Here are the webrevs:
>>>>>>>>>>>> 
>>>>>>>>>>>> http://cr.openjdk.java.net/~kvn/JVMCI/webrev.top/ <http://cr.openjdk.java.net/~kvn/JVMCI/webrev.top/>
>>>>>>>>>>>> http://cr.openjdk.java.net/~kvn/JVMCI/webrev.hotspot/ <http://cr.openjdk.java.net/~kvn/JVMCI/webrev.hotspot/>
>>>>>>>>>>>> 
>>>>>>>>>>>> The change has already undergone a few iterations of internal review by different people of different teams.  Most comments and suggestions were handled accordingly.  Although there is one open item we agreed we will address after the integration of JEP 243 and that work is captured in RFE:
>>>>>>>>>>>> 
>>>>>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8134994 <https://bugs.openjdk.java.net/browse/JDK-8134994>
>>>>>>>>>>>> 
>>>>>>>>>>>> SQE is still working on the tests and all test tasks can be seen as sub-tasks of the JEP.
>>>>>>>>>>>> 
>>>>>>>>>>>> The integration will happen under the bug number:
>>>>>>>>>>>> 
>>>>>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8136421 <https://bugs.openjdk.java.net/browse/JDK-8136421>
>>>>>>>>>>>> 
>>>>>>>>>>> Hi Christian,
>>>>>>>>>>> 
>>>>>>>>>>> (Adding build-dev since this review includes some major build changes.)
>>>>>>>>>>> 
>>>>>>>>>>> The makefile changes looks good in general to me. I have not reviewed the source code changes.
>>>>>>>>>>> 
>>>>>>>>>>> Some comments:
>>>>>>>>>>> 
>>>>>>>>>>> * modules.xml:
>>>>>>>>>>> Make sure you get sign-off from a Jigsaw developer for modifying this file.
>>>>>>>>>> 
>>>>>>>>>> I’ve asked Alan to take a look.
>>>>>>>>>> 
>>>>>>>>>>> 
>>>>>>>>>>> * hotspot/make/linux/makefiles/gcc.make:
>>>>>>>>>>> Seems unfortunate to have to disable a new warning (undefined-bool-conversion) for newly incorporated code. Is it not possible to fix the code emitting this warning instead?
>>>>>>>>>> 
>>>>>>>>>> We had this question before.  It’s not obvious because you can’t see it in the regular diff views but this is under:
>>>>>>>>>> 
>>>>>>>>>> ifeq ($(USE_CLANG), true)
>>>>>>>>>> 
>>>>>>>>>>> 
>>>>>>>>>>> * make/common/MakeBase.gmk and hotspot/make/gensrc/Gensrc-java.base.gmk:
>>>>>>>>>>> I see a coming merge conflict. In jdk9/dev, there is now a new function that performs the same function as CreatePath, but it's named PathList. (It's a bit unfortunate that this code snippet has bounced around as patches without a definite name.) I assume you are going to push this through a hotspot forest. If the PathList patch reaches the hotspot repo before this, please remove the CreatePath from MakeBase, and change the calls to CreatePath to PathList instead. (I could only find such calls in hotspot/make/gensrc/Gensrc-java.base.gmk). If this patch goes in before that, we'll need to give a heads-up to the integrator about this conflict.
>>>>>>>>>> 
>>>>>>>>>> Thanks for the heads up.
>>>>>>>>> 
>>>>>>>>> Erik sent me a patch to avoid merge conflicts.  I’ve integrated two changesets:
>>>>>>>>> 
>>>>>>>>> http://hg.openjdk.java.net/graal/graal-jvmci-9/rev/ddedccc5c0ab <http://hg.openjdk.java.net/graal/graal-jvmci-9/rev/ddedccc5c0ab><http://hg.openjdk.java.net/graal/graal-jvmci-9/rev/ddedccc5c0ab <http://hg.openjdk.java.net/graal/graal-jvmci-9/rev/ddedccc5c0ab>>
>>>>>>>>> http://hg.openjdk.java.net/graal/graal-jvmci-9/hotspot/rev/fee6b89199c9 <http://hg.openjdk.java.net/graal/graal-jvmci-9/hotspot/rev/fee6b89199c9><http://hg.openjdk.java.net/graal/graal-jvmci-9/hotspot/rev/fee6b89199c9 <http://hg.openjdk.java.net/graal/graal-jvmci-9/hotspot/rev/fee6b89199c9>>
>>>>>>>>> 
>>>>>>>>>> 
>>>>>>>>>>> 
>>>>>>>>>>> Another potential coming merge conflict relates to ListPathsSafely in Gensrc-java.base.gmk. There is currenlty a review out from Erik which modifies the API for ListPathsSafely. If/when it goes in, the call to ListPathsSafely in Gensrc-java.base.gmk will need to be modified (Erik can give advice on how). Depending on timing, this too might hit the integrator rather than your push.
>>>>>>>>>> 
>>>>>>>>>> Ok, thanks.
>>>>>>>>>> 
>>>>>>>>>>> 
>>>>>>>>>>> /Magnus
>>>>>>> 
>>>>>> 
>>> 
>> 
> 



More information about the hotspot-dev mailing list