LibGEOS.jl icon indicating copy to clipboard operation
LibGEOS.jl copied to clipboard

Swap to returning Tuples instead of `LibGEOS.Point` for anything GeoInterface related

Open rafaqz opened this issue 1 year ago • 5 comments

This is likely related to the memory leak too. We are creating LibGEOS Point objects for every single point we read in a geometry, but they're incredibly expensive and allocate objects that need to be finalized:

https://github.com/JuliaGeo/LibGEOS.jl/blob/7ad3b3c8c8743836924e8c75fb810a2378cb8e7e/src/geos_types.jl#L29-L50

Which leads to insane performance like this: https://github.com/asinghvi17/GeometryOps.jl/issues/32

74 allocations to get the area of an 11 point geometry.

We also call getPoint : https://github.com/JuliaGeo/LibGEOS.jl/blob/7ad3b3c8c8743836924e8c75fb810a2378cb8e7e/src/geos_functions.jl#L1516-L1546

We should replace all of this with just getting two floating point values in the simplest way possible, and getting them all at once for whole LinearRing/LineString in GI.getpoint(linestring)

rafaqz avatar Dec 28 '23 00:12 rafaqz

After #191 it's at least type stable, but indeed, much copying is done. Screenshot 2024-01-01 at 14 52 21

evetion avatar Jan 01 '24 13:01 evetion

I think it can be 50x faster, for linestrings anyway

rafaqz avatar Jan 01 '24 14:01 rafaqz

Sure, but not sure whether this is the right place to do so. Libgeos should be consistent. What could work is making a single efficient copy to a GI Polygon/Ring for Ops to work on?

evetion avatar Jan 01 '24 14:01 evetion

We already did this for ArchGDAL for the same reason.

We only need to be consistent with GeoInterface in GeoInterface methods.

The idea is to do what your saying, but how could you do that in GeometryOps without a LibGeos dep?

The only way I can see to do it is here in GI.getgeom on the iterator over LineString

rafaqz avatar Jan 01 '24 18:01 rafaqz

I like to keep packages internally consistent as well. I missed the ArchGDAL change, let me check that.

Maybe this isn't needed with all the other activities.

evetion avatar Jan 02 '24 06:01 evetion