Re: Misc. consequences of backend memory management changes
От | Bruce Momjian |
---|---|
Тема | Re: Misc. consequences of backend memory management changes |
Дата | |
Msg-id | 200006281727.NAA17478@candle.pha.pa.us обсуждение исходный текст |
Ответ на | Misc. consequences of backend memory management changes (Tom Lane <tgl@sss.pgh.pa.us>) |
Список | pgsql-hackers |
> I'm about halfway done with revising backend memory management per > the discussions on pghackers around the end of April (see the > just-committed src/backend/utils/mmgr/README for latest version > of the proposal). There are several things that weren't previously > discussed that are now looking like good ideas. I don't think > these'll be too controversial, but I wanted to mention them in case > anyone gets unhappy: > > 1. src/backend/nodes/freefuncs.c is currently dead code; it's not > called from anywhere. I had proposed reviving it to use for freeing > rule trees that are stored in relcache entries (currently, a relcache > flush leaks storage for any rules associated with flushed entries) > but Hiroshi and others objected that freeObject() is inherently unsafe > since it can't cope with nodetrees having multiple links to the same > node --- and we do create such trees in places. We now have a much > better, safer, and faster mechanism for getting rid of arbitrary node > trees: construct them in a private memory context that you can delete > (or just reset) when you don't want the tree anymore. Therefore I'm > now persuaded that freefuncs.c never will be anything but dead code. > I propose removing it entirely, so that people won't feel they have > to maintain it when adding/revising/deleting node types. Any > objections? Sure. > > 2. Currently there is some code in aset.c that arranges to wipe > freed space immediately upon its being freed, as an aid in detecting > attempted uses of already-freed objects. It's conditionally compiled > based on "#ifdef CLOBBER_FREED_MEMORY", which ought to be mentioned > in config.h.in (but isn't yet). This slows things down a little bit > so I wouldn't recommend having it turned on for production use, but > I think it would be a good idea to keep it turned on during the > development and early beta phases of 7.1. With the memory > management changes I think we will be at increased risk of having > use-of-freed-memory bugs, so I'd like to get as much testing as > possible done with this code enabled. Will anyone object if I make > CLOBBER_FREED_MEMORY defined by default until 7.1 release is near? Why not have it enabled when cassert is enabled? > > 3. The portal ("DECLARE CURSOR") code is now critically dependent on > being able to copy parse and plan trees with copyObject(). Formerly it > didn't copy them, instead using a hack that involved relabeling the > working "blank" portal as a permanent portal so that its contents > wouldn't go away at end of statement. That won't work anymore (blank > portals are history) so the constructed trees have to be explicitly > copied into the memory context that's created for the cursor portal. > The reason I bring this up is that we've had bugs in the past with some > node types or tree structures not being understood or properly copied by > copyObject(). Such a bug would now manifest itself as a failure > occurring only when some particular feature is used in a DECLARE CURSOR. > To help catch such problems sooner, it'd be a good idea to have > conditionally-compiled test code that forces *every* generated parse and > plan tree to be passed through copyObject(). Again, I propose setting > up config.h symbols for this and turning them on by default during the > 7.1 development phase. Same, why not use cassert, or maybe change cassert to debug and all developers will enable it? -- Bruce Momjian | http://www.op.net/~candle pgman@candle.pha.pa.us | (610) 853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania19026
В списке pgsql-hackers по дате отправления: