Hi Erik, Thank you so much for investigating a proper fix for the vm_version.o issue. I will withdraw the build change from my original webrev.
Thanks again for taking over the bug! Jiangli > On May 11, 2018, at 2:33 PM, Erik Joelsson <erik.joels...@oracle.com> wrote: > > Received: from [10.132.185.167] (/10.132.185.167) > by default (Oracle Beehive Gateway v4.0) > with ESMTP ; Fri, 11 May 2018 14:33:20 -0700 > Subject: Re: RFR: 8199807 & 8202738: AppCDS performs overly restrictive path > matching check > To: Jiangli Zhou <jiangli.z...@oracle.com>, > "hotspot-runtime-...@openjdk.java.net runtime" > <hotspot-runtime-...@openjdk.java.net>, > build-dev <build-dev@openjdk.java.net> > References: <b173b2ef-9908-418a-9dcc-ee6d9133d...@oracle.com> > From: Erik Joelsson <erik.joels...@oracle.com> > Organization: Oracle Corporation > Message-ID: <ae4c66a1-ad7d-00f1-ced1-729704d56...@oracle.com> > Date: Fri, 11 May 2018 14:33:20 -0700 > User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:52.0) > Gecko/20100101 Thunderbird/52.7.0 > MIME-Version: 1.0 > In-Reply-To: <b173b2ef-9908-418a-9dcc-ee6d9133d...@oracle.com> > Content-Type: text/plain; charset=utf-8; format=flowed > Content-Transfer-Encoding: 8bit > Content-Language: en-US > > Hello, > > For the build change, it's very undesirable to always have to relink libjvm > on every incremental build. Such a change cannot be accepted. > > I have a counter suggestion, which is still a bit of a hack, but it will > cause vm_version.cpp to be recompiled (almost) every time libjvm.so needs to > be relinked. The drawback is that compiling vm_version.cpp is now bound to > happen absolutely last and so cannot happen in parallel with other > compilations. > > Webrev: http://cr.openjdk.java.net/~erikj/8202738/webrev.01/index.html > > Bug: https://bugs.openjdk.java.net/browse/JDK-8202738 > > /Erik > > >> On 2018-05-10 16:11, Jiangli Zhou wrote: >> Hi, >> >> Please review the following webrev that addresses the issue of copied/moved >> JDK image after generating a CDS archive. Thanks Karen Kinnear and Alan >> Baterman for initiating the investigation & discussions in this area >> (especially the ease of usage). Thanks Ioi for implementing a test case for >> moved JDK (JDK-8202935). >> >> webrev: http://cr.openjdk.java.net/~jiangli/8199807_8202738/webrev.00/ >> RFE: https://bugs.openjdk.java.net/browse/JDK-8199807?filter=14921 >> Bug: https://bugs.openjdk.java.net/browse/JDK-8202738?filter=14921 >> >> The webrev includes the following three main parts: >> >> 1. Reduced check for JDK ‘modules’ image file at runtime. The runtime path >> to the ‘modules’ image is no longer required to the the same as dump time >> path. Runtime performs file size check only for the ‘modules’ image, which >> must match with the dump time ‘modules’ size. Invalidation of an outdated >> archive is achieved by the existing vm_version string check (the archived >> vm_version string must match with the runtime vm_version string). >> >> 2. Boot path check are now performed based on the content of the archive. >> Also added a new test case in BootClassPathMismatch.java and add more >> comments for existing test cases. >> >> 3. Fixed the stale vm_version string issue with incremental build. The issue >> was discovered during the work of 8199807. CDS uses vm_version string as >> part of the runtime validation check for archive. A stale vm_version string >> causes the CDS runtime to mistakenly accept an outdated archive. The fix is >> to make sure vm_version.o is recompiled properly when the library/vm is >> rebuilt. >> >> Tested with hs-tier1-4 and jdk-tier1-2. Tested by relocating the JDK image >> manually after generating an archive. Also tested with Ioi’s test both >> locally and via Mach5. >> >> Thanks, >> Jiangli >> > > Hello, > > For the build change, it's very undesirable to always have to relink libjvm > on every incremental build. Such a change cannot be accepted. > > I have a counter suggestion, which is still a bit of a hack, but it will > cause vm_version.cpp to be recompiled (almost) every time libjvm.so needs to > be relinked. The drawback is that compiling vm_version.cpp is now bound to > happen absolutely last and so cannot happen in parallel with other > compilations. > > Webrev: http://cr.openjdk.java.net/~erikj/8202738/webrev.01/index.html > > Bug: https://bugs.openjdk.java.net/browse/JDK-8202738 > > /Erik > > >> On 2018-05-10 16:11, Jiangli Zhou wrote: >> Hi, >> >> Please review the following webrev that addresses the issue of copied/moved >> JDK image after generating a CDS archive. Thanks Karen Kinnear and Alan >> Baterman for initiating the investigation & discussions in this area >> (especially the ease of usage). Thanks Ioi for implementing a test case for >> moved JDK (JDK-8202935). >> >> webrev: http://cr.openjdk.java.net/~jiangli/8199807_8202738/webrev.00/ >> RFE: https://bugs.openjdk.java.net/browse/JDK-8199807?filter=14921 >> Bug: https://bugs.openjdk.java.net/browse/JDK-8202738?filter=14921 >> >> The webrev includes the following three main parts: >> >> 1. Reduced check for JDK ‘modules’ image file at runtime. The runtime path >> to the ‘modules’ image is no longer required to the the same as dump time >> path. Runtime performs file size check only for the ‘modules’ image, which >> must match with the dump time ‘modules’ size. Invalidation of an outdated >> archive is achieved by the existing vm_version string check (the archived >> vm_version string must match with the runtime vm_version string). >> >> 2. Boot path check are now performed based on the content of the archive. >> Also added a new test case in BootClassPathMismatch.java and add more >> comments for existing test cases. >> >> 3. Fixed the stale vm_version string issue with incremental build. The issue >> was discovered during the work of 8199807. CDS uses vm_version string as >> part of the runtime validation check for archive. A stale vm_version string >> causes the CDS runtime to mistakenly accept an outdated archive. The fix is >> to make sure vm_version.o is recompiled properly when the library/vm is >> rebuilt. >> >> Tested with hs-tier1-4 and jdk-tier1-2. Tested by relocating the JDK image >> manually after generating an archive. Also tested with Ioi’s test both >> locally and via Mach5. >> >> Thanks, >> Jiangli >> >
