Re: Portability issues in shm_mq

On Mon, Mar 17, 2014 at 11:09 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> OK, I tried this out.  The major complication that cropped up was
>> that, if we make the length word always a Size but align the buffer to
>> MAXIMUM_ALIGNOF, then the length word might get split if sizeof(Size)
>> > MAXIMUM_ALIGNOF.
>
> Hmm ... do we support any platforms where that's actually the case?
> It's possible I guess but I don't know of any offhand.  The reverse
> case is real, but I'm not sure about this one.

Man, I don't have a clue what exists.  I certainly wouldn't want to
discourage someone from making a 64-bit machine that only requires
32-bit alignment for 8-bit values, but color me clueless as to whether
such things are really out there.

>> That doesn't look too bad, but required changing a
>> couple of if statements into while loops, and changing around the
>> structure of a shm_mq_handle a bit.  See attached.
>
> Would it get noticeably simpler or faster if you omitted support for
> the sizeof(Size) > MAXIMUM_ALIGNOF case?  It looks like perhaps not,
> but if we were paying anything much I'd be tempted to just put in
> a static assert to the contrary and see if anyone complains.

Not really.  I installed a fast path into the receive code for the
common case where the length word isn't split, which will always be
true on platforms where sizeof(Size) <= MAXIMUM_ALIGNOF and usually
true otherwise.  We could ditch the slow path completely by ignoring
that case, but it's not all that much code.  On the sending side, the
logic is pretty trivial, so I definitely don't feel bad about carrying
that.

The thing I kind of like about this approach is that it makes the code
fully independent of the relationship between MAXIMUM_ALIGNOF and
sizeof(Size).  If the former is smaller, we'll write the length word
in chunks if needed; if the latter is smaller, we'll insert useless
padding bytes.  In the perhaps-common case where they're identical, it
all works as before, except for minor space savings on 32-bit
platforms.  I was a little annoyed by having to write the extra code
and thought about objecting to this whole line of attack yet again,
but I think it's actually likely for the best.  If we start persuading
ourselves that certain cases don't need to work, and rely on that
throughout the backend, and then such machines crop up and we want to
support them, we'll have a deep hole to climb out of.  With this
approach, there might be bugs, of course, but it's a lot easier to fix
a bug that only occurs on a new platform than it is to reconsider the
whole design in light of a new platform.

> BTW, can't we drop the MAXALIGN64 stuff again?

It's still used by xlog.c.

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