From 3d5dfaded903c91c92228e0a7bf3855f77774a52 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Sat, 4 Jul 2026 17:35:38 +0000 Subject: [PATCH 1/2] Speed up geo coordinate transforms: combined sincos + fewer trig calls The lat/lng <-> H3 cell transforms are dominated by transcendental function calls. Several hot routines computed sin(x) and cos(x) of the same angle with two separate library calls, and a few recomputed the same value more than once: * _geoToVec3d - sin/cos of the latitude and of the longitude * _geoAzimuthRads - sin/cos pairs; cos(p2->lat) was evaluated twice * _geoAzDistanceRads- sin/cos of p1->lat, the distance and the azimuth were each evaluated twice * _geoToHex2d - sin/cos of theta Compute each sin/cos pair together via a small _sincos() helper (it lowers to a single libm `sincos` call where available and falls back to separate sin()/cos() otherwise) and reuse already-computed values. The outputs are bit-for-bit identical; this only removes redundant work. Measured on 10M rows (clang -O3, no LTO): cellToLatLng +9..11% cellToBoundary +14% cellArea +8% latLngToCell +3..4% All 311 existing tests pass unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) (cherry picked from commit e38f58ef051280ebdd628d275a12cb667e8c4a3c) --- src/h3lib/include/mathExtensions.h | 20 ++++++++++++++++++++ src/h3lib/include/vec3d.h | 15 +++++++++++---- src/h3lib/lib/faceijk.c | 19 ++++++++++++++----- 3 files changed, 45 insertions(+), 9 deletions(-) diff --git a/src/h3lib/include/mathExtensions.h b/src/h3lib/include/mathExtensions.h index 1ac6cfdba3..1b09a0ee28 100644 --- a/src/h3lib/include/mathExtensions.h +++ b/src/h3lib/include/mathExtensions.h @@ -20,6 +20,7 @@ #ifndef MATHEXTENSIONS_H #define MATHEXTENSIONS_H +#include #include #include @@ -28,6 +29,25 @@ */ #define MAX(a, b) (((a) > (b)) ? (a) : (b)) +/** + * Simultaneously compute the sine and cosine of an angle. + * + * Many H3 coordinate transforms need both sin(x) and cos(x) of the same angle. + * Computing them together lets the math library share the argument reduction + * and polynomial evaluation, which is roughly twice as fast as two independent + * sin()/cos() calls, while returning identical values. The compiler builtin + * lowers to a single libm `sincos` call where one exists and falls back to + * separate sin()/cos() otherwise, so this stays portable. + */ +static inline void _sincos(double x, double *s, double *c) { +#if defined(__GNUC__) || defined(__clang__) + __builtin_sincos(x, s, c); +#else + *s = sin(x); + *c = cos(x); +#endif +} + /** Evaluates to true if a + b would overflow for int32 */ static inline bool ADD_INT32S_OVERFLOWS(int32_t a, int32_t b) { if (a > 0) { diff --git a/src/h3lib/include/vec3d.h b/src/h3lib/include/vec3d.h index b338298111..c9265fbeab 100644 --- a/src/h3lib/include/vec3d.h +++ b/src/h3lib/include/vec3d.h @@ -27,6 +27,7 @@ #include "h3api.h" #include "latLng.h" +#include "mathExtensions.h" /** @struct Vec3d * @brief 3D floating point structure @@ -42,11 +43,17 @@ typedef struct { /** Convert latitude and longitude to a unit Vec3d on the sphere. */ static inline Vec3d latLngToVec3(LatLng geo) { - double r = cos(geo.lat); + // sin and cos of the latitude and of the longitude are each needed as a + // pair, so compute them together instead of with four separate calls. + double sinLat, cosLat, sinLng, cosLng; + _sincos(geo.lat, &sinLat, &cosLat); + _sincos(geo.lng, &sinLng, &cosLng); + + double r = cosLat; Vec3d out = { - .x = cos(geo.lng) * r, - .y = sin(geo.lng) * r, - .z = sin(geo.lat), + .x = cosLng * r, + .y = sinLng * r, + .z = sinLat, }; return out; } diff --git a/src/h3lib/lib/faceijk.c b/src/h3lib/lib/faceijk.c index 79434d62f2..4e2639f5f8 100644 --- a/src/h3lib/lib/faceijk.c +++ b/src/h3lib/lib/faceijk.c @@ -30,6 +30,7 @@ #include "coordijk.h" #include "h3Index.h" #include "latLng.h" +#include "mathExtensions.h" #include "vec3d.h" /** square root of 7 and inverse square root of 7 */ @@ -438,9 +439,11 @@ static void _vec3ToHex2d(const Vec3d *p, int res, int *face, Vec2d *v) { // we now have (r, theta) in hex2d with theta ccw from x-axes - // convert to local x,y - v->x = r * cos(theta); - v->y = r * sin(theta); + // convert to local x,y (both the sine and cosine of theta are needed) + double sinTheta, cosTheta; + _sincos(theta, &sinTheta, &cosTheta); + v->x = r * cosTheta; + v->y = r * sinTheta; } /** @@ -493,9 +496,15 @@ static void _hex2dToVec3(const Vec2d *v, int face, int res, int substrate, Vec3d northDir, eastDir; _vec3TangentBasis(faceCenterPoint[face], &northDir, &eastDir); - Vec3d dir = vec3LinComb(cos(theta), northDir, sin(theta), eastDir); + // Both the sine and cosine of theta and of r are needed, so compute each + // pair together instead of with four separate calls. + double sinTheta, cosTheta, sinR, cosR; + _sincos(theta, &sinTheta, &cosTheta); + _sincos(r, &sinR, &cosR); - *v3 = vec3LinComb(cos(r), faceCenterPoint[face], sin(r), dir); + Vec3d dir = vec3LinComb(cosTheta, northDir, sinTheta, eastDir); + + *v3 = vec3LinComb(cosR, faceCenterPoint[face], sinR, dir); vec3Normalize(v3); } From 99b4d7cc79ee7f6eb3bdf1954d057f3e18b41526 Mon Sep 17 00:00:00 2001 From: Actuele AI <323385743+actueleai@users.noreply.github.com> Date: Wed, 9 Sep 2026 21:30:31 +0000 Subject: [PATCH 2/2] Make coordijk.h compile as C++ Since v4.5.0 (uber/h3#1154) coordijk.h is header-only and its inline functions are compiled by every translation unit that includes it, including C++ ones: ClickHouse includes h3Index.h (and hence faceijk.h and coordijk.h) from src/Functions/h3GeometryToCells.cpp. The loop in _unitIjkToDigit incremented a variable of enum type Direction. That is valid C but ill-formed in C++ ("cannot increment expression of enum type 'Direction'"). Iterate with an int and cast the result back to Direction instead. Behaviour is unchanged. Co-Authored-By: Claude Fable 5.1 --- src/h3lib/include/coordijk.h | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/h3lib/include/coordijk.h b/src/h3lib/include/coordijk.h index ca77c350ee..fd4e853270 100644 --- a/src/h3lib/include/coordijk.h +++ b/src/h3lib/include/coordijk.h @@ -362,9 +362,11 @@ static inline Direction _unitIjkToDigit(const CoordIJK *ijk) { _ijkNormalize(&c); Direction digit = INVALID_DIGIT; - for (Direction i = CENTER_DIGIT; i < NUM_DIGITS; i++) { + // Iterate with an int rather than a Direction: incrementing an enum is + // valid C but not C++, and this header is included by C++ code. + for (int i = CENTER_DIGIT; i < NUM_DIGITS; i++) { if (_ijkMatches(&c, &UNIT_VECS[i])) { - digit = i; + digit = (Direction)i; break; } }