On Mon, 7 Oct 2024 10:49:39 GMT, Roman Kennke <rken...@openjdk.org> wrote:

>> Did you figure out if the code above is correct w.r.t. 
>> `MinObjectAlignment=16`?
>
> When MinObjectAlignment=16, then that method does nothing anyway:
> 
> 
>   if (MinObjAlignment > 1) {
>     return;
>   }
> 
> 
> 
> I think what it really means to say is
> 
>   if (MinObjAlignment >=  CollectedHeap::min_fill_size()) {
>     return;
>   }
> 
> 
> 
> That's also what the comment says: "The size of the gap (if any) right before 
> dense-prefix-end is MinObjAlignment. Need to fill in the gap only if it's 
> smaller than min-obj-size, and the filler obj will extend to next region."
> 
> If I interpret that correctly, we need to deal with the situation only when 
> MinObjAlignment < min_fill_size, because the filler object would extend to 
> the next region, and we need to adjust the next region and mark-bitmap for 
> that extra word. @albertnetymk might want to confirm.
> 
> I'll move the if (UCOH) block down a little bit to right before if 
> (MinObjAlignment) block.

After re-reading this again I agree with what you're writing. If you make the 
change to use:

if (MinObjAlignment >= CollectedHeap::min_fill_size()) {
 return;
 }


do you even have to check for UCOH in this function?

I also wonder if you could tweak the comment now that this is not true when 
UCOH is on:

// The size of the filler (min-obj-size) is 2 heap words with the default
// MinObjAlignment, since both markword and klass take 1 heap word.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20677#discussion_r1790074043

Reply via email to