Hi Ioi,
On 5/05/2020 3:43 am, Ioi Lam wrote:
On 4/27/20 7:05 PM, David Holmes wrote:
On 28/04/2020 10:01 am, David Holmes wrote:
On 28/04/2020 9:37 am, Ioi Lam wrote:
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
There's a way to pass properties from the VM that the user can't
override. I'll need to lookup the details.
It should suffice to define SystemProperty that is non-writeable and
internal (for good measure). But it seems we have introduced a bug
since Java 9 whereby a non-writeable property is actually being
overwritten! :(
David
Hi David,
I tried implementing the seed as a system property. However,
ImmutableCollections.<clinit>() is executed very early during JDK
bootstrap (about 500-th bytecode), much earlier than when System.props
is initialized (about 43689-th bytecode). So attempts to read the system
property would give a NullPointerException
Error occurred during initialization of VM
java.lang.ExceptionInInitializerError
at
java.util.ImmutableCollections$Set12.<init>(java.base/ImmutableCollections.java:626)
at java.util.Set.of(java.base/Set.java:468)
at java.lang.Module.<clinit>(java.base/Module.java:251)
Caused by: java.lang.NullPointerException
at java.lang.System.getProperty(java.base/System.java:834)
at
java.util.ImmutableCollections.<clinit>(java.base/ImmutableCollections.java:80)
at
java.util.ImmutableCollections$Set12.<init>(java.base/ImmutableCollections.java:626)
at java.util.Set.of(java.base/Set.java:468)
at java.lang.Module.<clinit>(java.base/Module.java:251)
Because of this, I have to keep using the JVM_GetRandomSeedForCDSDump()
native function.
Okay - thanks for trying the alternative. Initialization is always tricky.
David
Thanks
- Ioi
David
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.v03/
http://cr.openjdk.java.net/~iklam/jdk15/8241071-deterministic-cds-archive.v03.delta/
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.v02/
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
