go-spatial / geom

Geometry interfaces to help drive interoperability within the Go geospatial community
MIT License
168 stars 37 forks source link

simplify: memory optimization #71

Closed ear7h closed 5 years ago

ear7h commented 5 years ago

Firstly I added a couple more complicated test cases and benchmarks. The benchmarks include smooth circular geometries, a zig zag (causes the algorithm to degrade to O(n^2)) and a real geometry from the Natural Earth data set.

Modified simplify to only allocate memory once per call. I looked at other possible optimizations, like unrolling the dist loop, but it did not improve the algorithm (likely because the distance function is a variable and not being inlined).

The memory optimizations alone show significant time improvements, especially for larger geometries. However, the overall size of the allocations is much higher because we are preallocating an slice the same size as the original. The benchmarks where this is a problem is only in simple cases *Circle which can be heavily simplified. In the case of South Africa and the Zig Zag, the allocation size is still less than the original.

name                         old time/op    new time/op    delta
DouglasPeuckerCircle-4          514µs ± 1%     472µs ± 2%     -8.34%  (p=0.000 n=8+7)
DouglasPeuckerWavyCircle0-4     568µs ± 1%     516µs ± 1%     -9.16%  (p=0.000 n=8+8)
DouglasPeuckerWavyCircle1-4    7.90ms ± 0%    7.13ms ± 0%     -9.72%  (p=0.000 n=8+8)
DouglasPeuckerZigZag-4         3.82ms ± 1%    3.34ms ± 1%    -12.48%  (p=0.000 n=8+8)
DouglasPeuckerSA-4             7.42µs ± 0%    4.04µs ± 0%    -45.58%  (p=0.001 n=7+7)

name                         old alloc/op   new alloc/op   delta
DouglasPeuckerCircle-4         16.4kB ± 0%   164.0kB ± 0%   +897.94%  (p=0.000 n=8+7)
DouglasPeuckerWavyCircle0-4    18.6kB ± 0%   164.0kB ± 0%   +780.87%  (p=0.001 n=7+7)
DouglasPeuckerWavyCircle1-4    95.3kB ± 0%  1625.5kB ± 0%  +1605.74%  (p=0.000 n=7+8)
DouglasPeuckerZigZag-4          113kB ± 0%      16kB ± 0%    -85.46%  (p=0.000 n=8+8)
DouglasPeuckerSA-4             6.67kB ± 0%    1.41kB ± 0%    -78.90%  (p=0.000 n=8+8)

name                         old allocs/op  new allocs/op  delta
DouglasPeuckerCircle-4            190 ± 0%         1 ± 0%    -99.47%  (p=0.000 n=8+8)
DouglasPeuckerWavyCircle0-4       244 ± 0%         1 ± 0%    -99.59%  (p=0.000 n=8+8)
DouglasPeuckerWavyCircle1-4     1.00k ± 0%     0.00k ± 0%    -99.90%  (p=0.002 n=7+8)
DouglasPeuckerZigZag-4          2.01k ± 0%     0.00k ± 0%    -99.95%  (p=0.000 n=8+8)
DouglasPeuckerSA-4               99.0 ± 0%       1.0 ± 0%    -98.99%  (p=0.000 n=8+8)
coveralls commented 5 years ago

Pull Request Test Coverage Report for Build 310


Changes Missing Coverage Covered Lines Changed/Added Lines %
testing/dynamic_geoms.go 10 11 90.91%
planar/simplify/douglaspeucker.go 13 21 61.9%
planar/planar.go 0 15 0.0%
<!-- Total: 23 47 48.94% -->
Files with Coverage Reduction New Missed Lines %
planar/simplify/douglaspeucker.go 1 59.42%
<!-- Total: 1 -->
Totals Coverage Status
Change from base Build 287: 0.07%
Covered Lines: 5782
Relevant Lines: 9955

💛 - Coveralls
ear7h commented 5 years ago

Update: the postgis reference tests will fail due to #73