Re: [HACKERS] PATCH : Generational memory allocator (was PATCH: twoslab-like memory allocators) - Mailing list pgsql-hackers

On 14 August 2017 at 01:35, Tomas Vondra <tomas.vondra@2ndquadrant.com> wrote:
> Hi,
>
> Attached is a rebased version of the Generational context, originally
> submitted with SlabContext (which was already committed into Pg 10).
>
> The main change is that I've abandoned the pattern of defining a Data
> structure and then a pointer typedef, i.e.
>
>     typedef struct GenerationContextData { ... } GenerationContextData;
>     typedef struct GenerationContextData *GenerationContext;
>
> Now it's just
>
>     typedef struct GenerationContext { ... } GenerationContext;
>
> mostly because SlabContext was committed like that, and because Andres was
> complaining about this code pattern ;-)
>
> Otherwise the design is the same as repeatedly discussed before.
>
> To show that this is still valuable change (even after SlabContext and
> adding doubly-linked list to AllocSet), I've repeated the test done by
> Andres in [1] using the test case described in [2], that is
>
>   -- generate data
>   SELECT COUNT(*) FROM (SELECT test1()
>                           FROM generate_series(1, 50000)) foo;
>
>   -- benchmark (measure time and VmPeak)
>   SELECT COUNT(*) FROM (SELECT *
>                           FROM pg_logical_slot_get_changes('test', NULL,
>                                         NULL, 'include-xids', '0')) foo;
>
> with different values passed to the first step (instead of the 50000). The
> VmPeak numbers look like this:
>
>          N           master        patched
>     --------------------------------------
>     100000       1155220 kB      361604 kB
>     200000       2020668 kB      434060 kB
>     300000       2890236 kB      502452 kB
>     400000       3751592 kB      570816 kB
>     500000       4621124 kB      639168 kB
>
> and the timing (on assert-enabled build):
>
>          N           master        patched
>     --------------------------------------
>     100000      1103.182 ms     412.734 ms
>     200000      2216.711 ms     820.438 ms
>     300000      3320.095 ms    1223.576 ms
>     400000      4584.919 ms    1621.261 ms
>     500000      5590.444 ms    2113.820 ms
>
> So it seems it's still a significant improvement, both in terms of memory
> usage and timing. Admittedly, this is a single test, so ideas of other
> useful test cases are welcome.

This all looks good.

What I think this needs is changes to  src/backend/utils/mmgr/README
which decribe the various options that now exist (normal?, slab) and
will exist (generational)

Don't really care about the name, as long as its clear when to use it
and when not to use it.

This description of generational seems wrong: "When the allocated
chunks have similar lifespan, this works very well and is extremely
cheap."
They don't need the same lifespan they just need to be freed in groups
and in the order they were allocated.

For this patch specifically, we need additional comments in
reorderbuffer.c to describe the memory allocation pattern in that
module so that it is clear that the choice of allocator is useful and
appropriate, possibly with details of how that testing was performed,
so it can be re-tested later or tested on a variety of platforms.

Particularly in reorderbuffer, surely we will almost immediately reuse
chunks again, so is it worth issuing free() and then malloc() again
soon after? Does that cause additional overhead we could also avoid?
Could we possibly keep the last/few free'd chunks around rather than
re-malloc?

Seems like we should commit this soon.

-- 
Simon Riggs                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers