On Wed, 30 Oct 2024 23:14:53 GMT, Patricio Chilano Mateo
<pchilanom...@openjdk.org> wrote:
>> This might confuse the change for JEP 450 since with CompactObjectHeaders
>> there's no klass_gap, so depending on which change goes first, there will be
>> conditional code here. Good question though, it looks like we only ever want
>> to copy the payload of the object.
>
> If I recall correctly this was a bug where one of the stackChunk fields was
> allocated in that gap, but since we didn't zeroed it out it would start with
> some invalid value. I guess the reason why we are not hitting this today is
> because one of the fields we do initialize (sp/bottom/size) is being
> allocated there, but with the new fields I added to stackChunk that is not
> the case anymore.
This code in `StackChunkAllocator::initialize` mimics the clearing code in:
void MemAllocator::mem_clear(HeapWord* mem) const {
assert(mem != nullptr, "cannot initialize null object");
const size_t hs = oopDesc::header_size();
assert(_word_size >= hs, "unexpected object size");
oopDesc::set_klass_gap(mem, 0);
Copy::fill_to_aligned_words(mem + hs, _word_size - hs);
}
but with a limited amount of clearing at the end of the object, IIRC. So, this
looks like a good fix. With JEP 450 we have added an assert to set_klass_gap
and changed the code in `mem_clear` to be:
if (oopDesc::has_klass_gap()) {
oopDesc::set_klass_gap(mem, 0);
}
So, unchanged, this code will start to assert when the to projects merge. Maybe
it would be nice to make a small/trivial upstream PR to add this code to both
`MemAllocator::mem_clear` and `StackChunkAllocator::initialize`?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1827424227
