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
