Message-ID: <44D03D9F.80203@joeconway.com>
Date: Tue, 01 Aug 2006 22:52:31 -0700
From: Joe Conway <mail@joeconway.com>
User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US;
	rv:1.7.13) Gecko/20060501 Fedora/1.7.13-1.1.fc5
MIME-Version: 1.0
To: Tom Lane <tgl@sss.pgh.pa.us>
CC: pgsql-patches@postgreSQL.org
Subject: Re: Values list-of-targetlists patch for comments (was Re: 
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 >
In-Reply-To: <44D0379D.10805@joeconway.com>
Content-Type: multipart/mixed; boundary="------------050806020904010905070601"

This is a multi-part message in MIME format.
--------------050806020904010905070601
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Transfer-Encoding: 7bit

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

--------------050806020904010905070601
Content-Type: text/x-patch;
 name="multi-insert-memleak.r01.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="multi-insert-memleak.r01.diff"

? 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;
  }

--------------050806020904010905070601--