From: | Claudio Freire <klaussfreire(at)gmail(dot)com> |
---|---|
To: | Alexey Chernyshov <a(dot)chernyshov(at)postgrespro(dot)ru> |
Cc: | PostgreSQL-Dev <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Fwd: Vacuum: allow usage of more than 1GB of work mem |
Date: | 2017-07-14 01:06:52 |
Message-ID: | CAGTBQpbwhti4vA4dLbiiRhLbHWQeSzD07dByYEtjvXBP74cvYQ@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Wed, Jul 12, 2017 at 1:29 PM, Claudio Freire <klaussfreire(at)gmail(dot)com> wrote:
> On Wed, Jul 12, 2017 at 1:08 PM, Claudio Freire <klaussfreire(at)gmail(dot)com> wrote:
>> On Wed, Jul 12, 2017 at 11:48 AM, Alexey Chernyshov
>> <a(dot)chernyshov(at)postgrespro(dot)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.
Updated versions of the patches attached.
A few runs of the benchmark show no significant difference, as it
should (being all cosmetic changes).
The bigger benchmark will take longer.
Attachment | Content-Type | Size |
---|---|---|
0002-Vacuum-allow-using-more-than-1GB-work-mem-v12.patch | text/x-patch | 25.4 KB |
0003-Vacuum-free-dead-tuples-array-as-early-as-possible-v5.patch | text/x-patch | 2.7 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Chapman Flack | 2017-07-14 01:31:36 | Re: SCRAM auth and Pgpool-II |
Previous Message | Amit Langote | 2017-07-14 00:40:17 | Re: Update description of \d[S+] in \? |