Re: Patch: Optimize memory allocation in function 'bringetbitmap'

From: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
To: Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Patch: Optimize memory allocation in function 'bringetbitmap'
Date: 2017-04-07 22:18:38
Message-ID: 20170407221838.nsnw4qguzbs75nr6@alvherre.pgsql
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

Tomas Vondra wrote:

> 1) bringetbitmap(PG_FUNCTION_ARGS)
>
> + /*
> + * Estimate the approximate size of btup and allocate memory for btup.
> + */
> + btupInitSize = bdesc->bd_tupdesc->natts * 16;
> + btup = palloc(btupInitSize);
>
> What is the reasoning behind this estimate? I mean, this only accounts for
> the values, not the NULL bitmap or BrinTuple fields. Maybe it does not
> really matter much, but I'd like to see some brief description in the docs,
> please.
>
> This also interacts with our memory allocator in a rather annoying way,
> because palloc() always allocates memory in 2^k chunks, but sadly the
> estimate ignores that. So for natts=3 we get btupInitSize=48, but internally
> allocate 64B (and ignore the top 16B).

I fixed this point (and the following one, which is essentially
complaining about the same thing) by changing the API of
brin_copy_tuple, similarly to the changes we made to brin_deform_tuple:
pass the previously allocated memory (along with its size) as arguments,
so that it can be repalloc'ed if necessary, or just re-used as-is.

That seemed the most effective way to solve the points you raised.

Your review was extremely useful, many thanks.

In a extremely simpleminded test to measure the benefit, I did this:

create table t (a int, b bigint, c bigint) ;
insert into t select g, g, g from generate_series(1, 10000 * 1000) g;
-- generates about 800 MB of data; fits in shared_buffers
create index ti on t using brin (a, b, c) with (pages_per_range = 1);
-- index contains about 84 revmap pages

and then measured this query:
explain analyze select * from t where b between 245782 and 1256782;
with and without this patch. It goes from 40ms without patch to 25ms
with, which seems a decent enough improvement. (Assertions were
enabled, but I disabled memory clobbering).

--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Eisentraut 2017-04-07 22:30:47 Re: monitoring.sgml missing tag
Previous Message Andres Freund 2017-04-07 21:50:58 pgsql: Improve 64bit atomics support.