Skip site navigation (1) Skip section navigation (2)

Re: Memory leak in FDW

From: Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Memory leak in FDW
Date: 2011-04-27 08:19:32
Message-ID: 4DB7D194.6030906@enterprisedb.com (view raw or flat)
Thread:
Lists: pgsql-hackers
On 26.04.2011 21:30, Tom Lane wrote:
> Heikki Linnakangas<heikki(dot)linnakangas(at)enterprisedb(dot)com>  writes:
>> The trivial fix is to reset the per-tuple memory context between
>> iterations.
>
> Have you tested this with SRFs?
>
> ForeignNext seems like quite the wrong place for resetting
> exprcontext in any case ...

ExecScan would be more appropriate I guess (attached).

This means the context will be reset between each tuple even for nodes 
like seqscan that don't use the per-tuple context for anything. 
AllocSetReset exits quickly if there's nothing to do, but it takes a 
couple of function calls to get there. I wouldn't normally worry about 
that, but this is a very hot path for simple queries.

I tested this with:

CREATE TABLE foo AS SELECT generate_series(1,10000000);

I ran "SELECT COUNT(*) FROM foo" many times with \timing on, and took 
the smallest time with and without the patch. I got:

1230 ms with the patch
1186 ms without the patch

This was quite repeatable, it's consistently faster without the patch. 
That's a bigger difference than I expected. Any random code change can 
swing results on micro-benchmarks like this by 1-2%, but that's over 3%. 
Do we care?

I might be getting a bit carried away with this, but we could buy that 
back by moving the isReset flag from AllocSetContext to 
MemoryContextData. That way MemoryContextReset can exit more quickly if 
there's nothing to do, patch attached.

-- 
   Heikki Linnakangas
   EnterpriseDB   http://www.enterprisedb.com

Attachment: optimize-memorycontextreset-1.patch
Description: text/x-diff (5.0 KB)
Attachment: plug-foreign-scan-memory-leak-2.patch
Description: text/x-diff (944 bytes)

In response to

Responses

pgsql-hackers by date

Next:From: Vlad ArkhipovDate: 2011-04-27 08:35:01
Subject: Predicate locking
Previous:From: Heikki LinnakangasDate: 2011-04-27 07:27:31
Subject: Re: GSoC 2011: Fast GiST index build

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group