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

Simplify and optimize Hilbert tile ID <-> XYZ conversion #527

Merged
merged 4 commits into from
Feb 21, 2025

Conversation

ciscorn
Copy link
Contributor

@ciscorn ciscorn commented Feb 14, 2025

I found that we can simplify and optimize the conversion between XYZ tile coordinates and the Hilbert tile IDs.

This optimization is primarily based on the fact that the tile IDs for (zi, 0, 0) are given by the sum of a geometric series with a common ratio of 4.

Related PR: protomaps/go-pmtiles#208

@ciscorn ciscorn force-pushed the optimize-tileid-conversion branch 2 times, most recently from 3b7fab0 to 41a3e68 Compare February 14, 2025 06:51
@ciscorn ciscorn force-pushed the optimize-tileid-conversion branch from 41a3e68 to f256fa6 Compare February 14, 2025 07:03
@bdon
Copy link
Member

bdon commented Feb 14, 2025

Same with #470 - please provide some evidence or benchmark this is faster across different JavaScript engines, otherwise we will just have other PRs optimizing it in different ways (iterative, lookup table, etc)

@ciscorn
Copy link
Contributor Author

ciscorn commented Feb 14, 2025

Sorry for unintentionally creating a duplicate PR. I’ll take some time later to run benchmarks.

@ciscorn ciscorn force-pushed the optimize-tileid-conversion branch from 0a86ca6 to 20df0e5 Compare February 17, 2025 03:40
@ciscorn
Copy link
Contributor Author

ciscorn commented Feb 17, 2025

@bdon

I have now benchmarked the JavaScript implementation across multiple runtimes and browsers.

Benchmarking script: https://gist.github.com/ciscorn/e84d41fe5c7ac7c1857d8f64fc1df069

Results on my M1 Apple Silicon:

Chrome 133

zxyToTileId: Original=2665.60(ms), Modified=42.50(ms) (62.72x faster)
tileIdToZxy: Original=548.10(ms), Modified=381.60(ms) (1.44x faster)

Firefox 125

zxyToTileId: Original=2677.00(ms), Modified=47.00(ms) (56.96x faster)
tileIdToZxy: Original=942.00(ms), Modified=555.00(ms) (1.70x faster)

NodeJS 22.12

zxyToTileId: Original=2760.29(ms), Modified=40.69(ms) (67.83x faster)
tileIdToZxy: Original=625.26(ms), Modified=384.92(ms) (1.62x faster)

Bun 1.2.2

zxyToTileId: Original=1483.75(ms), Modified=80.07(ms) (18.53x faster)
tileIdToZxy: Original=1003.57(ms), Modified=702.73(ms) (1.43x faster)

Deno 2.1.10

zxyToTileId: Original=2730.63(ms), Modified=40.57(ms) (67.30x faster)
tileIdToZxy: Original=646.00(ms), Modified=380.45(ms) (1.70x faster)

@bdon bdon merged commit 9b69ab4 into protomaps:main Feb 21, 2025
2 checks passed
@bdon
Copy link
Member

bdon commented Feb 21, 2025

Released in https://www.npmjs.com/package/pmtiles 4.3.0, thanks!

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