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

Replaced s2 contains point methods with go-geom #5023

Merged
merged 1 commit into from
Mar 30, 2020

Conversation

sams96
Copy link
Contributor

@sams96 sams96 commented Mar 24, 2020

Leads to significantly higher performance.

goos: linux
goarch: amd64
BenchmarkMatchesFilterContainsPoint-8               3902            312904 ns/op
BenchmarkMatchesFilterContainsPoint_Before-8          31          35054224 ns/op
PASS
ok      command-line-arguments  2.636s

From my testing while working on a project of my own I found that while using s2 types is very fast, converting to them is very slow, so if you're converting each time it's much faster to just use the go-geom types directly. This PR just changes this for contains point queries.


This change is Reviewable

Leads to significantly higher performance.
@sams96 sams96 requested review from manishrjain and a team as code owners March 24, 2020 21:09
Copy link
Contributor

@manishrjain manishrjain left a comment

Choose a reason for hiding this comment

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

:lgtm: Thanks for the PR!

Reviewed 2 of 2 files at r1.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@manishrjain manishrjain merged commit ff69bd2 into hypermodeinc:master Mar 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants