From 03ca19f70eed130a26feccfa441b6e03150b6624 Mon Sep 17 00:00:00 2001 From: Mickey Rose Date: Fri, 3 Feb 2017 20:10:37 +0100 Subject: [PATCH 1/2] add large tree-variant benchmark --- test/recursive_wrapper_test.cpp | 27 ++++++++++++++++++++++++ test/unique_ptr_test.cpp | 37 +++++++++++++++++++++++++++++++-- 2 files changed, 62 insertions(+), 2 deletions(-) diff --git a/test/recursive_wrapper_test.cpp b/test/recursive_wrapper_test.cpp index 57143cb..e4f925d 100644 --- a/test/recursive_wrapper_test.cpp +++ b/test/recursive_wrapper_test.cpp @@ -92,6 +92,30 @@ struct to_string } }; +void bench_large_expression(std::size_t num) +{ + std::cerr << "----- sum of " << num << " ones -----" << std::endl; + expression sum = 0; + { + std::cerr << "construction "; + auto_cpu_timer t; + for (std::size_t i = 0; i < num; ++i) + { + sum = binary_op(std::move(sum), 1); + } + } + int total = 0; + { + std::cerr << "calculation "; + auto_cpu_timer t; + for (std::size_t i = 0; i < num; ++i) + { + total += util::apply_visitor(calculator(), sum); + } + } + std::cerr << "total=" << total << std::endl; +} + } // namespace test int main(int argc, char** argv) @@ -121,5 +145,8 @@ int main(int argc, char** argv) std::cerr << util::apply_visitor(test::to_string(), result) << "=" << util::apply_visitor(test::calculator(), result) << std::endl; + test::bench_large_expression(1000); + test::bench_large_expression(5000); + return EXIT_SUCCESS; } diff --git a/test/unique_ptr_test.cpp b/test/unique_ptr_test.cpp index f075900..baf4214 100644 --- a/test/unique_ptr_test.cpp +++ b/test/unique_ptr_test.cpp @@ -93,6 +93,36 @@ struct to_string } }; +template > +std::unique_ptr make_binary(expression&& lhs, expression&& rhs) +{ + return std::unique_ptr(new Node(std::move(lhs), std::move(rhs))); +} + +void bench_large_expression(std::size_t num) +{ + std::cerr << "----- sum of " << num << " ones -----" << std::endl; + expression sum = 0; + { + std::cerr << "construction "; + auto_cpu_timer t; + for (std::size_t i = 0; i < num; ++i) + { + sum = make_binary(std::move(sum), 1); + } + } + int total = 0; + { + std::cerr << "calculation "; + auto_cpu_timer t; + for (std::size_t i = 0; i < num; ++i) + { + total += util::apply_visitor(calculator(), sum); + } + } + std::cerr << "total=" << total << std::endl; +} + } // namespace test int main(int argc, char** argv) @@ -105,8 +135,8 @@ int main(int argc, char** argv) const std::size_t NUM_ITER = static_cast(std::stol(argv[1])); - test::expression sum(std::unique_ptr>(new test::binary_op(2, 3))); - test::expression result(std::unique_ptr>(new test::binary_op(std::move(sum), 4))); + test::expression sum = test::make_binary(2, 3); + test::expression result = test::make_binary(std::move(sum), 4); std::cerr << "TYPE OF RESULT-> " << util::apply_visitor(test::test(), result) << std::endl; @@ -122,5 +152,8 @@ int main(int argc, char** argv) std::cerr << util::apply_visitor(test::to_string(), result) << "=" << util::apply_visitor(test::calculator(), result) << std::endl; + test::bench_large_expression(1000); + test::bench_large_expression(5000); + return EXIT_SUCCESS; } From 57d9c7d1af41dabce2e68c8b92a1deab6f3f35dd Mon Sep 17 00:00:00 2001 From: Mickey Rose Date: Mon, 6 Feb 2017 16:11:16 +0100 Subject: [PATCH 2/2] reset moved-from variant to valueless state after the move This is the simplest way to achieve constant-time[1] complexity of variant move construction and assignment. [1] Constant with respect to the size of the recursive structure held by the variant object; it's still linear in the number of alternative types in the variant type. --- include/mapbox/recursive_wrapper.hpp | 19 +++++-------------- include/mapbox/variant.hpp | 3 +++ test/t/recursive_wrapper.cpp | 11 ++--------- test/t/variant.cpp | 1 - 4 files changed, 10 insertions(+), 24 deletions(-) diff --git a/include/mapbox/recursive_wrapper.hpp b/include/mapbox/recursive_wrapper.hpp index 4ffcbd7..407ee26 100644 --- a/include/mapbox/recursive_wrapper.hpp +++ b/include/mapbox/recursive_wrapper.hpp @@ -32,18 +32,6 @@ class recursive_wrapper public: using type = T; - /** - * Default constructor default initializes the internally stored value. - * For POD types this means nothing is done and the storage is - * uninitialized. - * - * @throws std::bad_alloc if there is insufficient memory for an object - * of type T. - * @throws any exception thrown by the default constructur of T. - */ - recursive_wrapper() - : p_(new T){} - ~recursive_wrapper() noexcept { delete p_; } recursive_wrapper(recursive_wrapper const& operand) @@ -52,8 +40,11 @@ class recursive_wrapper recursive_wrapper(T const& operand) : p_(new T(operand)) {} - recursive_wrapper(recursive_wrapper&& operand) - : p_(new T(std::move(operand.get()))) {} + recursive_wrapper(recursive_wrapper&& operand) noexcept + : p_(operand.p_) + { + operand.p_ = nullptr; + } recursive_wrapper(T&& operand) : p_(new T(std::move(operand))) {} diff --git a/include/mapbox/variant.hpp b/include/mapbox/variant.hpp index 9b73934..bb2ba5a 100644 --- a/include/mapbox/variant.hpp +++ b/include/mapbox/variant.hpp @@ -235,6 +235,7 @@ struct variant_helper if (old_type_index == sizeof...(Types)) { new (new_value) T(std::move(*reinterpret_cast(old_value))); + reinterpret_cast(old_value)->~T(); } else { @@ -613,6 +614,7 @@ class variant : type_index(old.type_index) { helper_type::move(old.type_index, &old.data, &data); + old.type_index = detail::invalid_value; } private: @@ -630,6 +632,7 @@ class variant type_index = detail::invalid_value; helper_type::move(rhs.type_index, &rhs.data, &data); type_index = rhs.type_index; + rhs.type_index = detail::invalid_value; } public: diff --git a/test/t/recursive_wrapper.cpp b/test/t/recursive_wrapper.cpp index c3d53f0..dde9669 100644 --- a/test/t/recursive_wrapper.cpp +++ b/test/t/recursive_wrapper.cpp @@ -27,7 +27,7 @@ TEST_CASE("recursive wrapper of int") rwi b{a}; REQUIRE(b.get() == 8); - rwi c; + rwi c{-1}; c = b; REQUIRE(b.get() == 8); REQUIRE(c.get() == 8); @@ -114,13 +114,6 @@ TEST_CASE("swap") TEST_CASE("recursive wrapper of pair") { - SECTION("default constructed") - { - rwp a; - REQUIRE(a.get().first == 0); - REQUIRE(a.get().second == 0); - } - SECTION("construct with value") { rwp a{std::make_pair(1, 2)}; @@ -139,7 +132,7 @@ TEST_CASE("recursive wrapper of pair") REQUIRE(b.get().first == 3); REQUIRE(b.get().second == 4); - rwp c; + rwp c{{-1, -2}}; c = b; REQUIRE(b.get().first == 3); REQUIRE(b.get().second == 4); diff --git a/test/t/variant.cpp b/test/t/variant.cpp index b4322a2..71223e5 100644 --- a/test/t/variant.cpp +++ b/test/t/variant.cpp @@ -29,7 +29,6 @@ TEST_CASE("variant can be moved into vector", "[variant]") variant_type v(std::string("test")); std::vector vec; vec.emplace_back(std::move(v)); - REQUIRE(v.get() != std::string("test")); REQUIRE(vec.at(0).get() == std::string("test")); }