Re: [PoC] Improve dead tuple storage for lazy vacuum

From: Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
To: John Naylor <john(dot)naylor(at)enterprisedb(dot)com>
Cc: Nathan Bossart <nathandbossart(at)gmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, Matthias van de Meent <boekewurm+postgres(at)gmail(dot)com>, Yura Sokolov <y(dot)sokolov(at)postgrespro(dot)ru>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [PoC] Improve dead tuple storage for lazy vacuum
Date: 2022-12-26 17:13:43
Message-ID: CAD21AoDyJWxmPJ8cZj3Y8P1o6EEjjkbmgho4Jb2ABjfhc19Kkw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Dec 23, 2022 at 8:47 PM John Naylor
<john(dot)naylor(at)enterprisedb(dot)com> wrote:
>
> I wrote:
>
> > - Try templating out the differences between local and shared memory.
>
> Here is a brief progress report before Christmas vacation.

Thanks!

>
> I thought the best way to approach this was to go "inside out", that is, start with the modest goal of reducing duplicated code for v16.
>
> 0001-0005 are copies from v13.
>
> 0006 whacks around the rt_node_insert_inner function to reduce the "surface area" as far as symbols and casts. This includes replacing the goto with an extra "unlikely" branch.
>
> 0007 removes the STRICT pragma for one of our benchmark functions that crept in somewhere -- it should use the default and not just return NULL instantly.
>
> 0008 further whacks around the node-growing code in rt_node_insert_inner to remove casts. When growing the size class within the same kind, we have no need for a "new32" (etc) variable. Also, to keep from getting confused about what an assert build verifies at the end, add a "newnode" variable and assign it to "node" as soon as possible.
>
> 0009 uses the bitmap logic from 0004 for node256 also. There is no performance reason for this, because there is no iteration needed, but it's good for simplicity and consistency.

These 4 patches make sense to me. We can merge them into 0002 patch
and I'll do similar changes for functions for leaf nodes as well.

> 0010 and 0011 template a common implementation for both leaf and inner nodes for searching and inserting.
>
> 0012: While at it, I couldn't resist using this technique to separate out delete from search, which makes sense and might give a small performance boost (at least on less capable hardware). I haven't got to the iteration functions, but they should be straightforward.

Cool!

>
> There is more that could be done here, but I didn't want to get too ahead of myself. For example, it's possible that struct members "children" and "values" are names that don't need to be distinguished. Making them the same would reduce code like
>
> +#ifdef RT_NODE_LEVEL_LEAF
> + n32->values[insertpos] = value;
> +#else
> + n32->children[insertpos] = child;
> +#endif
>
> ...but there could be downsides and I don't want to distract from the goal of dealing with shared memory.

With these patches, some functions in radixtree.h load the header
files, radixtree_xxx_impl.h, that have the function body. What do you
think about how we can expand this template method to deal with DSA
memory? I imagined that we load say radixtree_template.h with some
macros to use the radix tree like we do for simplehash.h. And
radixtree_template.h further loads xxx_impl.h files for some internal
functions.

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2022-12-26 17:47:02 Re: Error-safe user functions
Previous Message Michail Nikolaev 2022-12-26 15:19:35 Re: Data loss on logical replication, 12.12 to 14.5, ALTER SUBSCRIPTION