]> granicus.if.org Git - postgresql/commit
Improve packing/alignment annotation for ItemPointerData.
authorTom Lane <tgl@sss.pgh.pa.us>
Thu, 21 May 2015 21:21:46 +0000 (17:21 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Thu, 21 May 2015 21:21:46 +0000 (17:21 -0400)
commitd4b538ea367de43b2f2b939621272682417cd290
treed4cfb43337eb33511726e07a3ece9ca5a06279cc
parent160a9aaabf400106232e7e6fce0966ee5fdf84e2
Improve packing/alignment annotation for ItemPointerData.

We want this struct to be exactly a series of 3 int16 words, no more
and no less.  Historically, at least, some ARM compilers preferred to
pad it to 8 bytes unless coerced.  Our old way of doing that was just
to use __attribute__((packed)), but as pointed out by Piotr Stefaniak,
that does too much: it also licenses the compiler to give the struct
only byte-alignment.  We don't want that because it adds access overhead,
possibly quite significant overhead.  According to the GCC manual, what
we want requires also specifying __attribute__((align(2))).  It's not
entirely clear if all the relevant compilers accept this pragma as well,
but we can hope the buildfarm will tell us if not.  We can also add a
static assertion that should fire if the compiler padded the struct.

Since the combination of these pragmas should define exactly what we
want on any compiler that accepts them, let's try using them wherever
we think they exist, not only for __arm__.  (This is likely to expose
that the conditional definitions in c.h are inadequate, but finding
that out would be a good thing.)

The immediate motivation for this is that the current definition of
ExecRowMark allows its curCtid field to be misaligned.  It is not clear
whether there are any other uses of ItemPointerData with a similar hazard.
We could change the definition of ExecRowMark if this doesn't work, but
it would be far better to have a future-proof fix.

Piotr Stefaniak, some further hacking by me
src/backend/storage/page/itemptr.c
src/include/storage/itemptr.h