Re: [bug fix] Memory leak in dblink

Joe Conway <mail(at)joeconway(dot)com> writes:
> Not the most scientific of tests, but I think a reasonable one:
> ...
> 2.7% performance penalty

I made a slightly different test based on the original example.
This uses generate_series() which is surely faster than a SQL
function, so it shows the problem to a larger degree:

create table t1 as select x from generate_series(1,10000000) x;
vacuum t1;

-- time repeated executions of this:
select count(*) from t1
where exists (select 1 from generate_series(x,x+ (random()*10)::int));

-- and this:
select count(*) from t1
where exists (select 1 from
generate_series(x,x+ (random()*10)::int::text::int));

The first of these test queries doesn't leak memory because the argument
expressions are all pass-by-value; the second one adds some useless text
conversions just to provoke the leak (it eats about 1GB in HEAD).

HEAD, with asserts off:

first query:
Time: 21694.557 ms
Time: 21629.107 ms
Time: 21593.510 ms

second query:
Time: 26698.445 ms
Time: 26699.408 ms
Time: 26751.742 ms

With yesterday's patch:

first query:
Time: 23919.438 ms
Time: 24053.108 ms
Time: 23884.989 ms
... so about 10.7% slower, not good

second query no longer bloats, but:
Time: 29986.850 ms
Time: 29951.179 ms
Time: 29771.533 ms
... almost 12% slower

With the attached patch instead, I get:

first query:
Time: 21017.503 ms
Time: 21113.656 ms
Time: 21107.617 ms
... 2.5% faster??

second query:
Time: 27033.626 ms
Time: 26997.907 ms
Time: 26984.397 ms
... about 1% slower

In the first query, the MemoryContextReset is nearly free since there's
nothing to free (and we've special-cased that path in aset.c). It's
certainly a measurement artifact that it measures out faster, which says
to me that these numbers can only be trusted within a couple percent;
but at least we're not taking much hit in cases where the patch isn't
actually conferring any benefit. For the second query, losing 1% to avoid
memory bloat seems well worthwhile.

Barring objections I'll apply and back-patch this.

regards, tom lane