Opened 6 years ago
Closed 5 years ago
#4264 closed enhancement (wontfix)
Consider dropping pfree calls in liblwgeom
Reported by: | Algunenano | Owned by: | Algunenano |
---|---|---|---|
Priority: | medium | Milestone: | PostGIS 3.0.0 |
Component: | postgis | Version: | master |
Keywords: | Cc: |
Description
Recently I became aware that Postgresql deletes the memory context per tuple, not per function. With that in mind, it doesn't make sense for the library to be calling pfree since waiting it's going to wait until the memory context is freed anyway.
Rough Patch:
diff --git a/libpgcommon/lwgeom_pg.c b/libpgcommon/lwgeom_pg.c index 1ee41f0cb..2733d3cbe 100644 --- a/libpgcommon/lwgeom_pg.c +++ b/libpgcommon/lwgeom_pg.c @@ -113,7 +113,7 @@ pg_realloc(void *mem, size_t size) static void pg_free(void *ptr) { - pfree(ptr); + // pfree(ptr); }
I decided to test this patch to generate polygon MVTs, and I see a 5-10% improvement in the slowest cases (the ones that do the whole process).
3 main thoughts:
- Check that there isn't any function is requesting a lot of memory. This might mean redesigning some functions (none come to mind) to avoid allocating in a loop or recursively, but that by itself is a win.
- Check that it's only applied to liblwgeom when working as a library, and not to external libraries (GEOS, PROJ, etc.): I think this is already true, but I have to make sure.
- I don't intend to apply this to the extensions code itself (yet).
Any comments are very welcome.
Attachments (1)
Change History (4)
by , 6 years ago
Attachment: | 20181128_postgis_def_vs_20181128_postgis_nofree_def.pdf added |
---|
comment:1 by , 6 years ago
ST_ClusterKMeans is a thing to check (did it allocate in a loop?), also ST_Subdivide may requre many intermediates. Can we get a version of pg_free that just logs free() calls into some array and then frees say on 1000th free?
comment:2 by , 6 years ago
ST_ClusterKMeans is a thing to check (did it allocate in a loop?), also ST_Subdivide may requre many intermediates.
An easy way out would be to change handlers in those functions (they are both changing context too) and have pfree acting as it's acting now.
Can we get a version of pg_free that just logs free() calls into some array and then frees say on 1000th free?
I don't think that would be positive, it is the same as giving control to Postgres with the added complexity of handing an intermediate step ourselves.
comment:3 by , 5 years ago
Resolution: | → wontfix |
---|---|
Status: | assigned → closed |
MVT polygon perf comparison (trunk head vs trunk without free calls)