-
Notifications
You must be signed in to change notification settings - Fork 97
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
TileCoord supports up to zoom 15 using alternate ordering #266
Conversation
https://github.com/onthegomap/planetiler/actions/runs/2647800454 ℹ️ Base Logs 4b7a601
ℹ️ This Branch Logs 5ab10cb
|
Thanks for implementing this! A couple of concerns I'll want to mitigate before merging this:
Also, do you have a link to the C++ code you ported? Want to check if there's any license/attribution requirements. |
Also for the "Check formatting" failure - if you run |
Thanks, I'll get started on these tasks. The implementation is part of my own C++ library which is not on GitHub. |
Took a quick look for potential issues I can see with tiles not ordered by z/x/y:
If there's any reason we still might want z/x/y ordering then it might make sense to extract the ordering into a strategy you could swap out with a command-line flag. If there are no drawbacks of hilbert over z/x/y though then we don't need that. Nothing before tile writing should care about tile ordering. |
will start a new run tonight to get clean log output and post here, to address the rest:
|
planetiler-core/src/test/java/com/onthegomap/planetiler/PlanetilerTests.java
Outdated
Show resolved
Hide resolved
Re: performance:
So it might make sense to just compute the encoded value on-demand instead of storing on the |
planetiler-core/src/main/java/com/onthegomap/planetiler/geo/TileCoord.java
Outdated
Show resolved
Hide resolved
run of
|
Branch planet run output:
|
MBTiles benchmark for BRANCH:
sqlite3_analyzer output for BRANCH: https://gist.github.com/bdon/0022346493786e58e12b4300bfd443ee |
MBTiles benchmark for MAIN:
sqlite3_analyzer output for MAIN: https://gist.github.com/bdon/fe0111a676d8ba6bf432538cd6b69344 |
Thanks @bdon, looks like hilbert ordering is a tiny bit slower on random reads but that might not be a very accurate test. Seems like hilbert ordering is meant to help with more realistic traffic distribution on machine with less RAM. What have you done in the past to test pmtiles reads with a more realistic traffic distribution? |
I've only seen the gains from the ordering in serving latency from production traffic, but a reproducible benchmark would be good to add to https://github.com/bdon/TileSiege ; I think if we could get an anonymized day of real traffic that would be a more effective benchmark for testing query latency. It does look like the hilbert order also ends up with less full pages so the db ends up larger, but I wasn't running it with the vacuum DB option (is that the default?) |
Tested on a
And also tested reads on the
And a memory-constrained
|
SonarCloud Quality Gate failed. |
New TMS ordering run on same machine:
|
Kudos, SonarCloud Quality Gate passed! |
1 similar comment
Kudos, SonarCloud Quality Gate passed! |
This is a rough idea right now (don't merge yet) to explore alternate TileCoord encodings. it deserves some better tests for edge cases as right now it's blindly ported from C++ to java
The current TileCoord uses a fixed 14 bits for X and Y and the remaining bits for Z. This instead defines a mapping from a 32-bit unsigned encoded integer to X,Y,Z like this:
Tile 0 is
(0,0)
on z0Tile 1-4 are positions on the z1 Hilbert curve; each side of the curve is length 2, so tile 1 is
(0,0)
, tile 2 is(0,1)
, tile 3 is(1,1)
and tile 4 is(1,0)
Tile 5-20 are positions on the z2 Hilbert curve
etc...
Drawbacks:
Potential benefits of doing it this way: