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 09:01:20 UTC 2017
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. :-(
/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