Re: [HACKERS] Memory leak in vac_update_relstats ?

2007-07-21 Thread Heikki Linnakangas
Tom Lane wrote: Gregory Stark [EMAIL PROTECTED] writes: It seems like the impact of this is self-limiting though. The worst-case is going to be something which executes an extra pfree for every tuple. Or perhaps one for every expression in a complex query involving lots of expressions. Saving

[HACKERS] Memory leak in vac_update_relstats ?

2007-07-20 Thread Pavan Deolasee
Are we leaking memory in vac_update_relstats ? /* Fetch a copy of the tuple to scribble on */ ctup = SearchSysCacheCopy(RELOID, ObjectIdGetDatum(relid), 0, 0, 0); This copy is not subsequently freed in the function. Thanks, Pavan --

Re: [HACKERS] Memory leak in vac_update_relstats ?

2007-07-20 Thread Heikki Linnakangas
Pavan Deolasee wrote: Are we leaking memory in vac_update_relstats ? /* Fetch a copy of the tuple to scribble on */ ctup = SearchSysCacheCopy(RELOID, ObjectIdGetDatum(relid), 0, 0, 0); This copy is not subsequently freed in the

Re: [HACKERS] Memory leak in vac_update_relstats ?

2007-07-20 Thread NikhilS
Hi, It's palloc'd in the current memory context, so it's not serious. It'll be freed at the end of the transaction, if not before that. That's the beauty of memory contexts; no need to worry about small allocations like that. That's the beauty of memory contexts for small allocations. But

Re: [HACKERS] Memory leak in vac_update_relstats ?

2007-07-20 Thread NikhilS
Hi, That's the beauty of memory contexts for small allocations. But because of the 'convenience' of memory contexts we sometimes tend to not pay attention to doing explicit pfrees. As a general rule I think allocations in TopMemoryContext should be critically examined. I was bitten by this

Re: [HACKERS] Memory leak in vac_update_relstats ?

2007-07-20 Thread Tom Lane
NikhilS [EMAIL PROTECTED] writes: One specific case I want to mention here is hash_create(). For local hash tables if HASH_CONTEXT is not specified, they get created in a context which becomes a direct child of TopMemoryContext. Wouldn't it be a better idea to create the table in

Re: [HACKERS] Memory leak in vac_update_relstats ?

2007-07-20 Thread Pavan Deolasee
On 7/20/07, Heikki Linnakangas [EMAIL PROTECTED] wrote: Pavan Deolasee wrote: Are we leaking memory in vac_update_relstats ? /* Fetch a copy of the tuple to scribble on */ ctup = SearchSysCacheCopy(RELOID, ObjectIdGetDatum(relid),

Re: [HACKERS] Memory leak in vac_update_relstats ?

2007-07-20 Thread Tom Lane
Pavan Deolasee [EMAIL PROTECTED] writes: On 7/20/07, Heikki Linnakangas [EMAIL PROTECTED] wrote: It's palloc'd in the current memory context, so it's not serious. Right. But may be for code completeness, we should add that missing heap_freetuple. Personally I've been thinking of mounting an

Re: [HACKERS] Memory leak in vac_update_relstats ?

2007-07-20 Thread Gregory Stark
Tom Lane [EMAIL PROTECTED] writes: Personally I've been thinking of mounting an effort to get rid of unnecessary pfree's wherever possible. Particularly in user-defined functions, cleaning up at the end is a waste of code space and cycles too, because they're typically called in contexts

Re: [HACKERS] Memory leak in vac_update_relstats ?

2007-07-20 Thread Tom Lane
Gregory Stark [EMAIL PROTECTED] writes: It seems like the impact of this is self-limiting though. The worst-case is going to be something which executes an extra pfree for every tuple. Or perhaps one for every expression in a complex query involving lots of expressions. Saving a few extra

Re: [HACKERS] Memory leak in vac_update_relstats ?

2007-07-20 Thread Andrew Dunstan
Tom Lane wrote: I've actually thought about making short-term memory contexts use a variant MemoryContext type in which pfree was a no-op and palloc was simplified by not worrying at all about recycling space. That sounds like a good idea to me. cheers andrew