RFR: JDK-8188312 Use CDS if present when running the Boot JDK during build
Magnus Ihse Bursie
magnus.ihse.bursie at oracle.com
Wed Oct 4 08:47:48 UTC 2017
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)
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