> On Sep 17, 2015, at 10:20 PM, Volker Simonis <volker.simo...@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.
That’s odd. Vladimir created the webrevs. Maybe he did something different with the top-level one. > > Regards, > Volker > > > On Thu, Sep 17, 2015 at 10:32 PM, Christian Thalinger > <christian.thalin...@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.thalin...@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.koz...@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.thalin...@oracle.com> wrote: >>>>>> >>>>>> >>>>>>> On Sep 16, 2015, at 2:57 AM, Magnus Ihse Bursie >>>>>>> <magnus.ihse.bur...@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 >>> >>
