-
Notifications
You must be signed in to change notification settings - Fork 25
Open
Description
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:
Lines 29 to 50 in 7ad3b3c
| mutable struct Point <: AbstractGeometry | |
| ptr::GEOSGeom | |
| context::GEOSContext | |
| # create a point from a pointer - only makese sense if it is a pointer to a point, otherwise error | |
| function Point(obj::Union{Point,GEOSGeom}, context::GEOSContext = get_global_context()) | |
| id = LibGEOS.geomTypeId(obj, context) | |
| point = if id == GEOS_POINT | |
| point = new(cloneGeom(obj, context), context) | |
| else | |
| open_issue_if_conversion_makes_sense(Point, id) | |
| end | |
| finalizer(destroyGeom, point) | |
| point | |
| end | |
| # create a point from a vector of floats | |
| Point(coords::Vector{Float64}, context::GEOSContext = get_global_context()) = | |
| Point(createPoint(coords, context), context) | |
| Point(x::Real, y::Real, context::GEOSContext = get_global_context()) = | |
| Point(createPoint(x, y, context), context) | |
| Point(x::Real, y::Real, z::Real, context::GEOSContext = get_global_context()) = | |
| Point(createPoint(x, y, z, context), context) | |
| end |
Which leads to insane performance like this:
JuliaGeo/GeometryOps.jl#32
74 allocations to get the area of an 11 point geometry.
We also call getPoint :
LibGEOS.jl/src/geos_functions.jl
Lines 1516 to 1546 in 7ad3b3c
| # Call only on LINESTRING, and must be freed by caller (Returns NULL on exception) | |
| function getPoint( | |
| obj::Union{LineString,LinearRing}, | |
| n::Integer, | |
| context::GEOSContext = get_context(obj), | |
| ) | |
| if !(0 < n <= numPoints(obj, context)) | |
| error( | |
| "LibGEOS: n=$n is out of bounds for LineString with numPoints=$(numPoints(obj, context))", | |
| ) | |
| end | |
| result = GEOSGeomGetPointN_r(context, obj, n - 1) | |
| if result == C_NULL | |
| error("LibGEOS: Error in GEOSGeomGetPointN") | |
| end | |
| Point(result, context) | |
| end | |
| # Call only on LINESTRING, and must be freed by caller (Returns NULL on exception) | |
| function startPoint( | |
| obj::Union{LineString,LinearRing}, | |
| context::GEOSContext = get_context(obj), | |
| ) | |
| result = GEOSGeomGetStartPoint_r(context, obj) | |
| if result == C_NULL | |
| error("LibGEOS: Error in GEOSGeomGetStartPoint") | |
| end | |
| Point(result, context) | |
| end | |
| # Call only on LINESTRING, and must be freed by caller (Returns NULL on exception) |
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)
Metadata
Metadata
Assignees
Labels
No labels