From: Joe Conway <mail@joeconway.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-patches@postgreSQL.org
Subject: Re: Values list-of-targetlists patch for comments (was Re:
Date: Tue, 01 Aug 2006 22:52:31 -0700
Message-ID: <44D03D9F.80203@joeconway.com> (raw)
In-Reply-To: <44D0379D.10805@joeconway.com>
References: <44B69778.5050104@dunslane.net>
<50461.84.63.113.55.1152832991.squirrel@pezone.net>
<ECCAAC0A3182895D3D12B0E8@[172.26.14.247]>
<44B7ADFC.90205@dunslane.net>
<44BC0432.3040509@joeconway.com>
<44BC661F.80609@joeconway.com>
<44BCD06C.3060409@calorieking.com>
<44BCF1D4.3010807@dunslane.net>
<5842.1153246741@sss.pgh.pa.us>
<44BD8854.9060506@calorieking.com>
<15212.1153279954@sss.pgh.pa.us>
<44BDB2F5.7080003@joeconway.com>
<44BEA710.6060700@joeconway.com>
<23198.1153367887@sss.pgh.pa.us>
<44BF0382.2010501@joeconway.com>
<23454.1153370214@sss.pgh.pa.us>
<44BF122F.1080200@joeconway.com>
<23927.1153373971@sss.pgh.pa.us>
<44C00E1E.1000405@joeconway.com>
<14047.1153442773@sss.pgh.pa.us>
<44C4451A.4010906@joeconway.com>
<11495.1153757531@sss.pgh.pa.us>
<44C4F625.2050304@joeconway.com>
<12029.1153760969@sss.pgh.pa.us>
<44CC0A66.6070903@joeconway.com>
<16429.1154463351@sss.pgh.pa.us>
<44CFBC78.2020508@joeconway.com>
<27359.1154474195@sss.pgh.pa.us>
<44D0379D.10805@joeconway.com >
Joe Conway wrote:
> Tom Lane wrote:
>
>> Here's what I've got so far. I think there's probably more gold to be
>> mined in terms of reducing runtime memory consumption (I don't like the
>> list_free_deep bit, we should use a context), but functionally it seems
>> complete.
>
> I checked out memory usage, and it had regressed to about 1.4 GB (from
> 730 MB as reported yesterday) for 2 million inserts of 2 integers (i.e.
> with the php script I've been using).
>
> I know you're not too happy with the attached approach to solving this,
> but I'm not sure how creating a memory context is going to help. Part of
> the problem is that the various transformXXX functions sometimes return
> freshly palloc'd memory, and sometimes return the pointer they are given.
>
> Anyway, with the attached diff, the 2 million inserts case is back to
> about 730 MB memory use, and speed is pretty much the same as reported
> yesterday (i.e both memory use and performance better than mysql with
> innodb tables).
Of course it also breaks a bunch of regression tests -- I guess that
just points to the fragility of this approach.
This patch retains the memory consumption savings but doesn't break any
regression tests...
Joe
Attachments:multi-insert-memleak.r01.diff (text/x-patch, 2.9K) download
? src/test/regress/sql/insert.sql.new
Index: src/backend/parser/analyze.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/parser/analyze.c,v
retrieving revision 1.341
diff -c -r1.341 analyze.c
*** src/backend/parser/analyze.c 2 Aug 2006 01:59:46 -0000 1.341
--- src/backend/parser/analyze.c 2 Aug 2006 05:48:18 -0000
***************
*** 888,893 ****
--- 888,894 ----
icols = lnext(icols);
attnos = lnext(attnos);
}
+ list_free(exprlist);
return result;
}
***************
*** 2191,2196 ****
--- 2192,2198 ----
for (i = 0; i < sublist_length; i++)
{
coltypes[i] = select_common_type(coltype_lists[i], "VALUES");
+ list_free(coltype_lists[i]);
}
newExprsLists = NIL;
***************
*** 2203,2216 ****
foreach(lc2, sublist)
{
Node *col = (Node *) lfirst(lc2);
- col = coerce_to_common_type(pstate, col, coltypes[i], "VALUES");- newsublist = lappend(newsublist, col);
i++;
}
newExprsLists = lappend(newExprsLists, newsublist);
}
/*
* Generate the VALUES RTE
--- 2205,2224 ----
foreach(lc2, sublist)
{
Node *col = (Node *) lfirst(lc2);
+ Node *new_col;+ + new_col = coerce_to_common_type(pstate, col, coltypes[i], "VALUES");+ newsublist = lappend(newsublist, new_col);+ if (new_col != col)+ pfree(col);
i++;
}
newExprsLists = lappend(newExprsLists, newsublist);
+ list_free(sublist);
}
+ list_free(exprsLists);
/*
* Generate the VALUES RTE
Index: src/backend/parser/parse_target.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/parser/parse_target.c,v
retrieving revision 1.147
diff -c -r1.147 parse_target.c
*** src/backend/parser/parse_target.c 2 Aug 2006 01:59:47 -0000 1.147
--- src/backend/parser/parse_target.c 2 Aug 2006 05:48:18 -0000
***************
*** 172,177 ****
--- 172,178 ----
foreach(lc, exprlist)
{
Node *e = (Node *) lfirst(lc);
+ Node *p = e;
/*
* Check for "something.*". Depending on the complexity of the
***************
*** 188,193 ****
--- 189,195 ----
result = list_concat(result,
ExpandColumnRefStar(pstate, cref,
false));
+ pfree(e);
continue;
}
}
***************
*** 203,208 ****
--- 205,211 ----
result = list_concat(result,
ExpandIndirectionStar(pstate, ind,
false));
+ pfree(e);
continue;
}
}
***************
*** 210,218 ****
/*
* Not "something.*", so transform as a single expression
*/
! result = lappend(result,
! transformExpr(pstate, e));
}
return result;
}
--- 213,224 ----
/*
* Not "something.*", so transform as a single expression
*/
! p = transformExpr(pstate, e);
! result = lappend(result, p);
! if (e != p)
! pfree(e);
}
+ list_free(exprlist);
return result;
}
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-docs@postgresql.org
Cc: mail@joeconway.com, tgl@sss.pgh.pa.us, pgsql-patches@postgreSQL.org
Subject: Re: Values list-of-targetlists patch for comments (was Re:
In-Reply-To: <44D03D9F.80203@joeconway.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox