Обсуждение: MemoryContextCreate change in PG 11 how should contexts be created
On December 13th this change to context creation was committed, which broke PostGIS trunk compile against PostgreSQL 11 head. https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=9fa6f00b1308d d10da4eca2f31ccbfc7b35bb461 Ticketed in PostGIS here: https://trac.osgeo.org/postgis/ticket/3946 It's been broken for a couple of months https://trac.osgeo.org/postgis/ticket/3904 with just core dumping but at least it used to compile before December 13th. In going thru the changes I see that MemoryContextCreate was changed to not return a context anymore and to take in pointer to the context that should be returned among other changes. I also notice that MemoryContextCreate seems to be rarely used in PostgreSQL code in places I would expect and instead AllocSetContextCreateExtended is used. So is the preferred approach to not use MemoryContextCreate in extensions and instead for PG < 11 use AllocSetContextCreate and PG >= use AllocSetContextCreateExtended? Sorry if my questions don't make any sense. Still trying to grapple with all these memory context functions and how each is different from the other. For reference, one of the slices of code we have in place that broke looks something like this and we've got I think at least 5 more such instances sprinkled in PostGIS code base. https://git.osgeo.org/gitea/postgis/postgis/src/branch/svn-trunk/libpgcommon /lwgeom_transform.c#L550 MemoryContext PJMemoryContext; POSTGIS_DEBUGF(3, "adding SRID %d with proj4text \"%s\" to query cache at index %d", srid, proj_str, PROJ4Cache->PROJ4SRSCacheCount); PJMemoryContext = MemoryContextCreate(T_AllocSetContext, 8192, &PROJ4SRSCacheContextMethods, PROJ4Cache->PROJ4SRSCacheContext, "PostGIS PROJ4 PJ Memory Context"); Thanks, Regina
On Tue, Dec 19, 2017 at 12:24 AM, Regina Obe <lr@pcorp.us> wrote: > On December 13th this change to context creation was committed, which broke > PostGIS trunk compile against PostgreSQL 11 head. > https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=9fa6f00b1308d > d10da4eca2f31ccbfc7b35bb461 > > Ticketed in PostGIS here: > https://trac.osgeo.org/postgis/ticket/3946 > > > It's been broken for a couple of months > https://trac.osgeo.org/postgis/ticket/3904 with just core dumping but at > least it used to compile before December 13th. > > > In going thru the changes I see that MemoryContextCreate was changed to not > return a context anymore and to take in pointer to the context that should > be returned among other changes. > > I also notice that MemoryContextCreate seems to be rarely used in PostgreSQL > code in places I would expect and instead AllocSetContextCreateExtended is > used. > > So is the preferred approach to not use MemoryContextCreate in extensions > and instead for PG < 11 use AllocSetContextCreate and PG >= use > AllocSetContextCreateExtended? > > Sorry if my questions don't make any sense. Still trying to grapple with > all these memory context functions and how each is different from the other. > > For reference, one of the slices of code we have in place that broke looks > something like this and we've got I think at least 5 more such instances > sprinkled in PostGIS code base. > > https://git.osgeo.org/gitea/postgis/postgis/src/branch/svn-trunk/libpgcommon > /lwgeom_transform.c#L550 > > > > MemoryContext PJMemoryContext; > POSTGIS_DEBUGF(3, "adding SRID %d with proj4text \"%s\" to query > cache at index %d", srid, proj_str, PROJ4Cache->PROJ4SRSCacheCount); > > PJMemoryContext = MemoryContextCreate(T_AllocSetContext, 8192, > &PROJ4SRSCacheContextMethods, > > PROJ4Cache->PROJ4SRSCacheContext, > "PostGIS PROJ4 PJ Memory > Context"); As Regina noted, we're working in https://trac.osgeo.org/postgis/ticket/3946 Our use of MemoryContextCreate is solely in order to get use MemoryContextDelete as a callback so that, at the end of a statement, we can clean up externally allocated memory that we're holding in a cache. If we had some other callback to use for "the statement is complete, you can clean up now", we could avoid all this mucking around with raw MemoryContexts entirely. The MemoryContext trick/hack is very old, perhaps a callback or hook has been added since then that we could make use of?
Paul Ramsey wrote: > Our use of MemoryContextCreate is solely in order to get use > MemoryContextDelete as a callback so that, at the end of a statement, > we can clean up externally allocated memory that we're holding in a > cache. You should not use MemoryContextCreate at all -- it's somewhat of an internal API, as you could guess by looking at the weird arguments that you're forced into passing. Instead, the interface you're supposed to use is AllocSetContextCreate. Just make sure you attach your new context to one which has the right lifetime for your usage -- in your case ISTM the parent should be PortalContext, which makes it go away when the current portal (query) is gone. See src/backend/utils/mmgr/README for more. This applies to all releases, old and new, though recently the API of these memory context creation functions has been refined somewhat. -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Paul Ramsey <pramsey@cleverelephant.ca> writes:
> Our use of MemoryContextCreate is solely in order to get use
> MemoryContextDelete as a callback so that, at the end of a statement,
> we can clean up externally allocated memory that we're holding in a
> cache. If we had some other callback to use for "the statement is
> complete, you can clean up now", we could avoid all this mucking
> around with raw MemoryContexts entirely. The MemoryContext trick/hack
> is very old, perhaps a callback or hook has been added since then that
> we could make use of?
I'm not managing to wrap my head around how you could use
MemoryContextCreate directly, unless you are implementing your own memory
context type, in which case the API changes aren't that difficult to make
I should think.
However, if the need is to free some external resources when a memory
context is destroyed, seems like what you ought to be using is a memory
context reset callback. Look at MemoryContextRegisterResetCallback and
its callers (there are just a couple at the moment, though I'm fooling
with a patch that will add more).
regards, tom lane
On Tue, Dec 19, 2017 at 6:54 AM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote: > Paul Ramsey wrote: > >> Our use of MemoryContextCreate is solely in order to get use >> MemoryContextDelete as a callback so that, at the end of a statement, >> we can clean up externally allocated memory that we're holding in a >> cache. > > You should not use MemoryContextCreate at all -- it's somewhat of an > internal API, as you could guess by looking at the weird arguments that > you're forced into passing. > > Instead, the interface you're supposed to use is AllocSetContextCreate. > Just make sure you attach your new context to one which has the right > lifetime for your usage -- in your case ISTM the parent should be > PortalContext, which makes it go away when the current portal (query) is > gone. Thanks, it looks like PortalContext will serve perfectly. P
On Tue, Dec 19, 2017 at 7:00 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Paul Ramsey <pramsey@cleverelephant.ca> writes: >> Our use of MemoryContextCreate is solely in order to get use >> MemoryContextDelete as a callback so that, at the end of a statement, >> we can clean up externally allocated memory that we're holding in a >> cache. If we had some other callback to use for "the statement is >> complete, you can clean up now", we could avoid all this mucking >> around with raw MemoryContexts entirely. The MemoryContext trick/hack >> is very old, perhaps a callback or hook has been added since then that >> we could make use of? > > However, if the need is to free some external resources when a memory > context is destroyed, seems like what you ought to be using is a memory > context reset callback. Look at MemoryContextRegisterResetCallback and > its callers (there are just a couple at the moment, though I'm fooling > with a patch that will add more). During a query we'll look up a coordinate transformation object, which is an expensive lookup, and want to keep it around for the duration of the query. For extra fun, it's externally allocated, not palloc'ed. When the query ends, we want to clean up the objects associated with that query. Right now the trick is to create our custom memorycontext that has its delete method dedicated to cleaning out entries in our cache. If I'm reading right, using MemoryContextRegisterResetCallback on a AllocSetContext created under our PortalContext should do the trick, with less direct mucking about into the internals of contexts. P
On 12/19/17 10:11 AM, Paul Ramsey wrote: > On Tue, Dec 19, 2017 at 7:00 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> Paul Ramsey <pramsey@cleverelephant.ca> writes: > > If I'm reading right, using MemoryContextRegisterResetCallback on a > AllocSetContext created under our PortalContext should do the trick, > with less direct mucking about into the internals of contexts. Be aware that this function is only available in PostgreSQL >= 9.5. Regards, -- -David david@pgmasters.net