RFR (S) 8184753: Asserts against MinObjectAlignment should avoid integer division

Mikael Gerdin mikael.gerdin at oracle.com
Tue Jul 18 08:44:44 UTC 2017


Hi Aleksey,

On 2017-07-18 10:41, Aleksey Shipilev wrote:
> On 07/18/2017 10:02 AM, Aleksey Shipilev wrote:
>> On 07/18/2017 09:58 AM, Mikael Gerdin wrote:
>>> Hi Aleksey,
>>>
>>> On 2017-07-18 09:51, Aleksey Shipilev wrote:
>>>> Hi Mikael,
>>>>
>>>> On 07/18/2017 09:48 AM, Mikael Gerdin wrote:
>>>>>> Noticed two new asserts in arguments.cpp have excess whitespace, fixed:
>>>>>>      http://cr.openjdk.java.net/~shade/8184753/webrev.02
>>>>>
>>>>> Did you consider using is_object_aligned in the newly created align.hpp?
>>>>>
>>>>> http://hg.openjdk.java.net/jdk10/hs/hotspot/file/5e9c41536bd2/src/share/vm/utilities/align.hpp#l120
>>>>>
>>>>>
>>>>> inline bool is_object_aligned(size_t word_size) {
>>>>>     return is_aligned(word_size, MinObjAlignment);
>>>>> }
>>>>
>>>> I did, but haven't done this for two reasons:
>>>>     a) The asserts are about object _sizes_, not the objects themselves;
>>>>     b) Doing *Mask fits other uses of MinObjAlignment(InBytes)(Mask) around HS;
>>>
>>>
>>> But the is_object_aligned(size_t) function _is_ about object sizes, it just has
>>> a weird naming and a weird overload with the void* variant which asserts that a
>>> pointer is correctly aligned.
>>
>> Ah, unfortunate naming indeed. Let me try to switch to is_object_aligned.
> 
> That seems to work well:
>    http://cr.openjdk.java.net/~shade/8184753/webrev.03/
>    http://cr.openjdk.java.net/~shade/8184753/hotspot.changeset

Looks good to me, I'll sponsor it once we get a second pair of eyes on it.
/Mikael

> 
> hotspot_tier1 seems fine.
> 
> NB: Changed the synopsis for the change.
> 
> Thanks,
> -Aleksey
> 


More information about the hotspot-dev mailing list