<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">https://github.com/postgis/postgis/commit/f45231f3e4ea7217fa270e9c8b207c619e7c430e</a> the code would sometimes corrupt the actual LWGEOM on disk (how that works is still a mystery to me).</div><div><br></div><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.</div><div><br></div><div>/Björn<br><div><div class="gmail_extra"><br><div class="gmail_quote">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><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">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-HOEnZb"><font color="#888888"><div><br></div><div>P.</div><div><br></div><div><br></div></font></span></div>
<br>______________________________<wbr>_________________<br>
postgis-devel mailing list<br>
<a href="mailto:postgis-devel@lists.osgeo.org">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/<wbr>mailman/listinfo/postgis-devel</a><br></blockquote></div><br></div></div></div></div>