Re: Portability issues in shm_mq

On Sun, Mar 16, 2014 at 10:45 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> But I think there's another possible problem here. In order for reads
>> from the buffer not to suffer alignment problems, the chunk size for
>> reads and writes from the buffer needs to be MAXIMUM_ALIGNOF (or some
>> multiple of it). And in order to avoid a great deal of additional and
>> unwarranted complexity, the size of the message word also needs to be
>> MAXIMUM_ALIGNOF (or some multiple of it). So the message word can
>> only be of size 4 if MAXIMUM_ALIGNOF is also 4. IOW, I think your
>> approach is going to run into trouble on any system where
>> sizeof(Size)==4 but MAXIMUM_ALIGNOF==8.
>
> Well, it will result in padding space when you maxalign the length word,
> but I don't see why it wouldn't work; and it would certainly be no less
> efficient than what's there today.

Well, the problem is with this:

/* Write the message length into the buffer. */
if (!mqh->mqh_did_length_word)
{
res = shm_mq_send_bytes(mqh, sizeof(uint64), &nbytes, nowait,
&bytes_written);

If I change nbytes to be of type Size, and the second argument to
sizeof(Size), then it's wrong whenever sizeof(Size) % MAXIMUM_ALIGNOF
!= 0. I could do something like:

union
{
char pad[MAXIMUM_ALIGNOF];
Size val;
} padded_size;

padded_size.val = nbytes;
res = shm_mq_send_bytes(mqh, sizeof(padded_size), &padded_size,
nowait, &bytes_written);

...but that probably *is* less efficient, and it's certainly a lot
uglier; a similar hack will be needed when extracting the length word,
and assertions elsewhere will need adjustment. I wonder if it
wouldn't be better to adjust the external API to use Size just for
consistency but, internally to the module, keep using 8 byte sizes
within the buffer. Really, I think that would boil down to going
through and making sure that we use TYPEALIGN(...,sizeof(uint64))
everywhere instead of MAXALIGN(), which doesn't seem like a big deal.
On the third hand, maybe there are or will be platforms out there
where MAXIMUM_ALIGNOF > 8. If so, it's probably best to bite the
bullet and allow for padding now, so we don't have to monkey with this
again later.

Sorry for belaboring this point, but I want to make sure I only need
to fix this once.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company