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

quadtree: sort KNearest results closest first #75

Merged
merged 3 commits into from
Oct 16, 2021
Merged

Conversation

paulmach
Copy link
Owner

as requested here #67

The results of the quadtree.KNearest were just the "heap" order. Now they are sorted closest first.

It is slower however, would probably need to implement a custom heap to get rid of the allocs. I did that for the visvalingam simplification here. Hopefully generics will allow this to be simple and performant.

benchmark                         old ns/op     new ns/op     delta
BenchmarkRandomKNearest100-12     31442         43139         +37.20%

benchmark                         old allocs     new allocs     delta
BenchmarkRandomKNearest100-12     269            369            +37.17%

benchmark                         old bytes     new bytes     delta
BenchmarkRandomKNearest100-12     12083         15280         +26.46%

Copy link

@sid6mathur sid6mathur left a comment

Choose a reason for hiding this comment

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

Thank you so much @paulmach . Much gratitude.

benchmarks for move to our own maxHeap

benchmark                         old ns/op     new ns/op     delta
BenchmarkRandomKNearest10-12      4333          2094          -51.67%
BenchmarkRandomKNearest100-12     43245         15957         -63.10%

benchmark                         old allocs     new allocs     delta
BenchmarkRandomKNearest10-12      44             14             -68.18%
BenchmarkRandomKNearest100-12     369            104            -71.82%

benchmark                         old bytes     new bytes     delta
BenchmarkRandomKNearest10-12      1777          472           -73.44%
BenchmarkRandomKNearest100-12     15288         3432          -77.55%
@paulmach
Copy link
Owner Author

I implement an "internal" maxHeap vs using the std lib one. This makes it twice as fast AND returns sorted results. Allocations are now ~K vs. "nodes seen".

If you were doing this 1000s of times for the same quadtree you could implement something to reuse the maxHeap memory. Some sort of KNearestContainer and create one per goroutine/thread. That would put you down to 2-4 allocations per KNearest search.

benchmark                         old ns/op     new ns/op     delta
BenchmarkRandomKNearest10-12      3650          2094          -42.63%
BenchmarkRandomKNearest100-12     32616         15957         -51.08%

benchmark                         old allocs     new allocs     delta
BenchmarkRandomKNearest10-12      34             14             -58.82%
BenchmarkRandomKNearest100-12     269            104            -61.34%

benchmark                         old bytes     new bytes     delta
BenchmarkRandomKNearest10-12      1457          472           -67.60%
BenchmarkRandomKNearest100-12     12081         3432          -71.59%

@paulmach paulmach merged commit 59d7b22 into master Oct 16, 2021
@paulmach paulmach deleted the knearest-sort branch October 16, 2021 19:49
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