On Sun, Mar 16, 2014 at 10:45 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Robert Haas <robertmhaas@gmail.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
