RFR: 8253495: CDS generates non-deterministic output [v2]

[Thomas Stuefe]

On Wed, 9 Mar 2022 05:10:44 GMT, Ioi Lam <iklam at openjdk.org> wrote:

>> This patch makes the result of "java -Xshare:dump" deterministic:
>> - Disabled new Java threads from launching. This is harmless. See comments in jvm.cpp
>> - Fixed a problem in hashtable ordering in heapShared.cpp
>> - BasicHashtableEntry has a gap on 64-bit platforms that may contain random bits. Added code to zero it.
>> - Enabled checking of $JAVA_HOME/lib/server/classes.jsa in make/scripts/compare.sh
>> 
>> Note:  $JAVA_HOME/lib/server/classes_ncoops.jsa is still non-deterministic. This will be fixed in [JDK-8282828](https://bugs.openjdk.java.net/browse/JDK-8282828).
>> 
>> Testing under way:
>> - tier1~tier5
>> - Run all *-cmp-baseline jobs 20 times each (linux-aarch64-cmp-baseline, windows-x86-cmp-baseline, .... etc).
>
> Ioi Lam has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Fixed zero build

Hi Ioi,

some questions, comments inline.

Like David in the comments, I am also a bit vague on the usefulness, but I may not know the whole story. Is it to enable repackagers like Debian to check the "reproducable" tickbox on their OpenJDK package? Or is there a practical need for this?

Thanks, Thomas

src/hotspot/share/prims/jvm.cpp line 2887:

> 2885:     return;
> 2886:   }
> 2887: #endif

Should we do this for jni_AttachCurrentThread too?

src/hotspot/share/utilities/hashtable.hpp line 42:

> 40: 
> 41:   LP64_ONLY(unsigned int _gap;)
> 42: 

For 64-bit, you now lose packing potential in the theoretical case the following payload does not have to be aligned to 64 bit. E.g. for T=char, where the whole entry would fit into 8 bytes. Probably does not matter as long as entries are allocated individually from C-heap which is a lot more wasteful anyway. 

For 32-bit, I think you may have the same problem if the payload starts with a uint64_t. Would that not be aligned to a 64-bit boundary too? Whether or not you build on 64-bit?

I think setting the memory, or at least the first 8..16 bytes, of the entry to zero in BasicHashtable<F>::new_entry could be more robust:

(16 bytes in case the payload starts with a long double but that may be overthinking it :)


template <MEMFLAGS F> BasicHashtableEntry<F>* BasicHashtable<F>::new_entry(unsigned int hashValue) {
  char* p = :new (NEW_C_HEAP_ARRAY(char, this->entry_size(), F);
  ::memset(p, 0, MIN2(this->entry_size(), 16)); // needs reproducable
  BasicHashtableEntry<F>* entry = ::new (p) BasicHashtableEntry<F>(hashValue);
  return entry;
}

If you are worried about performance, this may also be controlled by a template parameter, and then you do it just for the system dictionary.

-------------

PR: https://git.openjdk.java.net/jdk/pull/7748