Custom variant type

[kkaefer]

As discussed with @kkaefer and @artemp - we should assess writing our own C++ variant implementation:

• boost::variant is our only boost dependency right now, and dropping it would ease setup
• we might be able to specialize for our limited use cases and to c++11, allowing for potentially more flexible or faster code (though very unlikely).
• even if we can't write a faster or more memory efficient variant, then it might at least save on final object size.

@artemp has made great progress on a prototype and has started benchmarking. Code is at https://github.com/artemp/variant

Next actions:

• - @springmeyer helps test/benchmark
• - @springmeyer assess object code size differences
• - @artemp continues to optimize and adds missing features (binary visitor, recursive support?)

[DennisOSRM]

Nice

[kkaefer]

Cool stuff!

Can we add support for cross-type comparisons, e.g. (std::string) "1" == (int) 1 == (double) 1?

[artemp]

Yes, we should.

Note to myself : https://www.inkling.com/read/javascript-definitive-guide-david-flanagan-6th/chapter-3/type-conversions

[DennisOSRM]

We should make it obey the never empty guarantee, too.

[artemp]

@DennisOSRM - yes, on my list, but not a trivial thing:) ^^

[DennisOSRM]

@artemp Cool, let me know if I can help

[kkaefer]

Actually... there may be instances where empty is a valid state (like a null state).

[DennisOSRM]

Empty or undefined? Empty is ok, undefined is evil.

[artemp]

@DennisOSRM - agreed. It's already initialised to 'invalid_type' so yes to 'empty' at least

[springmeyer]

Latest code is now at https://github.com/artemp/variant

I've added a Makefile target called sizes that compiles the entire variant.hpp and boost/variant.hpp headers to give a sense of the maximum possible object size you might eventually use, and two extremely minimal test files that just work with a single variant object. The minimal tests indicate that boost::variant compiles down to the same small size and that the object size may be more influenced by other headers than the variant code. The full header compile tests show that boost::variant code is much larger. Hard to say which is more meaningful for me without further investigation:

$ make sizes /Users/dane/projects/mapnik-packaging/osx/out/build-cpp11-libcpp-x86_64/include/boost/variant.hpp 
2.1M    /tmp/variant.out
 12M    /tmp/boost-variant.out
 12K    ./test/variant
 12K    ./test/boost-variant

[springmeyer]

I'll add: so I think object size is not looking like an important factor to push on this. But this custom variant is slightly faster (which is remarkable since its not yet heavily optimized) and would allow us to avoid the boost dependency, so it seems worthwhile on those accounts to keep pushing on this. Here are the timing results I'm seeing on a 2.8 GHz i7 mac with make test:

custom variant:  1.816887s wall, 1.240000s user + 0.560000s system = 1.800000s CPU (99.1%)
boost variant:   1.910785s wall, 1.350000s user + 0.570000s system = 1.920000s CPU (100.5%)
custom variant:  1.800957s wall, 1.240000s user + 0.560000s system = 1.800000s CPU (99.9%)
boost variant:   1.902760s wall, 1.340000s user + 0.560000s system = 1.900000s CPU (99.9%)
custom variant:  1.827337s wall, 1.250000s user + 0.570000s system = 1.820000s CPU (99.6%)
boost variant:   1.905020s wall, 1.340000s user + 0.570000s system = 1.910000s CPU (100.3%)

[springmeyer]

Ubuntu precise/g++-4.8 results:

$ cat /proc/cpuinfo | grep 'model name' |uniq
model name  : Intel(R) Xeon(R) CPU E5-2650 0 @ 2.00GHz

Benchmark:

./test-variant 5000000
custom variant:  3.225717s wall, 1.340000s user + 1.880000s system = 3.220000s CPU (99.8%)
boost variant:   3.358127s wall, 1.430000s user + 1.920000s system = 3.350000s CPU (99.8%)
custom variant:  3.139384s wall, 1.320000s user + 1.830000s system = 3.150000s CPU (100.3%)
boost variant:   3.310069s wall, 1.270000s user + 2.040000s system = 3.310000s CPU (100.0%)
custom variant:  3.140561s wall, 1.170000s user + 1.960000s system = 3.130000s CPU (99.7%)
boost variant:   3.271389s wall, 1.430000s user + 1.850000s system = 3.280000s CPU (100.3%)

sizes:

$ make sizes /home/ubuntu/mapnik-packaging/osx/out/build-cpp11-libstdcpp-gcc-x86_64/include/boost/variant.hpp
25M /tmp/variant.out
100M    /tmp/boost-variant.out
12K ./test/variant
12K ./test/boost-variant

[springmeyer]

More findings. At least with clang++ on OS X I can further shrink the final binary for the minimal test by forcing inlining.

Using inline __attribute__((always_inline)) drops the ./test/variant.cpp from 12k to 8k and helps slightly with perf, allowing some runs of the custom variant to drop below 1.8s:

./test-variant 5000000
custom variant:  1.783703s wall, 1.220000s user + 0.560000s system = 1.780000s CPU (99.8%)
boost variant:   1.890636s wall, 1.320000s user + 0.560000s system = 1.880000s CPU (99.4%)
custom variant:  1.788972s wall, 1.230000s user + 0.560000s system = 1.790000s CPU (100.1%)
boost variant:   1.954468s wall, 1.390000s user + 0.560000s system = 1.950000s CPU (99.8%)
custom variant:  1.847473s wall, 1.260000s user + 0.590000s system = 1.850000s CPU (100.1%)
boost variant:   1.919025s wall, 1.360000s user + 0.560000s system = 1.920000s CPU (100.1%)

[springmeyer]

Now hitting as fast as 1.74s with mapbox/variant@cebd6a3:

./test-variant 5000000
custom variant:  1.833468s wall, 1.260000s user + 0.560000s system = 1.820000s CPU (99.3%)
boost variant:   1.865846s wall, 1.320000s user + 0.550000s system = 1.870000s CPU (100.2%)
custom variant:  1.746228s wall, 1.200000s user + 0.550000s system = 1.750000s CPU (100.2%)
boost variant:   1.896906s wall, 1.350000s user + 0.540000s system = 1.890000s CPU (99.6%)
custom variant:  1.754098s wall, 1.210000s user + 0.550000s system = 1.760000s CPU (100.3%)
boost variant:   1.861776s wall, 1.310000s user + 0.540000s system = 1.850000s CPU (99.4%)

[kkaefer]

@artemp I can't access your repo.

[kkaefer]

@springmeyer yeah, boost variant didn't give us a big binary size increase; I tested that before switching to it.

[DennisOSRM]

I turned off multi-threaded which distorts the results. I had a look at the performance experiments and I am seeing more of a mixed result. The performance difference comes from constructing the std::string. Investigating further

[springmeyer]

Yeah, the multi-threading I added was purely to ensure things didn't crash with threaded load, not intended as meaningful for perf - sorry should not have left that in there.

[springmeyer]

I think this is ready to merge. No need to optimize more.

I've confirmed that the branch with @artemp's variant works (builds fine, app runs well) and also switching back to boost::variant is easy:

diff --git a/include/llmr/style/value.hpp b/include/llmr/style/value.hpp
index 2e9d661..b90f824 100644
--- a/include/llmr/style/value.hpp
+++ b/include/llmr/style/value.hpp
@@ -1,12 +1,12 @@
 #ifndef LLMR_STYLE_VALUE
 #define LLMR_STYLE_VALUE

-#include <llmr/util/variant.hpp>
+#include <boost/variant.hpp>
 #include <llmr/util/pbf.hpp>

 namespace llmr {

-typedef ::util::variant<bool, int64_t, uint64_t, double, std::string> Value;
+typedef boost::variant<bool, int64_t, uint64_t, double, std::string> Value;

 Value parseValue(pbf data); 

While ideally we won't ever need to switch back to boost::variant, having this easy to do will offer a fallback if we need to quickly dodge any unforeseen bugs.

[DennisOSRM]

👍