systemed / tilemaker

Make OpenStreetMap vector tiles without the stack
https://tilemaker.org/
Other
1.42k stars 228 forks source link

use vtzero instead of libprotobuf #625

Closed cldellow closed 7 months ago

cldellow commented 7 months ago

Replaces #624

Switch from libprotobuf to vtzero, and fix a few places where we copied the geometries during writing.

I expect a roughly 7% speedup based on the measurements in #624.

I don't use --merge, so my testing of the --merge mode was somewhat artificial. I built an mbtiles, then built it again with --merge and checked that features seemed duplicated.

systemed commented 7 months ago

Handy speedup, plus it makes the Mac build, in particular, much simpler (since Google helpfully changed protobuf to depend on Abseil, which unleashed the usual forces of Homebrew dependency hell).

I ran a quick test with --merge with the (adjacent) Gloucestershire and Oxfordshire extracts and that looks good.

Timings/memory for GB compared to current master (with protozero merged):

master (with --lazy-geometries):
    Elapsed (wall clock) time (h:mm:ss or m:ss): 5:03.94
    Maximum resident set size (kbytes): 9133392
master (without):
    Elapsed (wall clock) time (h:mm:ss or m:ss): 4:51.17
    Maximum resident set size (kbytes): 12174424

vtzero branch (with):
    Elapsed (wall clock) time (h:mm:ss or m:ss): 4:50.95
    Maximum resident set size (kbytes): 9154964
vtzero branch (without):
    Elapsed (wall clock) time (h:mm:ss or m:ss): 4:44.34
    Maximum resident set size (kbytes): 12307348

There is a very slight uptick in RAM usage - it's probably nothing (just an artefact of whatever vtzero does) but I thought I'd mention it in case there's any chance of a memory leak. But I couldn't see anything obvious on scanning the code.

cldellow commented 7 months ago

Thanks for measuring the memory - I was thinking I should do that, but didn't get around to it. :)

I suspect it could be explainable by how vtzero tracks the strings for key/values.

The old code maintained a vector of keys and a vector of values, and did a linear search to translate from value to index. Very memory efficient, and very fast when the total number of distinct values was less than, say, 100 items. I had noticed a few degenerate cases in the old code where a tile had > 1000 POIs and would take a few seconds to do the lookups.

By contrast, vtzero's default strategy is to make a std::map to track things. Much more consistent runtime, but with (relatively) much more memory overhead.

systemed commented 7 months ago

That would make sense. That suggests the impact shouldn't balloon with bigger areas - it's only 0.2% anyway so nothing really to worry about. Thanks.

I'll merge this and look at simplifying the Mac install in due course!

cldellow commented 7 months ago

Ah, this broke the docker build when merged to master. I'll see if I can figure out why, and maybe also send a PR to enable docker builds (but not pushes) in PRs.