Re: Fwd: [HACKERS] Vacuum: allow usage of more than 1GB of work mem - Mailing list pgsql-hackers

From Claudio Freire
Subject Re: Fwd: [HACKERS] Vacuum: allow usage of more than 1GB of work mem
Date
Msg-id CAGTBQpYJFJYAAVhN4TraEyq7yCpXR+PQLyGpKM+94HzHEpfvPw@mail.gmail.com
Whole thread Raw
In response to Re: Fwd: [HACKERS] Vacuum: allow usage of more than 1GB of work mem  (Claudio Freire <klaussfreire@gmail.com>)
Responses Re: Fwd: [HACKERS] Vacuum: allow usage of more than 1GB of work mem
List pgsql-hackers
On Wed, Jul 12, 2017 at 1:08 PM, Claudio Freire <klaussfreire@gmail.com> wrote:
> On Wed, Jul 12, 2017 at 11:48 AM, Alexey Chernyshov
> <a.chernyshov@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.



pgsql-hackers by date:

Previous
From: Claudio Freire
Date:
Subject: Re: Fwd: [HACKERS] Vacuum: allow usage of more than 1GB of work mem
Next
From: Jeff Janes
Date:
Subject: Re: [HACKERS] why not parallel seq scan for slow functions