Hi David & Jiangli, Thanks for your comments. I thought about using a system property, but the user can also specify it like java -Djdk.xshare.dump.salt=0 MyProgram This would circumvent the calculation of ImmutableCollections.SALT32L for normal execution. I am not sure if this is desirable, or if there's a way to prevent the user from doing that. I'll see what the core lib folks suggest. Updated webrev: http://cr.openjdk.java.net/~iklam/jdk15/8241071-deterministic-cds-archive.v0... http://cr.openjdk.java.net/~iklam/jdk15/8241071-deterministic-cds-archive.v0... More comments below On 4/26/20 11:20 PM, David Holmes wrote:
Hi Ioi,
On 27/04/2020 3:22 pm, Ioi Lam wrote:
https://bugs.openjdk.java.net/browse/JDK-8241071 http://cr.openjdk.java.net/~iklam/jdk15/8241071-deterministic-cds-archive.v0...
The goal is to for "java -Xshare:dump" to produce deterministic contents in the CDS archive that depend only on the contents of $JAVA_HOME/lib/modules.
Problems: [1] Symbols in the CDS archive may have non-deterministic order because Arena allocation is non-deterministic. [2] The contents of the CDS shared heap region may be randomized due to ImmutableCollections.SALT32L.
Fixes: [1] With -Xshare:dump, allocate Symbols from the class space (64-bit only).
I'm not seeing what restricts this to 64-bit only ??
The last version of the patch allocated using MetaspaceObj::ClassType. On 64-bit+compressed classes, this will allocate from the class space. But anyway, as Thomas commented, doing this is problematic.
Will changing the allocation of symbols cause any problems with the amount of "class space" we need?
Per Thomas's comment, I moved the allocation of Symbols in a separate virtual space. Please see my comments to Thomas's e-mail.
See changes in symbol.cpp for details. [2] When running the VM with -Xshare:dump, ImmutableCollections.SALT32L is initialized with a deterministic seed. NOTE: this affects ONLY when the VM is running with the special flag -Xshare:dump to dump the CDS archive. It does NOT affect normal execution of Java programs.
I can't say I like adding in the VM call for this. Another alternative would be a VM defined system property. But I'll let core-libs folk make the call here.
--- I also cleaned up the -Xlog:cds output and print out the CRC of each CDS region, to help diagnose why two CDS archives may have different contents.
Generally seems okay. A couple of minor nits:
src/hotspot/share/oops/symbol.cpp
typo: adresses -> addresses
Fixed
test/hotspot/jtreg/runtime/cds/DeterministicDump.java
Should the test use @driver mode?
Fixed
39 // use of SharedArchiveFile argument.
s/argument/arguments/
40 // methods to form command line to create/use shared archive.
s/line/lines/
s/shared/the shared/
I copied these comments from another test. They are not needed here so I deleted them in the new webrev.
71 if (n0 == 0) {
Shouldn't this check come before the loop at line 63?
Fixed. Thanks - Ioi
Thanks, David -----
Thanks - Ioi