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:26:53 -0700
Message-ID: <44D0379D.10805@joeconway.com> (raw)
In-Reply-To: <27359.1154474195@sss.pgh.pa.us>
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>
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).
Thoughts?
Thanks,
Joe
Attachments:multi-insert-memleak.r00.diff (text/x-patch, 3.2K) download
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:13:20 -0000
***************
*** 872,877 ****
--- 872,878 ----
foreach(lc, exprlist)
{
Expr *expr = (Expr *) lfirst(lc);
+ Expr *p = expr;
ResTarget *col;
col = (ResTarget *) lfirst(icols);
***************
*** 885,893 ****
--- 886,898 ----
result = lappend(result, expr);
+ if (expr != p)+ pfree(p);+
icols = lnext(icols);
attnos = lnext(attnos);
}
+ list_free(exprlist);
return result;
}
***************
*** 2191,2196 ****
--- 2196,2202 ----
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
--- 2209,2228 ----
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:13:21 -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: <44D0379D.10805@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