RFR: JDK-8188312 Use CDS if present when running the Boot JDK during build
Magnus Ihse Bursie
magnus.ihse.bursie at oracle.com
Thu Oct 5 08:59:39 UTC 2017
On 2017-10-04 16:43, Ioi Lam wrote:
>
>
> 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.
My worry here is that -Xshare:auto will not work correctly if
-Xbootclasspath/p is used. Maybe someone can guarantee that this will
work and convince me that it will never fail, but I don't think this
risk is worth the marginal gain.
This was supposed to be a quick and simple patch to get a small, but
useful improvement. It's not worth a lot of investigation or fixes, imho.
/Magnus
>
> - 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