<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">2017-10-03 14:07 GMT+02:00 Paul Ramsey <span dir="ltr"><<a href="mailto:pramsey@cleverelephant.ca" target="_blank">pramsey@cleverelephant.ca</a>></span>:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><span class="gmail-">On Mon, Oct 2, 2017 at 10:24 PM, Björn Harrtell <span dir="ltr"><<a href="mailto:bjorn.harrtell@gmail.com" target="_blank">bjorn.harrtell@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">I like it, but with some incoherent comments.<div><br></div><div>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 <a href="https://github.com/postgis/postgis/commit/f45231f3e4ea7217fa270e9c8b207c619e7c430e" target="_blank">https://github.com/post<wbr>gis/postgis/commit/f45231f3e4e<wbr>a7217fa270e9c8b207c619e7c430e</a> the code would sometimes corrupt the actual LWGEOM on disk (how that works is still a mystery to me).</div></div></blockquote><div><br></div></span><div>Your issue would have been the affine, in cases where you weren't intersecting. In that case you'd have been writing on an uncopied piece of pgsql memory (from PG_GETARG_GSERIALIZED_P()) which it might not like. I too am a little unclear on the lifecycle which would allow it to get corrupted before you escape by serializing your own return value to a fresh piece of memory. But we've seen it happen in lots of other functions, so.</div><span class="gmail-"><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div>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.<br></div></div></blockquote><div><br></div></span><div>The in_place() variants will all return an integer (status code), and they'll all have a non-const LWGEOM *argument. The copying variants will all return an LWGEOM *value and will have a const LWGEOM *argument. So there will be a number of signals that some mutation is going to occur.</div><div><br></div><div>From a PostGIS usage point of view the main requirement to use an in_place() method is that you start off your PostGIS function with a PG_GETARG_GSERIALIZED_P_COPY()<wbr>, so that you own your memory before you start scribbling on it. This should be cheaper than doing a PG_GETARG_GSERIALIZED_P() lwgeom_from_gserialized() and then an lwgeom_clone_deep(), as it involves one big allocation rather than multliple small ones.</div><div><br></div><div>Hopefully between the variation of arguments and return values and signature it'll be obvious which functions do what.</div><div><br></div><div>(Remember that, in all these discussions, when I say "cheaper" I mean very barely, since allocation overhead is quite small, only noticeable for bulk handling of small objects)</div><span class="gmail-HOEnZb"><font color="#888888"><div><br></div></font></span></div></div></div></blockquote><div><br></div><div>Good rationale and details of your suggested changes to the API. So, now I like it without incoherent comments.</div><div><br></div><div>While the real world benefit might be small depending on use case, I would assume it can affect more than "lots of small objects" i.e when processing or preseeding any large volume of geometries/tiles. Will use PG_GETARG_GSERIALIZED_P_COPY in my stuff if you do not beat me to it.</div><div><br></div><div>/Björn</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span class="gmail-HOEnZb"><font color="#888888"><div></div><div>P.</div></font></span><div><div class="gmail-h5"><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><span class="gmail-m_826297314077903003gmail-HOEnZb"><font color="#888888"><div><br></div></font></span><div><span class="gmail-m_826297314077903003gmail-HOEnZb"><font color="#888888">/Björn<br></font></span><div><div class="gmail_extra"><br><div class="gmail_quote"><div><div class="gmail-m_826297314077903003gmail-h5">2017-10-02 23:53 GMT+02:00 Paul Ramsey <span dir="ltr"><<a href="mailto:pramsey@cleverelephant.ca" target="_blank">pramsey@cleverelephant.ca</a>></span>:<br></div></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div><div class="gmail-m_826297314077903003gmail-h5"><div dir="ltr">Hey devs,<div>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.</div><div><br></div><div>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.</div><div><br></div><div>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.</div><div><br></div><div>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.</div><div><br></div><div>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.<br></div><div><br></div><div>I propose that we... cut the baby in half.</div><div><br></div><div>* 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.</div><div>* 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.<br></div><div><br></div><div>Functions on my list immediately:</div><div><br></div><div><div> * remove_repeated_points</div><div> * simplify</div><div> * affine</div><div> * grid</div><div> * transform</div><div> </div></div><div>I will, however, do a full review and attempt to ensure all non-GEOS functions have in_place variants available in liblwgeom.</div><div><br></div><div>Thoughts?</div><span class="gmail-m_826297314077903003gmail-m_8364327507234904924gmail-HOEnZb"><font color="#888888"><div><br></div><div>P.</div><div><br></div><div><br></div></font></span></div>
<br></div></div><span class="gmail-m_826297314077903003gmail-">______________________________<wbr>_________________<br>
postgis-devel mailing list<br>
<a href="mailto:postgis-devel@lists.osgeo.org" target="_blank">postgis-devel@lists.osgeo.org</a><br>
<a href="https://lists.osgeo.org/mailman/listinfo/postgis-devel" rel="noreferrer" target="_blank">https://lists.osgeo.org/mailma<wbr>n/listinfo/postgis-devel</a><br></span></blockquote></div><br></div></div></div></div>
<br>______________________________<wbr>_________________<br>
postgis-devel mailing list<br>
<a href="mailto:postgis-devel@lists.osgeo.org" target="_blank">postgis-devel@lists.osgeo.org</a><br>
<a href="https://lists.osgeo.org/mailman/listinfo/postgis-devel" rel="noreferrer" target="_blank">https://lists.osgeo.org/mailma<wbr>n/listinfo/postgis-devel</a><br></blockquote></div></div></div><br></div></div>
</blockquote></div><br></div></div>