Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

faster area #36

Merged
merged 3 commits into from
Jan 2, 2024
Merged

faster area #36

merged 3 commits into from
Jan 2, 2024

Conversation

rafaqz
Copy link
Member

@rafaqz rafaqz commented Jan 1, 2024

With all the fixes in LibGEOS this gets us to ~20x slower territory for #32

julia> @benchmark LG.area($pLG)
BenchmarkTools.Trial: 10000 samples with 990 evaluations.
 Range (min  max):  32.888 ns  71.911 ns  ┊ GC (min  max): 0.00%  0.00%
 Time  (median):     33.754 ns              ┊ GC (median):    0.00%
 Time  (mean ± σ):   35.655 ns ±  4.169 ns  ┊ GC (mean ± σ):  0.00% ± 0.00%

  ▆▄█▇ ▄▁▁▄  ▂             ▁    ▂ ▁▄▂   ▃    ▁                ▁
  █████████████▇▇█▄▇█▅▅▆▇▅▇█▅▄▃▆█▆███▆▆██▇▅▆▄█▇▄▅▆▆▆▅▅▅▇▅▆▅▄▅ █
  32.9 ns      Histogram: log(frequency) by time      49.9 ns <

 Memory estimate: 0 bytes, allocs estimate: 0.

julia> @benchmark GO.area($pLG)
BenchmarkTools.Trial: 10000 samples with 143 evaluations.
 Range (min  max):  714.762 ns   5.840 ms  ┊ GC (min  max): 0.00%  5.31%
 Time  (median):       1.111 μs              ┊ GC (median):    0.00%
 Time  (mean ± σ):     2.401 μs ± 73.988 μs  ┊ GC (mean ± σ):  2.36% ± 0.08%

    ▁█ ▁  ▁      ▃▂                                             
  ▁▂██▅█▆▅█▅▄▄▅▆▆██▃▃▂▂▁▂▂▃▃▃▃▃▃▃▃▃▂▂▂▂▂▂▁▁▂▂▂▂▂▂▂▂▂▂▂▂▂▁▁▁▁▁▁ ▂
  715 ns          Histogram: frequency by time         2.45 μs <

 Memory estimate: 80 bytes, allocs estimate: 3.

julia> @benchmark GO.area($pGI)
BenchmarkTools.Trial: 10000 samples with 996 evaluations.
 Range (min  max):  18.195 ns  461.430 ns  ┊ GC (min  max): 0.00%  0.00%
 Time  (median):     18.467 ns               ┊ GC (median):    0.00%
 Time  (mean ± σ):   22.606 ns ±  12.344 ns  ┊ GC (mean ± σ):  0.00% ± 0.00%

  █   ▁ ▁▂   ▃ ▃ ▅ ▅  ▁ ▂  ▁                       ▁▁▁▁▂▂▂▁▁   ▂
  █▇█▁██████▆█▆█████▅▇█▆█▇▆███▄▅▅▆▆▆▅▆▆▆▇▇▆▆▇███▇█████████████ █
  18.2 ns       Histogram: log(frequency) by time      37.9 ns <

 Memory estimate: 0 bytes, allocs estimate: 0.

(and 2x faster than libgeos when working with native geoms!)

The idea with deleting all the handling of closed/open polygons is that the last point will contribute zero area anyway if its a repeat of the first one - so we can just ignore it.

@rafaqz rafaqz requested a review from skygering January 1, 2024 23:01
np -= first_last_equal ? 1 : 0

first = true
local pfirst, p1
Copy link
Collaborator

@skygering skygering Jan 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does local do here? Just for my future knowledge?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It makes the variables available in the scope outside thep loop

# Skip the first and do it later
# This lets us work within one iteration over geom,
# which means on C call when using points from external libraries.
if first
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah this is a pretty good way to do this.

@skygering
Copy link
Collaborator

Looks great! Dang, the LibGEOS performance is still painful! But excited about the improvements with native polygons!

@rafaqz
Copy link
Member Author

rafaqz commented Jan 2, 2024

I got LibGEOS down to 3x faster than that with other changes...

@rafaqz rafaqz merged commit 75603bd into main Jan 2, 2024
4 checks passed
@rafaqz rafaqz deleted the faster_area branch January 2, 2024 11:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants