RFR: 8274400: HotSpot Style Guide should permit use of alignof [v3]

[Xin Liu]

On Sun, 22 Jan 2023 21:03:27 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

>>> > What is the use-case for permitting use of `alignof`? Usually we don't add such permits without some use-case in mind.
>>> > [...]
>>> > I'm not opposed to permitting the use of `alignof`, but wondering why we might need it.
>>> 
>>> Responding to myself, I've found a usage category where `alignof` would be appropriate. We have various places where we are aligning a pointer to the size of some type, where it would be more appropriate to align to the alignment of that type. See, for example, `Message::calc_size()` (logging/logAsyncWriter.hpp), which uses `sizeof(void*)` where it seems like it should be using `alignof(Message)`.
>> 
>> Alignment of 'sizeof(void*)' here is a performance hint. We would like to see the address of next Message is aligned of pointer size. It eases the load instruction of 'Message::_output'. It's more effective to load from the aligned address. 
>>  
>> 
>>     static constexpr size_t calc_size(size_t message_len) {
>>       return align_up(sizeof(Message) + message_len + 1, sizeof(void*));
>>     }
>> 
>> 
>> In this context, alignof(Message) is correct but a little bit wasteful.
>
>> > > What is the use-case for permitting use of `alignof`? Usually we don't add such permits without some use-case in mind.
>> > > [...]
>> > > I'm not opposed to permitting the use of `alignof`, but wondering why we might need it.
>> > 
>> > 
>> > Responding to myself, I've found a usage category where `alignof` would be appropriate. We have various places where we are aligning a pointer to the size of some type, where it would be more appropriate to align to the alignment of that type. See, for example, `Message::calc_size()` (logging/logAsyncWriter.hpp), which uses `sizeof(void*)` where it seems like it should be using `alignof(Message)`.
>> 
>> Alignment of 'sizeof(void*)' here is a performance hint. We would like to see the address of next Message is aligned of pointer size. It eases the load instruction of 'Message::_output'. It's more effective to load from the aligned address.
>> 
>> ```
>>     static constexpr size_t calc_size(size_t message_len) {
>>       return align_up(sizeof(Message) + message_len + 1, sizeof(void*));
>>     }
>> ```
>> 
>> In this context, alignof(Message) is correct but a little bit wasteful.
> 
> How is it wasteful?  I think alignof(Message) is exactly what should be used, since the calculation is used
> to determine the positioning of the start of a Message object in memory.  Given the current definition of
> Message the two are probably the same, but that's kind of accidental.

hi, @kimbarrett 

Currently, sizeof(Message) is 64. Message is just a header. The real payload is Message + a '\0'-terminated c-str and pads. 

You are right about the purpose of 'calc_size()', but we don't load 'Message" as an object. We only load its fields individually. eg. *(message->_output). That's why I think it's good enough to align to the pointer size.  

Let's assume the log string is empty ''. calc_size(0) returns 72 now, so the next Message starts at buffer+72.  *_output is still aligned load. If we alignof(Message), it starts with buffer+128. that takes 63 bytes to pad. 

thanks,
--lx

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

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