RFR: JDK-8188312 Use CDS if present when running the Boot JDK during build
Ioi Lam
ioi.lam at oracle.com
Wed Oct 4 14:43:35 UTC 2017
On 10/4/17 2:01 AM, Magnus Ihse Bursie wrote:
> On 2017-10-04 10:47, Magnus Ihse Bursie wrote:
>> On 2017-10-04 02:36, Ioi Lam wrote:
>>> If you use SharedArchiveFile, you should set -XX:-VerifySharedSpaces
>>> at the same time.
>>>
>>>
>>> Long story short -- for security, we don't want bad archives to be
>>> mapped into the JVM.
>>>
>>> If you don't specify SharedArchiveFile, the archive is loaded form
>>> the JDK installation directory, and we trust that this location is
>>> not tempered (or else your .so files, etc, could also be tempered,
>>> and the CDS archive is the least of your problem ...). So no need to
>>> verify. (VerifySharedSpaces is set to 0 during JVM start-up).
>>>
>>> However, if you specify SharedArchiveFile, it could point to an less
>>> secured location that could be tempered (e.g., /tmp). As a result,
>>> we picked the "safe default" of -XX:+VerifySharedSpaces, which means
>>> the archive is checksum'ed prior to loading.
>>>
>>> In the case of the JDK build process, we are directly executing
>>> binaries from the build output directory anyway (temporary copies of
>>> the "java" executable, for example). So we have historically
>>> (implicitly) fully trusted the safety of the build output directory.
>>> So it will should be safe to use -XX:-VerifySharedSpaces so we can
>>> squeeze out a few seconds.
>>>
>>> This option has been available since JDK 8u40. So it should be safe
>>> to use it now. There's no need to wait until we switch to using JDK
>>> 9 as the boot jdk.
>>
>> Ok. So if we have -XX:-VerifySharedSpaces we could use this solution,
>> since it includes all benefits (local control of CDS archive, no
>> overhead due to verification). And if we don't, we can always use
>> -Xshare:auto, if the user has installed a system-wide classes.jsa
>> herself.
>>
>> Updated patch that takes all this into consideration: (third time's a
>> charm)
>
> .... or maybe not. It turned out that this build failed, and with good
> reasons. TL;DR: I don't want to enable CDS for building. It's too risky.
>
> Long story: In this patch, I turned on -Xshare:on to force the use of
> CDS if we have a local jsa archive. However, this fails with:
>
> Error occurred during initialization of VM
> Unable to use shared archive.
> ass paths mismatch (hint: enable -XX:+TraceClassPaths to diagnose the
> failure)
>
> (Note the no-pun-intended (?) incorrect error message. Somebody should
> probably file a bug about that.)
>
> The reason for this is that for certain parts of the build, we're
> using a hybrid model where we use the old JDK (the bootjdk) to execute
> code, but use -Xbootclasspath/p to point to our interim classes based
> on the current source code. That means core classes that are present
> in the CDS archive will be replaced.
>
> The only reason this worked at all in my prior patch was that, by pure
> luck, the "ass" paths mismatched, so -Xshare:auto just dismissed it
> instead of using it. Otherwise, we would have used the old classes
> from the CDS archive instead.
>
> This all just seems to risky and prone to breakage, for little gain.
> We could try to apply it just in those cases were we don't override
> the bootclasspath, but then we need to add overhead to the makefiles
> to separate those cases. It's just not worth it.
>
> I will drop this patch altogether and close the bug as WNF. It was a
> good idea but it didn't fly. :-(
How often is -Xbootclasspath/p used?
Why not use "-XX:-VerifySharedSpaces -XX:SharedArchiveFile=local.jsa
-Xshare:auto"? That way you will have the start-up benefit if possible.
- Ioi
>
> /Magnus
>
>
>> http://cr.openjdk.java.net/~ihse/JDK-8188312-use-CDS-for-bootjdk/webrev.03
>>
>>
>> I'm currently running some benchmarking in this to confirm that it is
>> useful for jdk8u40. In any case, we're likely to switch to jdk9 as
>> boot jdk not too soon I hope, so it will at least be relevant by then
>> according to Erik's measurements.
>>
>> /Magnus
>>
>>>
>>>
>>> product(bool, VerifySharedSpaces,
>>> false, \
>>> "Verify shared spaces (false for default archive, true for
>>> " \
>>> "archive specified by
>>> -XX:SharedArchiveFile)") \
>>> \
>>>
>>>
>>> Thanks
>>>
>>> - Ioi
>>>
>>>
>>>
>>>
>>> On 10/3/17 1:37 PM, Magnus Ihse Bursie wrote:
>>>> It might be the case that we should not really continue with this
>>>> as long as we have JDK 8 as default boot jdk.
>>>>
>>>> If so, we should at least add a -Xshare:auto, which will allow the
>>>> user to manually add a system-wide CDS archive to their boot jdk,
>>>> and benefit from it.
>>>>
>>>> And then we'll revisit this when we switch to JDK 9, to see if
>>>> "-XX:-VerifySharedSpaces -XX:SharedArchiveFile=x" is worth the effort.
>>>>
>>>> Sounds reasonable?
>>>>
>>>> /Magnus
>>>>
>>>>
>>>> On 2017-10-03 16:45, Erik Joelsson wrote:
>>>>> The change looks good, but unfortunately I don't see a lot of
>>>>> gain. First of all it seems the -XX:SharedArchiveFile option
>>>>> removes a lot of the performance gain. Here are some rough numbers
>>>>> from my machine. All of them are "time make images" with a clean
>>>>> build directory, on a 16 core/32 threads linux box. In some cases
>>>>> I did two runs, which further shows the amount of variance in the
>>>>> build time.
>>>>>
>>>>> Baseline: boot jdk 8, no cds
>>>>> real 3m14.688s
>>>>> user 38m34.132s
>>>>> sys 5m4.032s
>>>>>
>>>>> real 3m11.086s
>>>>> user 38m16.820s
>>>>> sys 4m35.344s
>>>>>
>>>>> Magnus second patch: boot jdk 8, cds SharedArchiveFile
>>>>> real 3m6.268s
>>>>> user 38m9.168s
>>>>> sys 4m13.392s
>>>>>
>>>>> real 3m15.985s
>>>>> user 37m55.328s
>>>>> sys 3m50.140s
>>>>>
>>>>> Magnus first patch: boot jdk 8, cds in classes.jsa
>>>>> real 2m55.972s
>>>>> user 37m59.636s
>>>>> sys 4m1.020s
>>>>>
>>>>> Baseline with boot jdk 9, no cds
>>>>> real 3m14.262s
>>>>> user 40m9.216s
>>>>> sys 5m5.480s
>>>>>
>>>>> Magnus first patch: boot jdk 9, cds in classes.jsa
>>>>> real 3m12.721s
>>>>> user 39m29.332s
>>>>> sys 4m25.472s
>>>>>
>>>>> Magnus second patch: boot jdk 9, -XX:SharedArchiveFile,
>>>>> -XX:-VerifySharedSpaces
>>>>> real 3m4.297s
>>>>> user 39m24.556s
>>>>> sys 4m31.980s
>>>>>
>>>>> So from this (certainly limited) data set, the only configuration
>>>>> that shows meaningful improvement is with boot jdk 8 and the first
>>>>> patch, where we stealth add classes.jsa to the boot jdk. I find
>>>>> this a very bad thing to do in principle so would vote against
>>>>> such a solution anyway. Also note that we will switch default boot
>>>>> from 8 to 9 very soon, so it's really the results on 9 that are
>>>>> relevant here. In 9, there is another option,
>>>>> -XX:-VerifySharedSpaces which removes most of the overhead added
>>>>> by -XX:SharedArchiveFile. Perhaps using that this becomes worth
>>>>> while.
>>>>>
>>>>> I would like to see more data showing the optimization is actually
>>>>> relevant. On my machine it seems to make a measurable difference,
>>>>> but not a very significant one.
>>>>>
>>>>> One note on the proposed change. While working on this, when I
>>>>> changed my boot jdk, configure did not regenerate the classes.jsa
>>>>> file. I believe for safety we should regenerate on every configure
>>>>> run. It's not that expensive anyway.
>>>>>
>>>>> /Erik
>>>>>
>>>>>
>>>>> On 2017-10-03 14:55, David Holmes wrote:
>>>>>> Looks good to me.
>>>>>>
>>>>>> Thanks,
>>>>>> David
>>>>>>
>>>>>> On 3/10/2017 10:47 PM, Magnus Ihse Bursie wrote:
>>>>>>> On 2017-10-03 14:21, David Holmes wrote:
>>>>>>>> Erik J. raises a good point in the bug report that
>>>>>>>> -XX:SharedArchiveFile=xxx should be used if we create the
>>>>>>>> archive. The build system has no business creating an archive
>>>>>>>> inside the boot JDK.
>>>>>>>
>>>>>>> Agree, that is a better solution. I was not aware of the
>>>>>>> -XX:SharedArchiveFile option.
>>>>>>>
>>>>>>> Here's an updated webrev:
>>>>>>> http://cr.openjdk.java.net/~ihse/JDK-8188312-use-CDS-for-bootjdk/webrev.02
>>>>>>>
>>>>>>>
>>>>>>> I create the jsa file in configure-support, that way it can
>>>>>>> survive a "make clean".
>>>>>>>
>>>>>>> /Magnus
>>>>>>>
>>>>>>>>
>>>>>>>> David
>>>>>>>>
>>>>>>>> On 3/10/2017 9:02 PM, David Holmes wrote:
>>>>>>>>> Hi Claes,
>>>>>>>>>
>>>>>>>>> On 3/10/2017 8:48 PM, Claes Redestad wrote:
>>>>>>>>>> Hi,
>>>>>>>>>>
>>>>>>>>>> -Xshare:auto silently ignores failures to map the shared
>>>>>>>>>> archive and should be safe to use. I think you're thinking of
>>>>>>>>>> -Xshare:on which will fail/abort the VM if mapping fails.
>>>>>>>>>
>>>>>>>>> Ah okay.
>>>>>>>>>
>>>>>>>>> In that case seems reasonable. But please test thoroughly
>>>>>>>>> across all platforms in JPRT.
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> David
>>>>>>>>>
>>>>>>>>>> /Claes
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On 2017-10-03 12:28, David Holmes wrote:
>>>>>>>>>>> Hi Magnus,
>>>>>>>>>>>
>>>>>>>>>>> As I just put in the bug report, it isn't quite this simple.
>>>>>>>>>>> You have to be able to tolerate/recover from failure to map
>>>>>>>>>>> the shared archive.
>>>>>>>>>>>
>>>>>>>>>>> Cheers,
>>>>>>>>>>> David
>>>>>>>>>>>
>>>>>>>>>>> On 3/10/2017 8:24 PM, Magnus Ihse Bursie wrote:
>>>>>>>>>>>> We should use CDS to minimize Java startup time during
>>>>>>>>>>>> build. We run multiple Java commands, and every second counts.
>>>>>>>>>>>>
>>>>>>>>>>>> On my machine, I get a ~3% build time speedup with this fix.
>>>>>>>>>>>>
>>>>>>>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8188312
>>>>>>>>>>>> WebRev:
>>>>>>>>>>>> http://cr.openjdk.java.net/~ihse/JDK-8188312-use-CDS-for-bootjdk/webrev.01
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> /Magnus
>>>>>>>>>>
>>>>>>>
>>>>>
>>>>
>>>
>>
>
More information about the build-dev
mailing list