Improve performance of H3 geo functions (combined sincos in coordinate transforms)#109399
Merged
Conversation
Bump contrib/h3 to pick up combined sincos + redundant-trig removal in the lat/lng <-> H3 cell transforms. Bit-for-bit identical output; speeds up h3ToGeoBoundary ~14%, h3ToGeo ~9-11%, h3CellArea* ~8%, geoToH3 ~3-4%. Recovers the coordinate-transform speedups from the Rust h3o experiment (#100272) while keeping the C library backend. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
|
Workflow [PR], commit [f5ff4b2] Summary: ✅
AI ReviewSummaryThis PR updates Final VerdictStatus: ✅ Approve |
Member
Author
|
Sounds good. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Updates the
contrib/h3submodule to ClickHouse/h3#11, which speeds up the H3 geo coordinate transforms by computing pairedsin/costogether (onesincoscall) and removing redundant trig evaluations in the hot lat/lng <-> cell routines (_geoToVec3d,_geoAzimuthRads,_geoAzDistanceRads,_geoToHex2d).Why
PR #100272 explored replacing the H3 C library with the Rust
h3ocrate for performance. Investigation showed that in a fair-O3build the C library is already competitive withh3o(the large gaps in that PR came from an un-inlined C build), and the genuinely portable win is the combined-sincos/ redundant-trig cleanup — which needs no Rust, no FFI wrappers, no panics across the boundary, and no backward-incompatible metric changes.Correctness
Bit-for-bit identical output — the change only fuses paired calls (which return the same values) and removes duplicate evaluations. All 311 upstream h3 unit tests pass unchanged, so no ClickHouse
.referencefiles change.Performance (10M rows, clang -O3)
h3ToGeoBoundaryh3ToGeoh3CellAreaM2/h3CellAreaRads2geoToH3h3ToParent/h3IsValidare already optimal (loopless bit-twiddling) and unchanged.Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):
Improve the performance of H3 geo functions (
h3ToGeoBoundary,h3ToGeo,h3CellAreaM2/h3CellAreaRads2,geoToH3) by computing paired sine/cosine together and eliminating redundant trigonometric calls in the coordinate transforms. Results are unchanged (bit-for-bit identical).Version info
26.7.1.535(included in26.7and later)