Misc. consequences of backend memory management changes

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: pgsql-hackers(at)postgreSQL(dot)org
Subject: Misc. consequences of backend memory management changes
Date: 2000-06-28 17:04:27
Message-ID: 16071.962211867@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: 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?

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?

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.

Comments, objections?

regards, tom lane

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Hiroshi Inoue 2000-06-28 17:05:31 RE: AW: Big 7.1 open items
Previous Message Ross J. Reedstrom 2000-06-28 17:03:18 Re: AW: Big 7.1 open items