From: | Neil Conway <neilc(at)samurai(dot)com> |
---|---|
To: | pgsql-patches <pgsql-patches(at)postgresql(dot)org> |
Cc: | Oleg Bartunov <oleg(at)sai(dot)msu(dot)su> |
Subject: | GiST: memory allocation, cleanup |
Date: | 2004-11-05 05:41:00 |
Message-ID: | 1099633260.10449.100.camel@localhost.localdomain |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-patches |
Attached is a patch that makes some cleanups and improvements to GiST,
as well as a few semi-related cleanups. Changes:
- previously, GiST did not make any use of the backend's memory context
infrastructure. This made implementing indexes using GiST unnecessarily
difficult and fragile: if your implementation of a GiST method leaked
memory, this could result in a potentially large memory leak in
PostgreSQL itself. This patch changes GiST so that all invocations of
used-defined GiST methods are done within a short-lived memory context.
In practice, this means GiST implementors need not use pfree, unless
they are doing a _lot_ of allocations in their method implementations.
As part of this work, memory contexts were also used to cleanup a lot of
ugly and difficult to maintain memory management code in gist.c itself.
QUESTION: given a ScanKey for an index scan, GiST overwrites the
ScanKey's sk_func to point to the user-defined Consistent method
(gistrescan() in gistscan.c). When doing a GiST index scan we can ensure
that the sk_func is invoked in a short-lived memory context
(gistfindnext() in gistget.c). Is it safe to assume that anywhere else
in the backend that invokes the ScanKey's sk_func, it is done in a
short-lived memory context?
- previously, src/include/access/gist.h included both the "public" API
that is intended for use by GiST implementors as well as declarations
for the GiST implementation itself. I've created a new header file
"gist_private.h" and moved the latter declarations there.
- remove some "extern" keywords from function definitions in
contrib/btree_gist, and do some more minor code cleanup there
- minor GiST documentation cleanup
- mark the array that indicates NULLs that is passed to
index_formtuple() as 'const', and fix the resulting fallout
- after doing an index build, there was previously a long comment and a
few lines of code that used the # of index tuples and heap tuples
(computed by the index build) to update the optimizer's statistics. This
code was duplicated 4 times (once for each type of index); I added a
function called IndexUpdateStats() in catalog/index.c and changed each
index's build function to call that function.
- in GiST, move the definition of a bunch of scope-local variables to
the most tightly-enclosing scope. In other words, code like:
{
int a, b;
a = ...;
if (a)
{
b = ...;
}
}
was changed to move the definition of "b" into the inner scope. This is
per good style, IMHO (and it is consistent with the rest of the
PostgreSQL source).
- moved gistfreestack() to gistscan.c and made it "static", since it is
only used there
- cleaned up various other things in gist.c: added some comments,
basically tried to impose some sanity
Comments welcome. I would like to apply this to 8.1 at some point after
we branch for 8.0. I thought I would post this patch to get some
feedback before starting on further GiST work (speculatively, adding
support for concurrency and making it WAL-safe).
-Neil
Attachment | Content-Type | Size |
---|---|---|
gist_work-1.patch.gz | application/x-gzip | 17.1 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Oleg Bartunov | 2004-11-05 09:38:09 | Re: contrib/ sparse code cleanup |
Previous Message | Neil Conway | 2004-11-05 04:21:01 | contrib/ sparse code cleanup |