On 17 April 2018 at 14:33, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
> David Rowley wrote:
>
>> For a while, during my review of the faster partition pruning patch I
>> was asking Amit to add pfree() calls in various places for this exact
>> reason, but in the end, I gave up and decided it was easier to just
>> create a new memory context to call the planner function from. I've
>> now forgotten the exact reason why I finally decided it was too much
>> trouble. The pruning code now works using your step logic so perhaps
>> that reason no longer applies, although, on a quick scan of the
>> pruning code now, it seems to require that get_matching_partitions
>> performs a deep pfree of each PruneStepResult. However, there is still
>> partkey_datum_from_expr which performs ExecInitExpr, although perhaps
>> that can just be done once, and the result stashed in the
>> PartitionPruneContext.
>
> I think trying to replace a well-placed MemoryContextReset (or Delete)
> with a bunch of individual pfrees is pointless.
I agree. I think I'd sleep better at night with the context reset in
there rather than hoping we've managed to pfree everything.
I did go and start working on a patch to test how possible this would
be and came up with the attached. I've left a stray
MemoryContextStatsDetail call in there which does indicate that
something is not being freed. I'm just not sure what it is yet.
The patch does happen to improve performance slightly, but that is
most likely due to the caching of the ExprStates rather than the
change of memory management. It's not really possible to do that with
the reset unless we stored the executor's memory context in
PartitionPruneContext and did a context switch back inside
partkey_datum_from_expr before calling ExecInitExpr.
My test case was as follows:
create table p (a int, value int) partition by hash (a);
select 'create table p'||x|| ' partition of p for values with (modulus
10, remainder '||x||');' from generate_series(0,9) x;
\gexec
create table t1 (a int);
insert into p select x,x from generate_Series(1,10000000) x;
insert into t1 select x from generate_series(1,10000000) x;
create index on p(a);
set enable_hashjoin = 0;
set enable_mergejoin = 0;
explain analyze select count(*) from t1 inner join p on t1.a=p.a;
-- Unpatched
Execution Time: 19725.981 ms
Execution Time: 19533.655 ms
Execution Time: 19542.854 ms
-- Patched
Execution Time: 17389.537 ms
Execution Time: 17603.802 ms
Execution Time: 17618.670 ms
--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services