Use vector-tile C++ library

[kkaefer]

This pull request replaces our custom vector tile parsing code with the vector-tile C++ library.

Addresses the problem in #9167 by changing the GeometryTileData interface to return owning handles to GeometryTileLayer, which we can then retain in SymbolLayout so that the SymbolFeature handles don't end up with dangling handles. I opted not to make GeometryTileFeature handles fully owning their data due to possible overhead of a shared_ptr.

/cc @springmeyer

[springmeyer]

Is the reason this is not GeometryCollection && geometries_, is that clang-tidy claims that copying is faster here than explicitly asking for a move?

[kkaefer]

No, just a leftover from a previous implementation.

[springmeyer]

So do you think it should be GeometryCollection && to avoid the extra copy? If so, I presume that would require a separate refactor?

[springmeyer]

Looks great @kkaefer 🎉

[jfirebaugh]

Do we have any way to quantify the performance impact here?

[anandthakker]

I opted not to make GeometryTileFeature handles fully owning their data due to possible overhead of a shared_ptr

Since this reintroduces the implicit requirement that objects retaining a GeometryTileFeature must also retain its owning GeometryTileLayer, should we document that somewhere? I remember that when I was first working on #7939, this was not immediately obvious to me from scanning the code.

[kkaefer]

This will fail due to mapbox/vector-tile#19

I added a benchmark and these are the results:

before

Benchmark	Time	CPU
Parse_VectorTile	2.598.133 ns	2.589.664 ns
Parse_VectorTile	2.562.716 ns	2.558.520 ns
Parse_VectorTile	2.498.893 ns	2.496.094 ns
Parse_VectorTile	2.645.567 ns	2.637.008 ns
Parse_VectorTile	2.658.438 ns	2.649.066 ns
Parse_VectorTile	2.628.378 ns	2.620.168 ns
Parse_VectorTile	2.573.529 ns	2.567.988 ns
Parse_VectorTile	2.570.820 ns	2.567.393 ns
Parse_VectorTile	2.574.151 ns	2.568.861 ns
Parse_VectorTile	2.560.927 ns	2.555.828 ns
Parse_VectorTile_mean	2.587.155 ns	2.581.059 ns
Parse_VectorTile_stddev	44.733 ns	42.695 ns

after

Benchmark	Time	CPU
Parse_VectorTile	1.922.978 ns	1.908.669 ns
Parse_VectorTile	2.032.030 ns	1.994.519 ns
Parse_VectorTile	1.943.548 ns	1.920.645 ns
Parse_VectorTile	1.960.098 ns	1.936.279 ns
Parse_VectorTile	1.992.238 ns	1.973.358 ns
Parse_VectorTile	1.939.405 ns	1.931.519 ns
Parse_VectorTile	1.919.359 ns	1.916.416 ns
Parse_VectorTile	1.904.928 ns	1.902.651 ns
Parse_VectorTile	1.930.251 ns	1.927.384 ns
Parse_VectorTile	1.891.877 ns	1.888.255 ns
Parse_VectorTile_mean	1.943.671 ns	1.929.970 ns
Parse_VectorTile_stddev	39.796 ns	30.510 ns

[springmeyer]

Great to see these perf improvements @kkaefer 🎉 Can you tell if the improvements are due to addressing #9167 or due to the upgraded vector tile decoding code? If it is the latter I would suspect/hope that it is attributable to either how we now pre-allocate arrays for geometries or the new deferred parsing of feature values until accessed (rather than at layer parse).

[kkaefer]

I ran Instruments, and it by far the dominating part is geometry parsing, and it looks like it's spending less time on that now.

[springmeyer]

I ran Instruments, and it by far the dominating part is geometry parsing, and it looks like it's spending less time on that now.

👍

@kkaefer I would recommend adding getProperties or getValue calls to the benchmark as well. The deferred parseValue will then get invoked there. And your benchmark would be more apples to apples to previously.

[kkaefer]

Added getProperties() to the test, and seeing a 3ms -> 2.3ms improvement.

[springmeyer]

Added getProperties() to the test, and seeing a 3ms -> 2.3ms improvement.

👍 So, with getProperties() results were ~3ms before and ~2.3 after the changes in this branch? Assuming I'm understanding, that is great news, and confirms we are going to be overall faster.