[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-build-dev
Subject: Re: RFR (XXL): JEP 243: Java-Level JVM Compiler Interface
From: Christian Thalinger <christian.thalinger () oracle ! com>
Date: 2015-09-18 16:58:38
Message-ID: 25D9D308-0FBA-47A6-A30D-D5A672AB1AE3 () oracle ! com
[Download RAW message or body]
> On Sep 17, 2015, at 10:20 PM, Volker Simonis <volker.simonis@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.thalinger@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@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@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@oracle.com> wrote:
> > > > > >
> > > > > > > On Sep 16, 2015, at 2:57 AM, Magnus Ihse Bursie \
> > > > > > > <magnus.ihse.bursie@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
> > >
> >
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic