RFR: 8139457: Array bases are aligned at HeapWord granularity [v23]

Roman Kennke rkennke at openjdk.org
Tue Feb 28 11:26:21 UTC 2023


On Wed, 15 Feb 2023 14:32:45 GMT, Stefan Karlsson <stefank at openjdk.org> wrote:

>> Roman Kennke has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Clarify comment on arrayOopDesc::max_array_length()
>
> src/hotspot/share/gc/shared/jvmFlagConstraintsGC.cpp line 340:
> 
>> 338:     return JVMFlag::VIOLATES_CONSTRAINT;
>> 339:   }
>> 340:   if (value > ((uint64_t)ThreadLocalAllocBuffer::max_size() * HeapWordSize)) {
> 
> Was this change needed because _filler_array_max_size was changed and max_tlab_size uses it? Could you motivate why this is needed or add an assert that we don't overflow size_t.
> 
> I see that we have another place where we don't guard against a size_t overflow:
> 
>   static size_t max_size_in_bytes()              { return max_size() * BytesPerWord; }
> 
> If need to care about overflow in MinTLABSizeConstraintFunc, don't we need to care about it in usages of max_size_in_bytes()?

Using the new filler_array_max_size we can end up exactly at SIZE_MAX+1, which would wrap-around and become 0 and make the comparison fail. So yeah, maybe the new value should be reconsidered to ensure not overflowing when multiplying with BytesPerWord. (I'm wondering if it might be easier to hardcode a table of array-max-sizes depending on elem-type and LP64...)

The only use of max_size_in_bytes() can easily be converted to use words instead of bytes. And I guess that the comparison in the constraint function can be changed to use words as well, so as to avoid the overflow. Would that address your concern?

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

PR: https://git.openjdk.org/jdk/pull/11044


More information about the hotspot-dev mailing list