This is an automated email from the ASF dual-hosted git repository. alsay pushed a commit to branch optional in repository https://gitbox.apache.org/repos/asf/datasketches-cpp.git
commit 7c4ee32380dc362f2cb7c07a04efb87b7e0522d3 Author: AlexanderSaydakov <[email protected]> AuthorDate: Thu Jul 13 13:03:14 2023 -0700 simple substitute for std::optional until we require C++17 --- common/include/optional.hpp | 106 +++++++++++++++++++++++++++++++ common/test/CMakeLists.txt | 1 + common/test/optional_test.cpp | 64 +++++++++++++++++++ req/include/req_sketch.hpp | 8 +-- req/include/req_sketch_impl.hpp | 136 ++++++++++++++-------------------------- 5 files changed, 222 insertions(+), 93 deletions(-) diff --git a/common/include/optional.hpp b/common/include/optional.hpp new file mode 100644 index 0000000..543bf41 --- /dev/null +++ b/common/include/optional.hpp @@ -0,0 +1,106 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#ifndef _OPTIONAL_HPP_ +#define _OPTIONAL_HPP_ + +// This is a simplistic substitute for std::optional until we require C++17 + +#if (__cplusplus >= 201703L || (defined(_MSVC_LANG) && _MSVC_LANG >= 201703L)) +#include <optional> +using std::optional; +#else + +#include <type_traits> + +namespace datasketches { + +template<typename T> +class optional { +public: + + optional() noexcept: initialized_(false) {} + + optional(const T& value) { + new (&value_) T(value); + initialized_ = true; + } + + optional(T&& value) noexcept(std::is_nothrow_move_constructible<T>::value) { + new (&value_) T(std::move(value)); + initialized_ = true; + } + + template<typename TT> + optional(const TT& other) { + if (other.initialized_) { + new (&value_) T(other.value_); + initialized_ = true; + } + } + + optional(optional&& other) noexcept(std::is_nothrow_move_constructible<T>::value) { + if (other.initialized_) { + new (&value_) T(std::move(other.value_)); + initialized_ = true; + } + } + + ~optional() noexcept(std::is_nothrow_destructible<T>::value) { + if (initialized_) value_.~T(); + } + + explicit operator bool() const noexcept { + return initialized_; + } + + template<typename... Args> + void emplace(Args&&... args) { + new (&value_) T(args...); + initialized_ = true; + } + + T& operator*() & noexcept { return value_; } + const T& operator*() const & noexcept { return value_; } + T&& operator*() && noexcept { return std::move(value_); } + const T&& operator*() const && noexcept { return std::move(value_); } + + T* operator->() noexcept { return &value_; } + const T* operator->() const noexcept { return &value_; } + + void reset() noexcept(std::is_nothrow_destructible<T>::value) { + if (initialized_) value_.~T(); + initialized_ = false; + } + +private: + union { + T value_; + }; + bool initialized_; + + // for converting constructor + template<typename TT> friend class optional; +}; + +} // namespace + +#endif // C++17 + +#endif // _OPTIONAL_HPP_ diff --git a/common/test/CMakeLists.txt b/common/test/CMakeLists.txt index 1927d26..8691c01 100644 --- a/common/test/CMakeLists.txt +++ b/common/test/CMakeLists.txt @@ -69,6 +69,7 @@ add_test( target_sources(common_test PRIVATE quantiles_sorted_view_test.cpp + optional_test.cpp ) # now the integration test part diff --git a/common/test/optional_test.cpp b/common/test/optional_test.cpp new file mode 100644 index 0000000..53b4a7f --- /dev/null +++ b/common/test/optional_test.cpp @@ -0,0 +1,64 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#include <catch2/catch.hpp> + +#include <iostream> + +#include "optional.hpp" + +namespace datasketches { + +class tt { +public: + tt() = delete; // make sure it cannot be default constructed + tt(int val): val_(val) {} + tt(const tt& other): val_(other.val_) { std::cout << "tt copy constructor\n"; } + tt(tt&& other): val_(other.val_) { std::cout << "tt move constructor\n"; } + tt& operator=(const tt& other) { val_ = other.val_; std::cout << "tt copy assignment\n"; return *this; } + tt& operator=(tt&& other) { val_ = other.val_; std::cout << "tt move assignment\n"; return *this; } + int get_val() const { return val_; } +private: + int val_; +}; + +TEST_CASE("optional", "common") { + optional<tt> opt; + REQUIRE_FALSE(opt); + opt.emplace(5); + REQUIRE(bool(opt)); + REQUIRE((*opt).get_val() == 5); + REQUIRE(opt->get_val() == 5); + opt.reset(); + REQUIRE_FALSE(opt); + + optional<tt> opt2(std::move(opt)); + if (opt2) *opt2 = 6; // good if it is initialized + REQUIRE(opt2->get_val() == 6); + + optional<tt> opt3; + REQUIRE_FALSE(opt3); + *opt3 = 7; // don't do this! may be dangerous for arbitrary T, and it still thinks it is not initialized + REQUIRE_FALSE(opt3); + opt3.emplace(8); + REQUIRE(bool(opt3)); + REQUIRE(opt3->get_val() == 8); +} + +} /* namespace datasketches */ diff --git a/req/include/req_sketch.hpp b/req/include/req_sketch.hpp index 40563a1..09afedb 100755 --- a/req/include/req_sketch.hpp +++ b/req/include/req_sketch.hpp @@ -25,6 +25,7 @@ #include "req_common.hpp" #include "req_compactor.hpp" #include "quantiles_sorted_view.hpp" +#include "optional.hpp" namespace datasketches { @@ -434,8 +435,8 @@ private: uint32_t num_retained_; uint64_t n_; std::vector<Compactor, AllocCompactor> compactors_; - T* min_item_; - T* max_item_; + optional<T> min_item_; + optional<T> max_item_; mutable quantiles_sorted_view<T, Comparator, Allocator>* sorted_view_; void setup_sorted_view() const; // modifies mutable state @@ -462,9 +463,8 @@ private: static bool is_exact_rank(uint16_t k, uint8_t num_levels, double rank, uint64_t n, bool hra); // for deserialization - class item_deleter; req_sketch(uint16_t k, bool hra, uint64_t n, - std::unique_ptr<T, item_deleter> min_item, std::unique_ptr<T, item_deleter> max_item, + optional<T>&& min_item, optional<T>&& max_item, std::vector<Compactor, AllocCompactor>&& compactors, const Comparator& comparator); static void check_preamble_ints(uint8_t preamble_ints, uint8_t num_levels); diff --git a/req/include/req_sketch_impl.hpp b/req/include/req_sketch_impl.hpp index fb11676..7bd30d0 100755 --- a/req/include/req_sketch_impl.hpp +++ b/req/include/req_sketch_impl.hpp @@ -35,8 +35,8 @@ max_nom_size_(0), num_retained_(0), n_(0), compactors_(allocator), -min_item_(nullptr), -max_item_(nullptr), +min_item_(), +max_item_(), sorted_view_(nullptr) { grow(); @@ -44,14 +44,6 @@ sorted_view_(nullptr) template<typename T, typename C, typename A> req_sketch<T, C, A>::~req_sketch() { - if (min_item_ != nullptr) { - min_item_->~T(); - allocator_.deallocate(min_item_, 1); - } - if (max_item_ != nullptr) { - max_item_->~T(); - allocator_.deallocate(max_item_, 1); - } reset_sorted_view(); } @@ -65,13 +57,10 @@ max_nom_size_(other.max_nom_size_), num_retained_(other.num_retained_), n_(other.n_), compactors_(other.compactors_), -min_item_(nullptr), -max_item_(nullptr), +min_item_(other.min_item_), +max_item_(other.max_item_), sorted_view_(nullptr) -{ - if (other.min_item_ != nullptr) min_item_ = new (allocator_.allocate(1)) T(*other.min_item_); - if (other.max_item_ != nullptr) max_item_ = new (allocator_.allocate(1)) T(*other.max_item_); -} +{} template<typename T, typename C, typename A> req_sketch<T, C, A>::req_sketch(req_sketch&& other) noexcept : @@ -83,13 +72,10 @@ max_nom_size_(other.max_nom_size_), num_retained_(other.num_retained_), n_(other.n_), compactors_(std::move(other.compactors_)), -min_item_(other.min_item_), -max_item_(other.max_item_), +min_item_(std::move(other.min_item_)), +max_item_(std::move(other.max_item_)), sorted_view_(nullptr) -{ - other.min_item_ = nullptr; - other.max_item_ = nullptr; -} +{} template<typename T, typename C, typename A> req_sketch<T, C, A>& req_sketch<T, C, A>::operator=(const req_sketch& other) { @@ -135,8 +121,8 @@ max_nom_size_(other.max_nom_size_), num_retained_(other.num_retained_), n_(other.n_), compactors_(allocator), -min_item_(nullptr), -max_item_(nullptr), +min_item_(other.min_item_), +max_item_(other.max_item_), sorted_view_(nullptr) { static_assert( @@ -147,10 +133,6 @@ sorted_view_(nullptr) for (const auto& compactor: other.compactors_) { compactors_.push_back(req_compactor<T, C, A>(compactor, comparator_, allocator_)); } - if (!other.is_empty()) { - min_item_ = new (allocator_.allocate(1)) T(other.get_min_item()); - max_item_ = new (allocator_.allocate(1)) T(other.get_max_item()); - } } template<typename T, typename C, typename A> @@ -188,8 +170,8 @@ template<typename FwdT> void req_sketch<T, C, A>::update(FwdT&& item) { if (!check_update_item(item)) { return; } if (is_empty()) { - min_item_ = new (allocator_.allocate(1)) T(item); - max_item_ = new (allocator_.allocate(1)) T(item); + min_item_.emplace(item); + max_item_.emplace(item); } else { if (comparator_(item, *min_item_)) *min_item_ = item; if (comparator_(*max_item_, item)) *max_item_ = item; @@ -207,8 +189,8 @@ void req_sketch<T, C, A>::merge(FwdSk&& other) { if (is_HRA() != other.is_HRA()) throw std::invalid_argument("merging HRA and LRA is not valid"); if (other.is_empty()) return; if (is_empty()) { - min_item_ = new (allocator_.allocate(1)) T(conditional_forward<FwdSk>(*other.min_item_)); - max_item_ = new (allocator_.allocate(1)) T(conditional_forward<FwdSk>(*other.max_item_)); + min_item_.emplace(conditional_forward<FwdSk>(*other.min_item_)); + max_item_.emplace(conditional_forward<FwdSk>(*other.max_item_)); } else { if (comparator_(*other.min_item_, *min_item_)) *min_item_ = conditional_forward<FwdSk>(*other.min_item_); if (comparator_(*max_item_, *other.max_item_)) *max_item_ = conditional_forward<FwdSk>(*other.max_item_); @@ -426,8 +408,8 @@ void req_sketch<T, C, A>::serialize(std::ostream& os, const SerDe& sd) const { if (is_empty()) return; if (is_estimation_mode()) { write(os, n_); - sd.serialize(os, min_item_, 1); - sd.serialize(os, max_item_, 1); + sd.serialize(os, &*min_item_, 1); + sd.serialize(os, &*max_item_, 1); } if (raw_items) { sd.serialize(os, compactors_[0].begin(), num_raw_items); @@ -466,8 +448,8 @@ auto req_sketch<T, C, A>::serialize(unsigned header_size_bytes, const SerDe& sd) if (!is_empty()) { if (is_estimation_mode()) { ptr += copy_to_mem(n_, ptr); - ptr += sd.serialize(ptr, end_ptr - ptr, min_item_, 1); - ptr += sd.serialize(ptr, end_ptr - ptr, max_item_, 1); + ptr += sd.serialize(ptr, end_ptr - ptr, &*min_item_, 1); + ptr += sd.serialize(ptr, end_ptr - ptr, &*max_item_, 1); } if (raw_items) { ptr += sd.serialize(ptr, end_ptr - ptr, compactors_[0].begin(), num_raw_items); @@ -498,12 +480,9 @@ req_sketch<T, C, A> req_sketch<T, C, A>::deserialize(std::istream& is, const Ser const bool hra = flags_byte & (1 << flags::IS_HIGH_RANK); if (is_empty) return req_sketch(k, hra, comparator, allocator); - A alloc(allocator); - auto item_buffer_deleter = [&alloc](T* ptr) { alloc.deallocate(ptr, 1); }; - std::unique_ptr<T, decltype(item_buffer_deleter)> min_item_buffer(alloc.allocate(1), item_buffer_deleter); - std::unique_ptr<T, decltype(item_buffer_deleter)> max_item_buffer(alloc.allocate(1), item_buffer_deleter); - std::unique_ptr<T, item_deleter> min_item(nullptr, item_deleter(allocator)); - std::unique_ptr<T, item_deleter> max_item(nullptr, item_deleter(allocator)); + optional<T> tmp; // space to deserialize min and max + optional<T> min_item; + optional<T> max_item; const bool raw_items = flags_byte & (1 << flags::RAW_ITEMS); const bool is_level_0_sorted = flags_byte & (1 << flags::IS_LEVEL_ZERO_SORTED); @@ -512,12 +491,14 @@ req_sketch<T, C, A> req_sketch<T, C, A>::deserialize(std::istream& is, const Ser uint64_t n = 1; if (num_levels > 1) { n = read<uint64_t>(is); - sd.deserialize(is, min_item_buffer.get(), 1); - // serde call did not throw, repackage with destrtuctor - min_item = std::unique_ptr<T, item_deleter>(min_item_buffer.release(), item_deleter(allocator)); - sd.deserialize(is, max_item_buffer.get(), 1); - // serde call did not throw, repackage with destrtuctor - max_item = std::unique_ptr<T, item_deleter>(max_item_buffer.release(), item_deleter(allocator)); + sd.deserialize(is, &*tmp, 1); + // serde call did not throw, repackage and cleanup + min_item.emplace(*tmp); + tmp->~T(); + sd.deserialize(is, &*tmp, 1); + // serde call did not throw, repackage and cleanup + max_item.emplace(*tmp); + tmp->~T(); } if (raw_items) { @@ -537,12 +518,8 @@ req_sketch<T, C, A> req_sketch<T, C, A>::deserialize(std::istream& is, const Ser if (comparator(*it, *min_it)) min_it = it; if (comparator(*max_it, *it)) max_it = it; } - new (min_item_buffer.get()) T(*min_it); - // copy did not throw, repackage with destrtuctor - min_item = std::unique_ptr<T, item_deleter>(min_item_buffer.release(), item_deleter(allocator)); - new (max_item_buffer.get()) T(*max_it); - // copy did not throw, repackage with destrtuctor - max_item = std::unique_ptr<T, item_deleter>(max_item_buffer.release(), item_deleter(allocator)); + min_item.emplace(*min_it); + max_item.emplace(*max_it); } if (!is.good()) throw std::runtime_error("error reading from std::istream"); @@ -579,12 +556,9 @@ req_sketch<T, C, A> req_sketch<T, C, A>::deserialize(const void* bytes, size_t s const bool hra = flags_byte & (1 << flags::IS_HIGH_RANK); if (is_empty) return req_sketch(k, hra, comparator, allocator); - A alloc(allocator); - auto item_buffer_deleter = [&alloc](T* ptr) { alloc.deallocate(ptr, 1); }; - std::unique_ptr<T, decltype(item_buffer_deleter)> min_item_buffer(alloc.allocate(1), item_buffer_deleter); - std::unique_ptr<T, decltype(item_buffer_deleter)> max_item_buffer(alloc.allocate(1), item_buffer_deleter); - std::unique_ptr<T, item_deleter> min_item(nullptr, item_deleter(allocator)); - std::unique_ptr<T, item_deleter> max_item(nullptr, item_deleter(allocator)); + optional<T> tmp; // space to deserialize min and max + optional<T> min_item; + optional<T> max_item; const bool raw_items = flags_byte & (1 << flags::RAW_ITEMS); const bool is_level_0_sorted = flags_byte & (1 << flags::IS_LEVEL_ZERO_SORTED); @@ -594,12 +568,14 @@ req_sketch<T, C, A> req_sketch<T, C, A>::deserialize(const void* bytes, size_t s if (num_levels > 1) { ensure_minimum_memory(end_ptr - ptr, sizeof(n)); ptr += copy_from_mem(ptr, n); - ptr += sd.deserialize(ptr, end_ptr - ptr, min_item_buffer.get(), 1); - // serde call did not throw, repackage with destrtuctor - min_item = std::unique_ptr<T, item_deleter>(min_item_buffer.release(), item_deleter(allocator)); - ptr += sd.deserialize(ptr, end_ptr - ptr, max_item_buffer.get(), 1); - // serde call did not throw, repackage with destrtuctor - max_item = std::unique_ptr<T, item_deleter>(max_item_buffer.release(), item_deleter(allocator)); + ptr += sd.deserialize(ptr, end_ptr - ptr, &*tmp, 1); + // serde call did not throw, repackage and cleanup + min_item.emplace(*tmp); + tmp->~T(); + ptr += sd.deserialize(ptr, end_ptr - ptr, &*tmp, 1); + // serde call did not throw, repackage and cleanup + max_item.emplace(*tmp); + tmp->~T(); } if (raw_items) { @@ -623,12 +599,8 @@ req_sketch<T, C, A> req_sketch<T, C, A>::deserialize(const void* bytes, size_t s if (comparator(*it, *min_it)) min_it = it; if (comparator(*max_it, *it)) max_it = it; } - new (min_item_buffer.get()) T(*min_it); - // copy did not throw, repackage with destrtuctor - min_item = std::unique_ptr<T, item_deleter>(min_item_buffer.release(), item_deleter(allocator)); - new (max_item_buffer.get()) T(*max_it); - // copy did not throw, repackage with destrtuctor - max_item = std::unique_ptr<T, item_deleter>(max_item_buffer.release(), item_deleter(allocator)); + min_item.emplace(*min_it); + max_item.emplace(*max_it); } return req_sketch(k, hra, n, std::move(min_item), std::move(max_item), std::move(compactors), comparator); @@ -721,23 +693,9 @@ string<A> req_sketch<T, C, A>::to_string(bool print_levels, bool print_items) co return string<A>(os.str().c_str(), allocator_); } -template<typename T, typename C, typename A> -class req_sketch<T, C, A>::item_deleter { - public: - item_deleter(const A& allocator): allocator_(allocator) {} - void operator() (T* ptr) { - if (ptr != nullptr) { - ptr->~T(); - allocator_.deallocate(ptr, 1); - } - } - private: - A allocator_; -}; - template<typename T, typename C, typename A> req_sketch<T, C, A>::req_sketch(uint16_t k, bool hra, uint64_t n, - std::unique_ptr<T, item_deleter> min_item, std::unique_ptr<T, item_deleter> max_item, + optional<T>&& min_item, optional<T>&& max_item, std::vector<Compactor, AllocCompactor>&& compactors, const C& comparator): comparator_(comparator), allocator_(compactors.get_allocator()), @@ -747,8 +705,8 @@ max_nom_size_(0), num_retained_(0), n_(n), compactors_(std::move(compactors)), -min_item_(min_item.release()), -max_item_(max_item.release()), +min_item_(std::move(min_item)), +max_item_(std::move(max_item)), sorted_view_(nullptr) { update_max_nom_size(); --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
