On 17 April 2018 at 14:33, Alvaro Herrera <alvhe...@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
recycle_mem_part_prune.patch
Description: Binary data
