MemoryContextCreate change in PG 11 how should contexts be created

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