Skip to content

feat: add LatLngToCellBatch for lower cgo overhead#119

Open
zoobst wants to merge 4 commits into
uber:masterfrom
zoobst:feat/latlng-to-cells-batch
Open

feat: add LatLngToCellBatch for lower cgo overhead#119
zoobst wants to merge 4 commits into
uber:masterfrom
zoobst:feat/latlng-to-cells-batch

Conversation

@zoobst

@zoobst zoobst commented May 27, 2026 •
edited
Loading

Copy link
Copy Markdown

Resolves #113

Adds LatLngToCellBatch so a slice of LatLng -> []Cell costs one cgo transaction for the whole batch instead of one per row. The motivating use case is pipelines transforming millions of coordinate rows per cycle, where cgo overhead currently dominates actual H3 work.

Per @jogly's suggestion in the issue, the implementation lives in a new C extension (h3_latLngBatch.{c,h}) that supplements the cloned H3 core. If H3 core later exposes and equivalent latLngToCellBatch, swapping the Go wrapper to call it directly is a one-line change.

Changes

File Change
h3_latLngBatch.h (new) Declaration of latLngToCellBatch
h3_latLngBatch.c (new) C-side loop calling latLngToCell
h3.go #include <h3_latLngBatch.h> in cgo preamble; new LatLngToCellBatch wrapper
h3_test.go Parity tests against per-call across all resolutions, empty input, single element, invalid resolution
bench_test.go BenchmarkLatLngToCellBatch plus a BenchmarkLatLngToCellBaseline for comparing results

Benchmark

goos: darwin
goarch: arm64
pkg: github.com/uber/h3-go/v4
cpu: Apple M1
BenchmarkLatLngToCellsBatch/1-8                  7131451           319.4 ns/op
BenchmarkLatLngToCellsBatch/64-8                  144639           16473 ns/op
BenchmarkLatLngToCellsBatch/1024-8                  8794           263703 ns/op
BenchmarkLatLngToCellsBatch/16384-8                  564           4244748 ns/op
BenchmarkLatLngToCellsBatch/1000000-8                  8           259712438 ns/op
BenchmarkLatLngToCellsBatch/10000000-8                 1           2623521042 ns/op
BenchmarkLatLngToCellsBaseline/1-8               7072687           309.8 ns/op
BenchmarkLatLngToCellsBaseline/64-8               120739           19834 ns/op
BenchmarkLatLngToCellsBaseline/1024-8               7389           319792 ns/op
BenchmarkLatLngToCellsBaseline/16384-8               465           5118101 ns/op
BenchmarkLatLngToCellsBaseline/1000000-8               7           312783863 ns/op
BenchmarkLatLngToCellsBaseline/10000000-8              1           3151075250 ns/op

At small n (n < 32) the per-call path wins. The crossover is around n=16-32; from there the batched version grows roughly linearly while the loop pays the cgo cost N times. At n=16384 the batched version is ~1.21x faster

Conventions

  • Returns first error encountered, no partial results.
  • Conversion of degrees -> radians happens in Go via the existing LatLng.toC(), so the C code sees radians-LatLng identical to the single-call path.
  • No changes to public types or existing function signatures.

zoobst added 2 commits May 27, 2026 09:35
@zoobst
feat: add LatLngToCellsBatch for lower cgo overhead. Resolves uber#113.…
c4df3c8 
… Adds a batched LatLng -> Cell API in a new C file (h3_latLngBatch.{c,h}) that supplements the cloned H3 core.
@zoobst
updated benchmarks to align with benchmark format
c977b0c
@CLAassistant

CLAassistant commented May 27, 2026 •
edited
Loading

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

jogly
jogly previously approved these changes May 28, 2026
View reviewed changes
@coveralls

coveralls commented May 28, 2026 •
edited
Loading

Copy link
Copy Markdown

Coverage Report for CI Build 26683177239

Coverage remained the same at 100.0%

Details

  • Coverage remained the same as the base build.
  • Patch coverage: 24 of 24 lines across 1 file are fully covered (100%).
  • No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.

Coverage Stats

Coverage Status
Relevant Lines: 815
Covered Lines: 815
Line Coverage: 100.0%
Coverage Strength: 298.19 hits per line
💛 - Coveralls

justinhwang
justinhwang reviewed May 29, 2026
View reviewed changes
Comment thread h3.go Outdated
zachcoleman
zachcoleman previously approved these changes May 29, 2026
View reviewed changes

@zachcoleman zachcoleman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

lgtm and clean

@zoobst zoobst dismissed stale reviews from zachcoleman and jogly via b9716ae May 30, 2026 11:54
@zoobst zoobst requested review from justinhwang May 30, 2026 11:59
@zoobst zoobst changed the title feat: add LatLngToCellsBatch for lower cgo overhead feat: add LatLngToCellBatch for lower cgo overhead May 30, 2026
@justinhwang

justinhwang commented Jun 1, 2026

Copy link
Copy Markdown
Collaborator

Actually had a thought on pure Go implementations (as we can see in #120) - I'm wondering if its a pure Go implementation, then would we also get rid of the Cgo overhead and wouldn't need a batched version?

Before (CGo):

BenchmarkLatLngToCell-16         4155702               270.4 ns/op            24 B/op          2 allocs/op

After (pure Go):

BenchmarkLatLngToCell-16         4177394               288.9 ns/op             0 B/op          0 allocs/op

benchstat (n=10):

                │ /tmp/latlng_old.txt │        /tmp/latlng_new3.txt        │
                │       sec/op        │   sec/op     vs base               │
LatLngToCell-16           273.6n ± 1%   291.2n ± 1%  +6.43% (p=0.000 n=10)

                │ /tmp/latlng_old.txt │        /tmp/latlng_new3.txt        │
                │        B/op         │   B/op     vs base                 │
LatLngToCell-16            24.00 ± 0%   0.00 ± 0%  -100.00% (p=0.000 n=10)

                │ /tmp/latlng_old.txt │        /tmp/latlng_new3.txt         │
                │      allocs/op      │ allocs/op   vs base                 │
LatLngToCell-16            2.000 ± 0%   0.000 ± 0%  -100.00% (p=0.000 n=10)

@zoobst

zoobst commented Jun 2, 2026

Copy link
Copy Markdown
Author

Actually had a thought on pure Go implementations (as we can see in #120) - I'm wondering if its a pure Go implementation, then would we also get rid of the Cgo overhead and wouldn't need a batched version?

As a user and h3 & Go enthusiast, I'm all for pure Go optimizations.
However, the codebase in this repo seems to avoid becoming a pure Go h3 implementation and tries to stay as aligned as possible to core h3.
Not my call here, but would be happy to contribute to a pure Go h3 lib in the future.

@jogly

jogly commented Jun 3, 2026

Copy link
Copy Markdown
Collaborator

I'm open to allowing this batch function in, with an additional comment that it is specifically an optimization for CGo overhead in bulk. Simultaneously, @justinhwang - looks like you have at least a branch for a pure Go h3? This should be pretty easy to maintain with AI now, would you be interested in putting up a branch & PR, then we can move this discussion there? The original motivations for the bindings was to avoid the maintenance overhead for us humans as development moved on core, but this constraint has largely become moot now, imo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jogly jogly jogly left review comments

@zachcoleman zachcoleman zachcoleman left review comments

@justinhwang justinhwang Awaiting requested review from justinhwang

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Consider batched LatLngToCells API to lower cgo transition overhead

6 participants

@zoobst @CLAassistant @coveralls @justinhwang @jogly @zachcoleman