[postgis-tickets] r16167 - Make PostgreSQL 11 compile against PostGIS 2.5 (trunk) and change PostgreSQL 9.6+ to use built-in PostgreSQL memory context (CacheMemoryContext for now) instead of our own custom one which is too prone to version change breaks.

Regina Obe lr at pcorp.us
Wed Dec 20 07:20:23 PST 2017


Author: robe
Date: 2017-12-20 07:20:22 -0800 (Wed, 20 Dec 2017)
New Revision: 16167

Modified:
   trunk/libpgcommon/lwgeom_transform.c
   trunk/postgis/lwgeom_geos_prepared.c
Log:
Make PostgreSQL 11 compile against  PostGIS 2.5 (trunk) and change PostgreSQL 9.6+ to use built-in PostgreSQL memory context (CacheMemoryContext for now) instead of our own custom one which is too prone to version change breaks.
(still need to patch address standardizer)
References #3946 

Modified: trunk/libpgcommon/lwgeom_transform.c
===================================================================
--- trunk/libpgcommon/lwgeom_transform.c	2017-12-20 14:30:19 UTC (rev 16166)
+++ trunk/libpgcommon/lwgeom_transform.c	2017-12-20 15:20:22 UTC (rev 16167)
@@ -64,6 +64,9 @@
 {
 	MemoryContext ProjectionContext;
 	projPJ projection;
+#if POSTGIS_PGSQL_VERSION >= 96
+	MemoryContextCallback callback; /* for releasing hashentry when done */
+#endif
 }
 PJHashEntry;
 
@@ -87,24 +90,24 @@
 static bool IsPROJ4LibPathSet = false;
 void SetPROJ4LibPath(void);
 
-/* Memory context cache functions */
+#if POSTGIS_PGSQL_VERSION < 96
+static void PROJ4SRSCacheDelete(MemoryContext context);
+#else
+static void PROJ4SRSCacheDelete(void *arg);
+#endif
+/* Memory context cache function prototypes
+Only need for PostgreSQL where we will not be using a built-in memory context*/
+#if POSTGIS_PGSQL_VERSION < 96
 static void PROJ4SRSCacheInit(MemoryContext context);
-static void PROJ4SRSCacheDelete(MemoryContext context);
 static void PROJ4SRSCacheReset(MemoryContext context);
 static bool PROJ4SRSCacheIsEmpty(MemoryContext context);
 
-#if POSTGIS_PGSQL_VERSION >= 96
-static void PROJ4SRSCacheStats(MemoryContext context, int level, bool print, MemoryContextCounters *totals);
-#else
 static void PROJ4SRSCacheStats(MemoryContext context, int level);
-#endif
 
 #ifdef MEMORY_CONTEXT_CHECKING
 static void PROJ4SRSCacheCheck(MemoryContext context);
 #endif
 
-
-/* Memory context definition must match the current version of PostgreSQL */
 static MemoryContextMethods PROJ4SRSCacheContextMethods =
 {
 	NULL,
@@ -121,7 +124,6 @@
 #endif
 };
 
-
 static void
 PROJ4SRSCacheInit(MemoryContext context)
 {
@@ -132,26 +134,6 @@
 }
 
 static void
-PROJ4SRSCacheDelete(MemoryContext context)
-{
-	projPJ projection;
-
-	/* Lookup the projPJ pointer in the global hash table so we can free it */
-	projection = GetPJHashEntry(context);
-
-	if (!projection)
-		elog(ERROR, "PROJ4SRSCacheDelete: Trying to delete non-existant projection object with MemoryContext key (%p)", (void *)context);
-
-	POSTGIS_DEBUGF(3, "deleting projection object (%p) with MemoryContext key (%p)", projection, context);
-
-	/* Free it */
-	pj_free(projection);
-
-	/* Remove the hash entry as it is no longer needed */
-	DeletePJHashEntry(context);
-}
-
-static void
 PROJ4SRSCacheReset(MemoryContext context)
 {
 	/*
@@ -196,7 +178,39 @@
 }
 #endif
 
+#endif
 
+
+static void
+#if POSTGIS_PGSQL_VERSION < 96
+PROJ4SRSCacheDelete(MemoryContext context)
+{
+	projPJ projection;
+	/* Lookup the projPJ pointer in the global hash table so we can free it */
+	projection = GetPJHashEntry(context);
+#else  /** to use built-in memory context we need to provide our delete hook in a form suitable for callback **/
+PROJ4SRSCacheDelete(void *arg)
+{
+	PJHashEntry *he = (PJHashEntry *) arg;
+	projPJ projection = he->projection;
+	MemoryContext context;
+	if (projection)
+		context = he->ProjectionContext;
+
+#endif
+	if (!projection)
+		elog(ERROR, "PROJ4SRSCacheDelete: Trying to delete non-existant projection object with MemoryContext key (%p)", (void *)context);
+
+	POSTGIS_DEBUGF(3, "deleting projection object (%p) with MemoryContext key (%p)", projection, context);
+
+	/* Free it */
+	pj_free(projection);
+
+	/* Remove the hash entry as it is no longer needed */
+	DeletePJHashEntry(context);
+}
+
+
 /*
  * PROJ4 projPJ Hash Table functions
  */
@@ -546,12 +560,21 @@
 	 * store it in the backend hash
 	 */
 	POSTGIS_DEBUGF(3, "adding SRID %d with proj4text \"%s\" to query cache at index %d", srid, proj_str, PROJ4Cache->PROJ4SRSCacheCount);
-
+#if POSTGIS_PGSQL_VERSION < 96
 	PJMemoryContext = MemoryContextCreate(T_AllocSetContext, 8192,
-	                                      &PROJ4SRSCacheContextMethods,
-	                                      PROJ4Cache->PROJ4SRSCacheContext,
-	                                      "PostGIS PROJ4 PJ Memory Context");
+											&PROJ4SRSCacheContextMethods,
+											PROJ4Cache->PROJ4SRSCacheContext,
+											"PostGIS PROJ4 PJ Memory Context");
+#else
+/** TODO: Verify that CacheMemoryContext is best.  I originally tried PortalMemoryContext
+ * as suggested on pg-hackers, but that caused failures in delete of hash entries,
+ * I presume because PortalMemory was sometimes cleaned before delete happened.  **/
+	PJMemoryContext = AllocSetContextCreate(CacheMemoryContext,
+											 "PostGIS PROJ4 PJ Memory Context",
+											 ALLOCSET_SMALL_SIZES);
 
+#endif
+
 	/* Create the backend hash if it doesn't already exist */
 	if (!PJHash)
 		PJHash = CreatePJHash();
@@ -564,6 +587,25 @@
 
 	AddPJHashEntry(PJMemoryContext, projection);
 
+	/* Register cleanup callback only for PostgreSQL where
+	 * we are using built-in context instead of our own
+	 */
+	#if POSTGIS_PGSQL_VERSION >= 96
+	/* The hash key is the MemoryContext pointer */
+	void **key;
+	key = (void *)&PJMemoryContext;
+	/** We need to do this because GetPJHashEntry returns the projection
+	 * and NOT the entry as the name would lead you to believe **/
+	PJHashEntry *he;
+
+	/* Return the projection object from the hash */
+	he = (PJHashEntry *) hash_search(PJHash, key, HASH_FIND, NULL);
+	he->callback.func = PROJ4SRSCacheDelete;
+	he->callback.arg = (void *) he;
+	MemoryContextRegisterResetCallback(he->ProjectionContext,
+										&he->callback);
+	#endif
+
 	PROJ4Cache->PROJ4SRSCache[PROJ4Cache->PROJ4SRSCacheCount].srid = srid;
 	PROJ4Cache->PROJ4SRSCache[PROJ4Cache->PROJ4SRSCacheCount].projection = projection;
 	PROJ4Cache->PROJ4SRSCache[PROJ4Cache->PROJ4SRSCacheCount].projection_mcxt = PJMemoryContext;

Modified: trunk/postgis/lwgeom_geos_prepared.c
===================================================================
--- trunk/postgis/lwgeom_geos_prepared.c	2017-12-20 14:30:19 UTC (rev 16166)
+++ trunk/postgis/lwgeom_geos_prepared.c	2017-12-20 15:20:22 UTC (rev 16167)
@@ -79,11 +79,14 @@
 
 #define PREPARED_BACKEND_HASH_SIZE	32
 
-typedef struct
+typedef struct PrepGeomHashEntry
 {
 	MemoryContext context;
 	const GEOSPreparedGeometry* prepared_geom;
 	const GEOSGeometry* geom;
+#if POSTGIS_PGSQL_VERSION >= 96
+	MemoryContextCallback callback; /* for releasing hashentry when done */
+#endif
 }
 PrepGeomHashEntry;
 
@@ -94,22 +97,25 @@
 static PrepGeomHashEntry *GetPrepGeomHashEntry(MemoryContext mcxt);
 static void DeletePrepGeomHashEntry(MemoryContext mcxt);
 
-/* Memory context cache function prototypes */
+#if POSTGIS_PGSQL_VERSION < 96
+static void PreparedCacheDelete(MemoryContext context);
+#else
+static void  PreparedCacheDelete(void *arg);
+#endif
+/* Memory context cache function prototypes
+Only need for PostgreSQL where we will not be using a built-in memory context*/
+#if POSTGIS_PGSQL_VERSION < 96
 static void PreparedCacheInit(MemoryContext context);
 static void PreparedCacheReset(MemoryContext context);
-static void PreparedCacheDelete(MemoryContext context);
+
 static bool PreparedCacheIsEmpty(MemoryContext context);
-#if POSTGIS_PGSQL_VERSION >= 96
-static void PreparedCacheStats(MemoryContext context, int level, bool print, MemoryContextCounters *totals);
-#else
+
 static void PreparedCacheStats(MemoryContext context, int level);
-#endif
 
 #ifdef MEMORY_CONTEXT_CHECKING
 static void PreparedCacheCheck(MemoryContext context);
 #endif
 
-/* Memory context definition must match the current version of PostgreSQL */
 static MemoryContextMethods PreparedCacheContextMethods =
 {
 	NULL,
@@ -135,29 +141,7 @@
 	 */
 }
 
-static void
-PreparedCacheDelete(MemoryContext context)
-{
-	PrepGeomHashEntry* pghe;
 
-	/* Lookup the hash entry pointer in the global hash table so we can free it */
-	pghe = GetPrepGeomHashEntry(context);
-
-	if (!pghe)
-		elog(ERROR, "PreparedCacheDelete: Trying to delete non-existant hash entry object with MemoryContext key (%p)", (void *)context);
-
-	POSTGIS_DEBUGF(3, "deleting geom object (%p) and prepared geom object (%p) with MemoryContext key (%p)", pghe->geom, pghe->prepared_geom, context);
-
-	/* Free them */
-	if ( pghe->prepared_geom )
-		GEOSPreparedGeom_destroy( pghe->prepared_geom );
-	if ( pghe->geom )
-		GEOSGeom_destroy( (GEOSGeometry *)pghe->geom );
-
-	/* Remove the hash entry as it is no longer needed */
-	DeletePrepGeomHashEntry(context);
-}
-
 static void
 PreparedCacheReset(MemoryContext context)
 {
@@ -178,11 +162,7 @@
 }
 
 static void
-#if POSTGIS_PGSQL_VERSION >= 96
-PreparedCacheStats(MemoryContext context, int level, bool print, MemoryContextCounters *totals)
-#else
 PreparedCacheStats(MemoryContext context, int level)
-#endif
 {
 	/*
 	 * Simple stats display function - we must supply a function since this call is mandatory according to tgl
@@ -202,7 +182,41 @@
 	 */
 }
 #endif
+#endif
 
+
+
+static void
+#if POSTGIS_PGSQL_VERSION < 96
+PreparedCacheDelete(MemoryContext context)
+{
+	PrepGeomHashEntry* pghe;
+	/* Lookup the hash entry pointer in the global hash table so we can free it */
+	pghe = GetPrepGeomHashEntry(context);
+#else  /** to use built-in memory context we need to provide our delete hook in a form suitable for callback **/
+PreparedCacheDelete(void *arg)
+{
+	PrepGeomHashEntry *pghe =  (PrepGeomHashEntry *) arg;
+	MemoryContext context;
+	if (pghe)
+		context = pghe->context;
+#endif
+	if (!pghe)
+		elog(ERROR, "PreparedCacheDelete: Trying to delete non-existant hash entry object with MemoryContext key (%p)", (void *)context);
+
+	POSTGIS_DEBUGF(3, "deleting geom object (%p) and prepared geom object (%p) with MemoryContext key (%p)", pghe->geom, pghe->prepared_geom, context);
+
+	/* Free them */
+	if ( pghe->prepared_geom )
+		GEOSPreparedGeom_destroy( pghe->prepared_geom );
+	if ( pghe->geom )
+		GEOSGeom_destroy( (GEOSGeometry *)pghe->geom );
+
+	/* Remove the hash entry as it is no longer needed */
+	DeletePrepGeomHashEntry(context);
+}
+
+
 /* TODO: put this in common are for both transform and prepared
 ** mcxt_ptr_hash
 ** Build a key from a pointer and a size value.
@@ -307,7 +321,6 @@
 {
 	PrepGeomCache* prepcache = (PrepGeomCache*)cache;
 	PrepGeomHashEntry* pghe;
-
 	/*
 	* First time through? allocate the global hash.
 	*/
@@ -319,11 +332,26 @@
 	*/
 	if ( ! prepcache->context_callback )
 	{
+		/** TODO: This is yucky that we are reusing the same variable name
+		 * we used as a global pointer above, as a local variable.  We should reconsider this.
+		 * Took me a while to realize they weren't the same. **/
 		PrepGeomHashEntry pghe;
-		prepcache->context_callback = MemoryContextCreate(T_AllocSetContext, 8192,
-		                             &PreparedCacheContextMethods,
-		                             prepcache->context_statement,
-		                             "PostGIS Prepared Geometry Context");
+		#if POSTGIS_PGSQL_VERSION < 96
+			prepcache->context_callback = MemoryContextCreate(T_AllocSetContext, 8192,
+											&PreparedCacheContextMethods,
+											prepcache->context_statement,
+											"PostGIS Prepared Geometry Context");
+		#else
+/**For 9.5+ we can use a PostgreSQL context instead of creating our own and add a delete callback to it.
+ * TODO: change so not just PostgreSQL 11 goes thru this loop **/
+/** TODO: Verify that CacheMemoryContext is best.  I originally tried PortalMemoryContext
+ * as suggested on pg-hackers, but that caused failures in delete of hash entries,
+ * I presume because PortalMemory was sometimes cleaned before delete happened.  **/
+			prepcache->context_callback = AllocSetContextCreate(CacheMemoryContext,
+											 "PostGIS PROJ4 PJ Memory Context",
+											ALLOCSET_SMALL_SIZES
+			);
+		#endif
 		pghe.context = prepcache->context_callback;
 		pghe.geom = 0;
 		pghe.prepared_geom = 0;
@@ -361,6 +389,15 @@
 	* extra references in a global hash object.
 	*/
 	pghe = GetPrepGeomHashEntry(prepcache->context_callback);
+
+	#if POSTGIS_PGSQL_VERSION >= 96
+	/* Immediately register cleanup callback only for PostgreSQL where
+	 we are using built in context instead of our own  */
+	pghe->callback.func = PreparedCacheDelete;
+	pghe->callback.arg = (void *) pghe;
+	MemoryContextRegisterResetCallback(pghe->context,
+										&pghe->callback);
+	#endif
 	if ( ! pghe )
 	{
 		lwpgerror("PrepGeomCacheBuilder failed to find hash entry for context %p", prepcache->context_callback);
@@ -370,6 +407,8 @@
 	pghe->geom = prepcache->geom;
 	pghe->prepared_geom = prepcache->prepared_geom;
 
+
+
 	return LW_SUCCESS;
 }
 



More information about the postgis-tickets mailing list