On Wed, 9 Mar 2022 07:47:19 GMT, Thomas Stuefe <stu...@openjdk.org> wrote:
>> Ioi Lam has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Fixed zero build
>
> 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.
Thanks for pointing this out. I ran more tests and found that on certain
platforms, there are other structures that have problems with uninitialized
gaps. I ended up changing `os::malloc()` to zero the buffer when running with
-Xshare:dump. Hopefully one extra check of `if (DumpSharedSpaces)` doesn't
matter too much for regular VM executions because `os::malloc()` already has a
high overhead.
-------------
PR: https://git.openjdk.java.net/jdk/pull/7748
