RFR: JDK-8188312 Use CDS if present when running the Boot JDK during build

Wed Oct 4 00:36:08 UTC 2017

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.

   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
>>>>>>>
>>>>
>>
>