On Wed, Jul 12, 2017 at 1:08 PM, Claudio Freire <klaussfre...@gmail.com> wrote: > On Wed, Jul 12, 2017 at 11:48 AM, Alexey Chernyshov > <a.chernys...@postgrespro.ru> wrote: >> Thank you for the patch and benchmark results, I have a couple remarks. >> Firstly, padding in DeadTuplesSegment >> >> typedef struct DeadTuplesSegment >> >> { >> >> ItemPointerData last_dead_tuple; /* Copy of the last dead tuple >> (unset >> >> * until the segment is fully >> >> * populated). Keep it first to >> simplify >> >> * binary searches */ >> >> unsigned short padding; /* Align dt_tids to 32-bits, >> >> * sizeof(ItemPointerData) is aligned to >> >> * short, so add a padding short, to make >> the >> >> * size of DeadTuplesSegment a multiple of >> >> * 32-bits and align integer components for >> >> * better performance during lookups into >> the >> >> * multiarray */ >> >> int num_dead_tuples; /* # of entries in the segment */ >> >> int max_dead_tuples; /* # of entries allocated in the >> segment */ >> >> ItemPointer dt_tids; /* Array of dead tuples */ >> >> } DeadTuplesSegment; >> >> In the comments to ItemPointerData is written that it is 6 bytes long, but >> can be padded to 8 bytes by some compilers, so if we add padding in a >> current way, there is no guaranty that it will be done as it is expected. >> The other way to do it with pg_attribute_alligned. But in my opinion, there >> is no need to do it manually, because the compiler will do this optimization >> itself. > > I'll look into it. But my experience is that compilers won't align > struct size like this, only attributes, and this attribute is composed > of 16-bit attributes so it doesn't get aligned by default.
Doing sizeof(DeadTuplesSegment) suggests you were indeed right, at least in GCC. I'll remove the padding. Seems I just got the wrong impression at some point. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers