[postgis-devel] lwgeom_*_in_place()

[Björn Harrtell]

I like it, but with some incoherent comments.

To start, it has caused quite a bit of confusion on my part that it's not
clear which methods are modifying in place and which are not. Before
https://github.com/postgis/postgis/commit/f45231f3e4ea7217fa270e9c8b207c619e7c430e
the code would sometimes corrupt the actual LWGEOM on disk (how that works
is still a mystery to me).

To throw in an observation on signature - lwgeom_affine returns void so
it's clear that it must be mutating? I would also guess that anything that
uses GEOS will not mutate because the geometry has to be converted. Is not
returning void enough to indicate a function (that does something) mutates?
In either case, I'm not certain adding a *_in_place() variants to a lot of
places would result in a nicer API.

/Björn

2017-10-02 23:53 GMT+02:00 Paul Ramsey <pramsey at cleverelephant.ca>:

> Hey devs,
> So, in pursuit of spare CPU cycles for the good folks at Carto, I've been
> putting various workloads through a profiler. Some of the results you've
> seen in recent tweaks committed.
>
> One thing I found as a general rule was that as the number of simple
> objects goes up, the more noticeable the overhead of things like memory
> allocation / deallocation becomes. So the overhead of just constructing an
> LWPOINT on a point, when you're slamming through 1M of them, is not nothing.
>
> For some workloads, like generating MVT (there it is) we call numerous
> geometry processing functions in a row. For some functions, that expect
> const LWGEOM inputs, that can result in a lot of cloning of objects, and
> hence, memory management. For other functions, that expect to modify things
> in place, the coordinates are just altered right there.
>
> This has always been a little ugliness in our API, and for a while we
> settled on defaulting to making copies, so the API was at least getting
> cleaner. But also getting slower.
>
> I built a prototype for generating MVT that does all the coordinate
> processing in place, and for bulk simple objects it ran about 30% faster
> than the existing code. There's a benefit to be had to in working in place.
>
> I propose that we... cut the baby in half.
>
> * All liblwgeom functions that do in place coordinate modification should
> be re-named to *_in_place() and a bare version that takes a const LWGEOM
> and returns a copy as necessary be added.
> * All liblwgeom functions that already take const LWGEOM be supplemented
> with *_in_place() variants so that it's possible to build processing chains
> that are capable of doing processing with a minimal allocation/deallocation
> footprint.
>
> Functions on my list immediately:
>
>   * remove_repeated_points
>   * simplify
>   * affine
>   * grid
>   * transform
>
> I will, however, do a full review and attempt to ensure all non-GEOS
> functions have in_place variants available in liblwgeom.
>
> Thoughts?
>
> P.
>
>
>
> _______________________________________________
> postgis-devel mailing list
> postgis-devel at lists.osgeo.org
> https://lists.osgeo.org/mailman/listinfo/postgis-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osgeo.org/pipermail/postgis-devel/attachments/20171003/06126072/attachment.html>