RFR: 8178489: Make align functions more type safe and consistent

Stefan Karlsson stefan.karlsson at oracle.com
Tue Jul 4 13:33:46 UTC 2017


Hi Mikael,

Thanks for the review.

Inlined:

On 2017-07-04 13:55, Mikael Gerdin wrote:
> Hi Stefan,
> 
> On 2017-06-30 11:16, Stefan Karlsson wrote:
>> Hi all,
>>
>> Please review this patch to make the align functions more type safe 
>> and consistent.
>>
>> http://cr.openjdk.java.net/~stefank/8178489/webrev.00
> 
> I looked through this and I think it looks good but I think you may need 
> to add some extra brackets around the use of the parameters in 
> is_size_aligned_
> 
> change
> #define is_size_aligned_(size, alignment) ((size) == 
> (align_size_up_(size, alignment)))
> 
> to
> #define is_size_aligned_(size, alignment) ((size) == 
> (align_size_up_((size), (alignment))))

This is a pre-existing issue with these macros, I just moved it in this 
file.

I talked to Mikael offline and we agreed on creating a new Bug for this, 
and push after the current set of patches have been pushed. This way I 
won't have to rebase the follow-up patches.

StefanK

> 
> Thanks
> /Mikael
> 
> 
>> https://bugs.openjdk.java.net/browse/JDK-8178489
>>
>> Note that this patch needs to be applied on top of the following 
>> patches that are out for review:
>>   http://cr.openjdk.java.net/~stefank/8178491/webrev.02
>>   http://cr.openjdk.java.net/~stefank/8178495/webrev.01
>>
>> Currently, the align functions forces the user to often explicitly 
>> cast either the input parameters, or the return type, or both.
>>
>> Two examples of the current API:
>> inline intptr_t align_size_up(intptr_t size, intptr_t alignment);
>> inline void* align_ptr_up(const void* ptr, size_t alignment);
>>
>> I propose that we change the API to use templates to return the 
>> aligned value as the same type as the type of the unaligned input.
>>
>> The proposed API would look like this:
>>
>> template <typename T, typename A>
>> inline T align_size_up(T size, A alignment);
>>
>> template <typename T, typename A>
>> inline T* align_ptr_up(T* ptr, A alignment);
>>
>> and a follow-up RFE (JDK-8178499) would get rid of _size_ and _ptr_ 
>> from the names.
>>
>> Usages of these align functions would then look like:
>>
>> size_t aligned_size = align_up(alloc_size, os::vm_page_size())
>> HeapWord* aligned_top = align_up(top, region_size)
>>
>> Please, take an extra close look at the reinterpret_cast I added in 
>> atomic.hpp. This was needed because the align_ptr_down now returns T* 
>> and not void*, and the compiler complained when we tried to do a 
>> static cast from a volatile jbyte* to a volatile jint*.
>>
>> Tested with the align unit test and JPRT.
>>
>> Thanks,
>> StefanK


More information about the hotspot-dev mailing list