From a2c9e41c6705de95ad8fba570c1d2b03d2e1bf85 Mon Sep 17 00:00:00 2001 From: Yingchun Lai Date: Mon, 15 Jul 2024 23:37:50 +0800 Subject: [PATCH 1/4] feat(util): Introduce map utilities --- .licenserc.yaml | 8 + LICENSE | 23 + src/CMakeLists.txt | 1 + src/base/idl_utils.h | 9 +- src/common/duplication_common.cpp | 1 + src/gutil/CMakeLists.txt | 21 + src/gutil/map_traits.h | 80 ++ src/gutil/map_util.h | 720 ++++++++++++++++++ src/gutil/no_destructor.h | 113 +++ src/gutil/test/CMakeLists.txt | 32 + src/gutil/test/config.ini | 75 ++ src/gutil/test/main.cpp | 28 + src/gutil/test/map_traits_test.cpp | 78 ++ src/gutil/test/map_util_test.h | 475 ++++++++++++ src/gutil/test/map_util_unittest.cpp | 522 +++++++++++++ src/gutil/test/no_destructor_test.cpp | 181 +++++ src/gutil/test/run.sh | 19 + src/http/http_call_registry.h | 8 +- src/http/http_server.cpp | 3 +- src/meta/app_balance_policy.cpp | 3 +- src/meta/app_env_validator.cpp | 32 +- src/meta/cluster_balance_policy.cpp | 12 +- .../duplication/meta_duplication_service.cpp | 5 +- src/meta/test/cluster_balance_policy_test.cpp | 7 +- .../test/meta_bulk_load_ingestion_test.cpp | 9 +- src/meta/test/meta_bulk_load_service_test.cpp | 13 +- src/nfs/nfs_server_impl.cpp | 17 +- src/replica/replica_config.cpp | 35 +- src/utils/metrics.h | 9 +- src/utils/test/logger.cpp | 3 +- src/utils/test/metrics_test.cpp | 50 +- 31 files changed, 2478 insertions(+), 114 deletions(-) create mode 100644 src/gutil/CMakeLists.txt create mode 100644 src/gutil/map_traits.h create mode 100644 src/gutil/map_util.h create mode 100644 src/gutil/no_destructor.h create mode 100644 src/gutil/test/CMakeLists.txt create mode 100644 src/gutil/test/config.ini create mode 100644 src/gutil/test/main.cpp create mode 100644 src/gutil/test/map_traits_test.cpp create mode 100644 src/gutil/test/map_util_test.h create mode 100644 src/gutil/test/map_util_unittest.cpp create mode 100644 src/gutil/test/no_destructor_test.cpp create mode 100755 src/gutil/test/run.sh diff --git a/.licenserc.yaml b/.licenserc.yaml index 6128a75b7c..b8e45b5ec4 100644 --- a/.licenserc.yaml +++ b/.licenserc.yaml @@ -667,5 +667,13 @@ header: - 'src/zookeeper/zookeeper_session.h' - 'src/zookeeper/zookeeper_session_mgr.cpp' - 'src/zookeeper/zookeeper_session_mgr.h' + # Apache License, Version 2.0, Copyright 2018 Google LLC + - 'src/gutil/test/map_traits_test.cpp' + - 'src/gutil/test/map_util_test.h' + - 'src/gutil/test/map_util_unittest.cpp' + - 'src/gutil/test/no_destructor_test.cpp' + - 'src/gutil/map_traits.h' + - 'src/gutil/map_util.h' + - 'src/gutil/no_destructor.h' comment: on-failure diff --git a/LICENSE b/LICENSE index c89dd134d9..a8e975f469 100644 --- a/LICENSE +++ b/LICENSE @@ -570,3 +570,26 @@ THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +-------------------------------------------------------------------------------- + +src/gutil/test/map_traits_test.cpp +src/gutil/test/map_util_test.h +src/gutil/test/map_util_unittest.cpp +src/gutil/test/no_destructor_test.cpp +src/gutil/map_traits.h +src/gutil/map_util.h +src/gutil/no_destructor.h + +Copyright 2018 Google LLC + +Licensed 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. diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 1a90098773..0bd9f65395 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -41,6 +41,7 @@ add_subdirectory(client_lib) add_subdirectory(common) add_subdirectory(failure_detector) add_subdirectory(geo) +add_subdirectory(gutil) add_subdirectory(http) add_subdirectory(meta) add_subdirectory(nfs) diff --git a/src/base/idl_utils.h b/src/base/idl_utils.h index bb04018102..88324929b0 100644 --- a/src/base/idl_utils.h +++ b/src/base/idl_utils.h @@ -23,16 +23,17 @@ #include "rrdb/rrdb_types.h" #include "utils/fmt_utils.h" +#include "gutil/map_util.h" namespace pegasus { inline std::string cas_check_type_to_string(dsn::apps::cas_check_type::type type) { - auto it = dsn::apps::_cas_check_type_VALUES_TO_NAMES.find(type); - if (it == dsn::apps::_cas_check_type_VALUES_TO_NAMES.end()) { - return std::string("INVALID=") + std::to_string(int(type)); + const auto *name = gutil::FindOrNull(dsn::apps::_cas_check_type_VALUES_TO_NAMES, type); + if (dsn_unlikely(name == nullptr)) { + return fmt::format("INVALID={}", type); } - return it->second; + return *name; } inline bool cas_is_check_operand_needed(dsn::apps::cas_check_type::type type) diff --git a/src/common/duplication_common.cpp b/src/common/duplication_common.cpp index 0aea933473..4dc5323c65 100644 --- a/src/common/duplication_common.cpp +++ b/src/common/duplication_common.cpp @@ -122,6 +122,7 @@ class duplication_group_registry : public utils::singleton + +namespace gutil { +namespace subtle { +namespace internal_map_traits { +struct Rank1 +{ +}; +struct Rank0 : Rank1 +{ +}; + +template +auto GetKey(V &&v, Rank0) -> decltype((std::forward(v).first)) +{ + return std::forward(v).first; +} +template +auto GetKey(V &&v, Rank1) -> decltype(std::forward(v).key()) +{ + return std::forward(v).key(); +} + +template +auto GetMapped(V &&v, Rank0) -> decltype((std::forward(v).second)) +{ + return std::forward(v).second; +} +template +auto GetMapped(V &&v, Rank1) -> decltype(std::forward(v).value()) +{ + return std::forward(v).value(); +} + +} // namespace internal_map_traits + +// Accesses the `key_type` from a `value_type`. +template +auto GetKey(V &&v) + -> decltype(internal_map_traits::GetKey(std::forward(v), internal_map_traits::Rank0())) +{ + return internal_map_traits::GetKey(std::forward(v), internal_map_traits::Rank0()); +} + +// Accesses the `mapped_type` from a `value_type`. +template +auto GetMapped(V &&v) + -> decltype(internal_map_traits::GetMapped(std::forward(v), internal_map_traits::Rank0())) +{ + return internal_map_traits::GetMapped(std::forward(v), internal_map_traits::Rank0()); +} + +} // namespace subtle +} // namespace gutil diff --git a/src/gutil/map_util.h b/src/gutil/map_util.h new file mode 100644 index 0000000000..6c054e2aae --- /dev/null +++ b/src/gutil/map_util.h @@ -0,0 +1,720 @@ +// +// Copyright 2018 Google LLC +// +// Licensed 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. +// + +#pragma once + +// This file provides utility functions for use with STL map-like data +// structures, such as std::map and hash_map. Some functions will also work with +// sets, such as ContainsKey(). +// +// The main functions in this file fall into the following categories: +// +// - Find*() +// - Contains*() +// - Insert*() +// - Lookup*() +// +// These functions often have "...OrDie" or "...OrDieNoPrint" variants. These +// variants will crash the process with a CHECK() failure on error, including +// the offending key/data in the log message. The NoPrint variants will not +// include the key/data in the log output under the assumption that it's not a +// printable type. +// +// Most functions are fairly self explanatory from their names, with the +// exception of Find*() vs Lookup*(). The Find functions typically use the map's +// .find() member function to locate and return the map's value type. The +// Lookup*() functions typically use the map's .insert() (yes, insert) member +// function to insert the given value if necessary and returns (usually a +// reference to) the map's value type for the found item. +// +// See the per-function comments for specifics. +// +// There are also a handful of functions for doing other miscellaneous things. +// +// A note on terminology: +// +// In this file, `m` and `M` represent a map and its type. +// +// Map-like containers are collections of pairs. Like all STL containers they +// contain a few standard typedefs identifying the types of data they contain. +// Given the following map declaration: +// +// std::map my_map; +// +// the notable typedefs would be as follows: +// +// - key_type -- string +// - value_type -- std::pair +// - mapped_type -- int +// +// Note that the map above contains two types of "values": the key-value pairs +// themselves (value_type) and the values within the key-value pairs +// (mapped_type). A value_type consists of a key_type and a mapped_type. +// +// The documentation below is written for programmers thinking in terms of keys +// and the (mapped_type) values associated with a given key. For example, the +// statement +// +// my_map["foo"] = 3; +// +// has a key of "foo" (type: string) with a value of 3 (type: int). +// + +#include +#include +#include +#include +#include +#include +#include + +#include "absl/meta/type_traits.h" +#include "utils/fmt_logging.h" +#include "gutil/map_traits.h" +#include "gutil/no_destructor.h" + +namespace gutil { + +// These helper template aliases are implementation details of map_util, +// provided for notational convenience. Despite the file-level documentation +// about map typedefs, map_util doesn't actually use them. +// It uses the Map's value_type, and the value_type's first_type and +// second_type. This can matter for nonconformant containers. +template +using MapUtilValueT = typename M::value_type; +template +using MapUtilKeyT = typename MapUtilValueT::first_type; +template +using MapUtilMappedT = typename MapUtilValueT::second_type; + +namespace internal_map_util { + +template +struct HasTryEmplace : std::false_type +{ +}; + +template +struct HasTryEmplace< + M, + absl::void_t().try_emplace(std::declval &>()))>> + : std::true_type +{ +}; + +template +struct InitType +{ + using type = MapUtilValueT; +}; + +template +struct InitType>::value>::type> +{ + using type = typename M::init_type; +}; + +template +const V &ValueInitializedDefault() +{ + static const gutil::NoDestructor value_initialized_default{}; + return *value_initialized_default; +} + +} // namespace internal_map_util + +template +using MapUtilInitT = typename internal_map_util::InitType::type; + +// +// Find*() +// + +// Returns a const reference to the value associated with the given key if it +// exists. Crashes otherwise. +// +// This is intended as a replacement for operator[] as an rvalue (for reading) +// when the key is guaranteed to exist. +// +// operator[] for lookup is discouraged for several reasons (note that these +// reasons may apply to only some map types): +// * It has a side-effect of inserting missing keys +// * It is not thread-safe (even when it is not inserting, it can still +// choose to resize the underlying storage) +// * It invalidates iterators (when it chooses to resize) +// * It default constructs a value object even if it doesn't need to +// +// This version assumes the key is printable, and includes it in the fatal log +// message. +template > +const MapUtilMappedT &FindOrDie(const M &m, const KeyType &key) +{ + auto it = m.find(key); + CHECK(it != m.end(), "Map key not found: {}", key); + return gutil::subtle::GetMapped(*it); +} + +// Same as above, but returns a non-const reference. +template > +MapUtilMappedT &FindOrDie(M &m, // NOLINT + const KeyType &key) +{ + auto it = m.find(key); + CHECK(it != m.end(), "Map key not found: {}", key); + return gutil::subtle::GetMapped(*it); +} + +// Same as FindOrDie above, but doesn't log the key on failure. +template > +const MapUtilMappedT &FindOrDieNoPrint(const M &m, const KeyType &key) +{ + auto it = m.find(key); + CHECK(it != m.end(), "Map key not found"); + return gutil::subtle::GetMapped(*it); +} + +// Same as above, but returns a non-const reference. +template > +MapUtilMappedT &FindOrDieNoPrint(M &m, // NOLINT + const KeyType &key) +{ + auto it = m.find(key); + CHECK(it != m.end(), "Map key not found"); + return gutil::subtle::GetMapped(*it); +} + +// Returns a const reference to the value associated with the given key if it +// exists, otherwise returns a const reference to a value-initialized object +// that is never destroyed. +template > +const MapUtilMappedT &FindWithDefault(const M &m, const KeyType &key) +{ + auto it = m.find(key); + if (it != m.end()) + return gutil::subtle::GetMapped(*it); + return internal_map_util::ValueInitializedDefault>(); +} + +// Returns a const reference to the value associated with the given key if it +// exists, otherwise returns a const reference to the provided default value. +// +// Prefer the two-argument form unless you need to specify a custom default +// value (i.e., one that is not equal to a value-initialized instance). +// +// WARNING: If a temporary object is passed as the default "value," +// this function will return a reference to that temporary object, +// which will be destroyed at the end of the statement. A common +// example: if you have a map with string values, and you pass a char* +// as the default "value," either use the returned value immediately +// or store it in a string (not string&). +// +// TODO: Stop using this. +template +const MapUtilMappedT & +FindWithDefault(const M &m, const MapUtilKeyT &key, const MapUtilMappedT &value) +{ + auto it = m.find(key); + if (it != m.end()) + return gutil::subtle::GetMapped(*it); + return value; +} + +// Returns a pointer to the const value associated with the given key if it +// exists, or null otherwise. +template > +const MapUtilMappedT *FindOrNull(const M &m, const KeyType &key) +{ + auto it = m.find(key); + if (it == m.end()) + return nullptr; + return &gutil::subtle::GetMapped(*it); +} + +// Returns a pointer to the non-const value associated with the given key if it +// exists, or null otherwise. +template > +MapUtilMappedT *FindOrNull(M &m, // NOLINT + const KeyType &key) +{ + auto it = m.find(key); + if (it == m.end()) + return nullptr; + return &gutil::subtle::GetMapped(*it); +} + +// Returns the pointer value associated with the given key. If none is found, +// null is returned. The function is designed to be used with a map of keys +// to pointers. +// +// This function does not distinguish between a missing key and a key mapped +// to a null value. +template > +MapUtilMappedT FindPtrOrNull(const M &m, const KeyType &key) +{ + auto it = m.find(key); + if (it == m.end()) + return MapUtilMappedT(); + return gutil::subtle::GetMapped(*it); +} + +// Same as above, except takes non-const reference to m. +// +// This function is needed for containers that propagate constness to the +// pointee, such as boost::ptr_map. +template > +MapUtilMappedT FindPtrOrNull(M &m, // NOLINT + const KeyType &key) +{ + auto it = m.find(key); + if (it == m.end()) + return MapUtilMappedT(); + return gutil::subtle::GetMapped(*it); +} + +// Finds the value associated with the given key and copies it to *value (if +// non-null). Returns false if the key was not found, true otherwise. +template +bool FindCopy(const M &m, const Key &key, Value *value) +{ + auto it = m.find(key); + if (it == m.end()) + return false; + if (value) + *value = gutil::subtle::GetMapped(*it); + return true; +} + +// +// Contains*() +// + +// Returns true if and only if the given m contains the given key. +template +bool ContainsKey(const M &m, const Key &key) +{ + return m.find(key) != m.end(); +} + +// Returns true if and only if the given m contains the given key-value +// pair. +template +bool ContainsKeyValuePair(const M &m, const Key &key, const Value &value) +{ + auto range = m.equal_range(key); + for (auto it = range.first; it != range.second; ++it) { + if (gutil::subtle::GetMapped(*it) == value) { + return true; + } + } + return false; +} + +// +// Insert*() +// + +// Inserts the given key-value pair into the m. Returns true if and +// only if the key from the given pair didn't previously exist. Otherwise, the +// value in the map is replaced with the value from the given pair. +template +bool InsertOrUpdate(M *m, const MapUtilInitT &vt) +{ + auto ret = m->insert(vt); + if (ret.second) + return true; + subtle::GetMapped(*ret.first) = subtle::GetMapped(vt); // update + return false; +} + +// Same as above, except that the key and value are passed separately. +template +bool InsertOrUpdate(M *m, const MapUtilKeyT &key, const MapUtilMappedT &value) +{ + return InsertOrUpdate(m, {key, value}); +} + +// Inserts/updates all the key-value pairs from the range defined by the +// iterators "first" and "last" into the given m. +template +void InsertOrUpdateMany(M *m, InputIterator first, InputIterator last) +{ + for (; first != last; ++first) { + InsertOrUpdate(m, *first); + } +} + +// Change the value associated with a particular key in a map or hash_map +// of the form std::map which owns the objects pointed to by the +// value pointers. If there was an existing value for the key, it is deleted. +// True indicates an insert took place, false indicates an update + delete. +template +bool InsertAndDeleteExisting(M *m, const MapUtilKeyT &key, const MapUtilMappedT &value) +{ + auto ret = m->insert(MapUtilValueT(key, value)); + if (ret.second) + return true; + delete ret.first->second; + ret.first->second = value; + return false; +} + +// Inserts the given key and value into the given m if and only if the +// given key did NOT already exist in the m. If the key previously +// existed in the m, the value is not changed. Returns true if the +// key-value pair was inserted; returns false if the key was already present. +template +bool InsertIfNotPresent(M *m, const MapUtilInitT &vt) +{ + return m->insert(vt).second; +} + +// Same as above except the key and value are passed separately. +template +bool InsertIfNotPresent(M *m, const MapUtilKeyT &key, const MapUtilMappedT &value) +{ + return InsertIfNotPresent(m, {key, value}); +} + +// Same as above except dies if the key already exists in the m. +template +void InsertOrDie(M *m, const MapUtilInitT &value) +{ + CHECK(InsertIfNotPresent(m, value), "duplicate value: {}", value); +} + +// Same as above except doesn't log the value on error. +template +void InsertOrDieNoPrint(M *m, const MapUtilInitT &value) +{ + CHECK(InsertIfNotPresent(m, value), "duplicate value."); +} + +// Inserts the key-value pair into the m. Dies if key was already +// present. +template +void InsertOrDie(M *m, const MapUtilKeyT &key, const MapUtilMappedT &data) +{ + CHECK(InsertIfNotPresent(m, key, data), "duplicate key: ", key); +} + +// Same as above except doesn't log the key on error. +template +void InsertOrDieNoPrint(M *m, const MapUtilKeyT &key, const MapUtilMappedT &data) +{ + CHECK(InsertIfNotPresent(m, key, data), "duplicate key."); +} + +// Inserts a new key and default-initialized value. Dies if the key was already +// present. Returns a reference to the value. Example usage: +// +// std::map m; +// SomeProto& proto = InsertKeyOrDie(&m, 3); +// proto.set_field("foo"); +template +auto InsertKeyOrDie(M *m, const MapUtilKeyT &key) -> + typename std::enable_if::value, MapUtilMappedT &>::type +{ + auto res = m->try_emplace(key); + CHECK(res.second, "duplicate key: ", key); + return gutil::subtle::GetMapped(*res.first); +} + +// Anything without try_emplace, we support with the legacy code path. +template +auto InsertKeyOrDie(M *m, const MapUtilKeyT &key) -> + typename std::enable_if::value, MapUtilMappedT &>::type +{ + auto res = m->insert(MapUtilValueT(key, MapUtilMappedT())); + CHECK(res.second, "duplicate key: ", key); + return res.first->second; +} + +// +// Lookup*() +// + +// Looks up a given key and value pair in m and inserts the key-value pair if +// it's not already present. Returns a reference to the value associated with +// the key. +template +MapUtilMappedT &LookupOrInsert(M *m, const MapUtilInitT &vt) +{ + return subtle::GetMapped(*m->insert(vt).first); +} + +// Same as above except the key-value are passed separately. +template +MapUtilMappedT &LookupOrInsert(M *m, const MapUtilKeyT &key, const MapUtilMappedT &value) +{ + return LookupOrInsert(m, {key, value}); +} + +// Returns a reference to the pointer associated with key. If not found, a +// pointee is constructed and added to the map. In that case, the new pointee is +// forwarded constructor arguments; when no arguments are provided the default +// constructor is used. +// +// Useful for containers of the form Map, where Ptr is pointer-like. +template +MapUtilMappedT &LookupOrInsertNew(M *m, const MapUtilKeyT &key, Args &&...args) +{ + using Mapped = MapUtilMappedT; + using MappedDeref = decltype(*std::declval()); + using Element = typename std::decay::type; + auto ret = m->insert(MapUtilValueT(key, Mapped())); + if (ret.second) { + ret.first->second = Mapped(new Element(std::forward(args)...)); + } + return ret.first->second; +} + +// +// Misc Utility Functions +// + +// Updates the value associated with the given key. If the key was not already +// present, then the key-value pair are inserted and "previous" is unchanged. If +// the key was already present, the value is updated and "*previous" will +// contain a copy of the old value. +// +// Returns true if and only if there was an already existing value. +// +// InsertOrReturnExisting has complementary behavior that returns the +// address of an already existing value, rather than updating it. + +template +bool UpdateReturnCopy(M *m, const MapUtilValueT &vt, MapUtilMappedT *previous) +{ + auto ret = m->insert(vt); + if (ret.second) + return false; + if (previous) + *previous = ret.first->second; + ret.first->second = vt.second; // update + return true; +} + +// Same as above except that the key and mapped value are passed separately. +template +bool UpdateReturnCopy(M *m, + const MapUtilKeyT &key, + const MapUtilMappedT &value, + MapUtilMappedT *previous) +{ + return UpdateReturnCopy(m, MapUtilValueT(key, value), previous); +} + +// Tries to insert the given key-value pair into the m. Returns null +// if the insert succeeds. Otherwise, returns a pointer to the existing value. +// +// This complements UpdateReturnCopy in that it allows to update only after +// verifying the old value and still insert quickly without having to look up +// twice. Unlike UpdateReturnCopy this also does not come with the issue of an +// undefined previous* in case new data was inserted. +template +MapUtilMappedT *InsertOrReturnExisting(M *m, const MapUtilValueT &vt) +{ + auto ret = m->insert(vt); + if (ret.second) + return nullptr; // Inserted, no previous value. + return &ret.first->second; // Return address of previous value. +} + +// Same as above, except for explicit key and data. +template +MapUtilMappedT * +InsertOrReturnExisting(M *m, const MapUtilKeyT &key, const MapUtilMappedT &data) +{ + return InsertOrReturnExisting(m, MapUtilValueT(key, data)); +} + +// Saves the reverse mapping into reverse. Returns true if values could all be +// inserted. +template +bool ReverseMap(const M &m, ReverseM *reverse) +{ + CHECK_NOTNULL(reverse, ""); + bool all_unique = true; + for (const auto &kv : m) { + if (!InsertOrUpdate(reverse, kv.second, kv.first)) { + all_unique = false; + } + } + return all_unique; +} + +// Like ReverseMap above, but returns its output m. Return type has to +// be specified explicitly. Example: +// M::M(...) : m_(...), r_(ReverseMap(m_)) {} +template +ReverseM ReverseMap(const M &m) +{ + typename std::remove_const::type reverse; + ReverseMap(m, &reverse); + return reverse; +} + +// Erases the m item identified by the given key, and returns the value +// associated with that key. It is assumed that the value (i.e., the +// mapped_type) is a pointer. Returns null if the key was not found in the +// m. +// +// Examples: +// std::map my_map; +// +// One line cleanup: +// delete EraseKeyReturnValuePtr(&my_map, "abc"); +// +// Use returned value: +// std::unique_ptr value_ptr( +// EraseKeyReturnValuePtr(&my_map, "abc")); +// if (value_ptr.get()) +// value_ptr->DoSomething(); +// +template +MapUtilMappedT EraseKeyReturnValuePtr(M *m, const MapUtilKeyT &key) +{ + auto it = m->find(key); + if (it == m->end()) + return nullptr; + MapUtilMappedT v = std::move(gutil::subtle::GetMapped(*it)); + m->erase(it); + return v; +} + +// Inserts all the keys from m into key_container, which must +// support insert(M::key_type). +// +// Note: any initial contents of the key_container are not cleared. +template +void InsertKeysFromMap(const M &m, KeyContainer *key_container) +{ + CHECK_NOTNULL(key_container, ""); + for (const auto &kv : m) { + key_container->insert(kv.first); + } +} + +// Appends all the keys from m into key_container, which must +// support push_back(M::key_type). +// +// Note: any initial contents of the key_container are not cleared. +template +void AppendKeysFromMap(const M &m, KeyContainer *key_container) +{ + CHECK_NOTNULL(key_container, ""); + for (const auto &kv : m) { + key_container->push_back(kv.first); + } +} + +// A more specialized overload of AppendKeysFromMap to optimize reallocations +// for the common case in which we're appending keys to a vector and hence can +// (and sometimes should) call reserve() first. +// +// (It would be possible to play SFINAE games to call reserve() for any +// m that supports it, but this seems to get us 99% of what we need +// without the complexity of a SFINAE-based solution.) +template +void AppendKeysFromMap(const M &m, std::vector *key_container) +{ + CHECK_NOTNULL(key_container, ""); + // We now have the opportunity to call reserve(). Calling reserve() every + // time is a bad idea for some use cases: libstdc++'s implementation of + // std::vector<>::reserve() resizes the vector's backing store to exactly the + // given size (unless it's already at least that big). Because of this, + // the use case that involves appending a lot of small maps (total size + // N) one by one to a vector would be O(N^2). But never calling reserve() + // loses the opportunity to improve the use case of adding from a large + // map to an empty vector (this improves performance by up to 33%). A + // number of heuristics are possible. Here we use the simplest one. + if (key_container->empty()) { + key_container->reserve(m.size()); + } + for (const auto &kv : m) { + key_container->push_back(kv.first); + } +} + +// Inserts all the values from m into value_container, which must +// support push_back(M::mapped_type). +// +// Note: any initial contents of the value_container are not cleared. +template +void AppendValuesFromMap(const M &m, ValueContainer *value_container) +{ + CHECK_NOTNULL(value_container, ""); + for (const auto &kv : m) { + value_container->push_back(kv.second); + } +} + +// A more specialized overload of AppendValuesFromMap to optimize reallocations +// for the common case in which we're appending values to a vector and hence +// can (and sometimes should) call reserve() first. +// +// (It would be possible to play SFINAE games to call reserve() for any +// m that supports it, but this seems to get us 99% of what we need +// without the complexity of a SFINAE-based solution.) +template +void AppendValuesFromMap(const M &m, std::vector *value_container) +{ + CHECK_NOTNULL(value_container, ""); + // See AppendKeysFromMap for why this is done. + if (value_container->empty()) { + value_container->reserve(m.size()); + } + for (const auto &kv : m) { + value_container->push_back(kv.second); + } +} + +// Erases all elements of m where predicate evaluates to true. +// Note: To avoid unnecessary temporary copies of map elements passed to the +// predicate, the predicate must accept 'const M::value_type&'. +// In particular, the value type for a map is 'std::pair', and so a +// predicate accepting 'std::pair' will result in temporary copies. +template +auto AssociativeEraseIf(M *m, Predicate predicate) -> + typename std::enable_iferase(m->begin()))>::value>::type +{ + CHECK_NOTNULL(m, ""); + for (auto it = m->begin(); it != m->end();) { + if (predicate(*it)) { + m->erase(it++); + } else { + ++it; + } + } +} + +template +auto AssociativeEraseIf(M *m, Predicate predicate) -> typename std::enable_if< + std::is_samebegin()), decltype(m->erase(m->begin()))>::value>::type +{ + CHECK_NOTNULL(m, ""); + for (auto it = m->begin(); it != m->end();) { + if (predicate(*it)) { + it = m->erase(it); + } else { + ++it; + } + } +} + +} // namespace gutil diff --git a/src/gutil/no_destructor.h b/src/gutil/no_destructor.h new file mode 100644 index 0000000000..4385f8ec83 --- /dev/null +++ b/src/gutil/no_destructor.h @@ -0,0 +1,113 @@ +// +// Copyright 2018 Google LLC +// +// Licensed 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. +// + +#pragma once + +#include +#include + +namespace gutil { + +// NoDestructor is a wrapper around an object of type T that +// * stores the object of type T inline inside NoDestructor +// * eagerly forwards constructor arguments to it (i.e. acts like T in terms +// of construction) +// * provides access to the object of type T like a pointer via ->, *, and get() +// (note that const NoDestructor works like a pointer to const T) +// * never calls T's destructor for the object +// (hence NoDestructor objects created on the stack or as member variables +// will lead to memory and/or resource leaks) +// +// One key use case of NoDestructor (which in itself is not lazy) is optimizing +// the following pattern of safe on-demand construction of an object with +// non-trivial constructor in static storage without destruction ever happening: +// const string& MyString() { +// static string* x = new string("foo"); // note the "static" +// return *x; +// } +// By using NoDestructor we do not need to involve heap allocation and +// corresponding pointer following (and hence extra CPU cache usage/needs) +// on each access: +// const string& MyString() { +// static NoDestructor x("foo"); +// return *x; +// } +// Since C++11 this static-in-a-function pattern results in exactly-once, +// thread-safe, on-demand construction of an object, and very fast access +// thereafter (the cost is a few extra cycles). +// NoDestructor makes accesses even faster by storing the object inline in +// static storage. +// +// Note that: +// * Since destructor is never called, the object lives on during program exit +// and can be safely accessed by any threads that have not been joined. +// +// Also note that +// static NoDestructor ptr(whatever); +// can safely replace +// static NonPOD* ptr = new NonPOD(whatever); +// or +// static NonPOD obj(whatever); +// at file-level scope when the safe static-in-a-function pattern is infeasible +// to use for some good reason. +// All three of the NonPOD patterns above suffer from the same issue that +// initialization of that object happens non-thread-safely at +// a globally-undefined point during initialization of static-storage objects, +// but NoDestructor<> usage provides both the safety of having the object alive +// during program exit sequence and the performance of not doing extra memory +// dereference on access. +// +template +class NoDestructor +{ +public: + typedef T element_type; + + // Forwards arguments to the T's constructor: calls T(args...). + template ::type...), void(NoDestructor)>::value, + int>::type = 0> + explicit NoDestructor(Ts &&...args) + { + new (&space_) T(std::forward(args)...); + } + + // Forwards copy and move construction for T. Enables usage like this: + // static NoDestructor> x{{{"1", "2", "3"}}}; + // static NoDestructor> x{{1, 2, 3}}; + explicit NoDestructor(const T &x) { new (&space_) T(x); } + explicit NoDestructor(T &&x) { new (&space_) T(std::move(x)); } + + // No copying. + NoDestructor(const NoDestructor &) = delete; + NoDestructor &operator=(const NoDestructor &) = delete; + + // Pretend to be a smart pointer to T with deep constness. + // Never returns a null pointer. + T &operator*() { return *get(); } + T *operator->() { return get(); } + T *get() { return reinterpret_cast(&space_); } + const T &operator*() const { return *get(); } + const T *operator->() const { return get(); } + const T *get() const { return reinterpret_cast(&space_); } + +private: + typename std::aligned_storage::type space_; +}; + +} // namespace gutil diff --git a/src/gutil/test/CMakeLists.txt b/src/gutil/test/CMakeLists.txt new file mode 100644 index 0000000000..af9b3ca69c --- /dev/null +++ b/src/gutil/test/CMakeLists.txt @@ -0,0 +1,32 @@ +# 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. + +set(MY_PROJ_NAME pgs_gutil_test) +set(MY_PROJ_SRC "") +set(MY_SRC_SEARCH_MODE "GLOB") +set(MY_PROJ_LIBS + absl::btree + absl::flat_hash_map + absl::node_hash_map + dsn_runtime + dsn_utils + gmock + gtest) +set(MY_BINPLACES + "${CMAKE_CURRENT_SOURCE_DIR}/run.sh" +) +dsn_add_test() diff --git a/src/gutil/test/config.ini b/src/gutil/test/config.ini new file mode 100644 index 0000000000..744d8d412a --- /dev/null +++ b/src/gutil/test/config.ini @@ -0,0 +1,75 @@ +; 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. + +[apps..default] +run = true +count = 1 +network.client.RPC_CHANNEL_TCP = dsn::tools::asio_network_provider, 65536 +network.client.RPC_CHANNEL_UDP = dsn::tools::asio_udp_provider, 65536 +network.server.0.RPC_CHANNEL_TCP = dsn::tools::asio_network_provider, 65536 +network.server.0.RPC_CHANNEL_UDP = dsn::tools::asio_udp_provider, 65536 + +[apps.test] +type = test +arguments = +run = true +ports = 20001 +count = 1 +pools = THREAD_POOL_DEFAULT + +[core] +tool = nativerun + +toollets = tracer, profiler +pause_on_start = false + +logging_start_level = LOG_LEVEL_DEBUG +logging_factory_name = dsn::tools::simple_logger + +[tools.simple_logger] +fast_flush = true +short_header = false +stderr_start_level = LOG_LEVEL_DEBUG + +[network] +; how many network threads for network library (used by asio) +io_service_worker_count = 2 + +[task..default] +is_trace = true +is_profile = true +allow_inline = false +rpc_call_channel = RPC_CHANNEL_TCP +rpc_message_header_format = dsn +rpc_timeout_milliseconds = 1000 + +[task.LPC_RPC_TIMEOUT] +is_trace = false +is_profile = false + +[task.RPC_TEST_UDP] +rpc_call_channel = RPC_CHANNEL_UDP +rpc_message_crc_required = true + +; specification for each thread pool +[threadpool..default] +worker_count = 2 + +[threadpool.THREAD_POOL_DEFAULT] +partitioned = false +worker_priority = THREAD_xPRIORITY_NORMAL + diff --git a/src/gutil/test/main.cpp b/src/gutil/test/main.cpp new file mode 100644 index 0000000000..7473ada88a --- /dev/null +++ b/src/gutil/test/main.cpp @@ -0,0 +1,28 @@ +// 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 +#include + +#include "runtime/app_model.h" + +GTEST_API_ int main(int argc, char **argv) +{ + testing::InitGoogleTest(&argc, argv); + int ret = RUN_ALL_TESTS(); + dsn_exit(ret); +} diff --git a/src/gutil/test/map_traits_test.cpp b/src/gutil/test/map_traits_test.cpp new file mode 100644 index 0000000000..783a98d7f8 --- /dev/null +++ b/src/gutil/test/map_traits_test.cpp @@ -0,0 +1,78 @@ +// +// Copyright 2018 Google LLC +// +// Licensed 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 "gutil/map_traits.h" + +#include + +#include "gmock/gmock.h" +#include "gtest/gtest.h" +#include "absl/container/node_hash_map.h" + +namespace gutil { +namespace subtle { +namespace { + +TEST(MapTraitsTest, UnorderedMap) +{ + absl::node_hash_map m = {{1, 2}}; + EXPECT_EQ(1, GetKey(*m.begin())); + EXPECT_EQ(2, GetMapped(*m.begin())); +} + +TEST(MapTraitsTest, UnorderedMapReferences) +{ + absl::node_hash_map m = {{1, 2}}; + auto it = m.begin(); + const int *k = &it->first; + int *v = &it->second; + EXPECT_EQ(k, &GetKey(*it)); + EXPECT_EQ(v, &GetMapped(*it)); + GetMapped(*it) = 3; + EXPECT_EQ(3, m[1]); +} + +TEST(MapTraitsTest, UnorderedMapConstReferences) +{ + const absl::node_hash_map m = {{1, 2}}; + auto it = m.begin(); + const int *k = &it->first; + const int *v = &it->second; + EXPECT_EQ(k, &GetKey(*it)); + EXPECT_EQ(v, &GetMapped(*it)); +} + +struct CustomMapValueType +{ + int first; + int second; + + // Intentionally add 1 to the result to verify that first/second are preferred + // to key()/value(). + int key() const { return first + 1; } + int value() const { return second + 1; } +}; + +TEST(MapTraitsTest, ValueTypeHasBothFieldsAndGetters) +{ + CustomMapValueType entry = {100, 1000}; + EXPECT_EQ(100, GetKey(entry)); + EXPECT_EQ(1000, GetMapped(entry)); +} + +} // namespace +} // namespace subtle +} // namespace gutil diff --git a/src/gutil/test/map_util_test.h b/src/gutil/test/map_util_test.h new file mode 100644 index 0000000000..fae785768d --- /dev/null +++ b/src/gutil/test/map_util_test.h @@ -0,0 +1,475 @@ +// +// Copyright 2018 Google LLC +// +// Licensed 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. +// + +#pragma once + +// Contains map_util tests templated on STL std::map-like types. + +#include +#include +#include + +#include "gtest/gtest.h" +#include "utils/fmt_logging.h" +#include "gutil/map_util.h" + +namespace gutil { + +template +class MapUtilIntIntTest : public ::testing::Test +{ +}; +TYPED_TEST_SUITE_P(MapUtilIntIntTest); + +template +class MapUtilIntIntPtrTest : public ::testing::Test +{ +}; +TYPED_TEST_SUITE_P(MapUtilIntIntPtrTest); + +template +class MapUtilIntIntSharedPtrTest : public ::testing::Test +{ +}; +TYPED_TEST_SUITE_P(MapUtilIntIntSharedPtrTest); + +template +class MapUtilIntIntSharedPtrOnlyTest : public ::testing::Test +{ +}; +TYPED_TEST_SUITE_P(MapUtilIntIntSharedPtrOnlyTest); + +template +class MultiMapUtilIntIntTest : public ::testing::Test +{ +}; +TYPED_TEST_SUITE_P(MultiMapUtilIntIntTest); + +TYPED_TEST_P(MapUtilIntIntTest, ValueMapTests) +{ + using Map = TypeParam; + Map m; + + // Check that I get a default when the key is not present. + EXPECT_EQ(0, FindWithDefault(m, 0, 0)); + EXPECT_EQ(0, FindWithDefault(m, 0)); + // Check that I can insert a value. + EXPECT_TRUE(InsertOrUpdate(&m, 0, 1)); + // .. and get that value back. + EXPECT_EQ(1, FindWithDefault(m, 0, 0)); + EXPECT_EQ(1, FindWithDefault(m, 0)); + // Check that I can update a value. + EXPECT_FALSE(InsertOrUpdate(&m, 0, 2)); + // .. and get that value back. + EXPECT_EQ(2, FindWithDefault(m, 0, 0)); + EXPECT_EQ(2, FindWithDefault(m, 0)); + // Check that FindOrDie works when the value exists. + EXPECT_EQ(2, FindOrDie(m, 0)); + EXPECT_EQ(2, FindOrDieNoPrint(m, 0)); + + // Check FindCopy + int i = 0; + EXPECT_TRUE(FindCopy(m, 0, &i)); + EXPECT_EQ(i, 2); + EXPECT_FALSE(FindCopy(m, 1, &i)); + EXPECT_TRUE(FindCopy(m, 0, static_cast(nullptr))); + EXPECT_FALSE(FindCopy(m, 1, static_cast(nullptr))); + + // Check FindOrNull + int *p1 = FindOrNull(m, 0); + ASSERT_EQ(*p1, 2); + ++(*p1); + const int *p2 = FindOrNull(const_cast(m), 0); + ASSERT_EQ(*p2, 3); + ASSERT_TRUE(FindOrNull(m, 1) == nullptr); + + // Check contains + EXPECT_TRUE(ContainsKey(m, 0)); + EXPECT_FALSE(ContainsKey(m, 1)); + + // Check ContainsKeyValuePair + EXPECT_TRUE(ContainsKeyValuePair(m, 0, 3)); + EXPECT_FALSE(ContainsKeyValuePair(m, 0, 4)); + EXPECT_FALSE(ContainsKeyValuePair(m, 1, 0)); + + // Check insert if not present + EXPECT_FALSE(InsertIfNotPresent(&m, 0, 2)); + EXPECT_TRUE(InsertIfNotPresent(&m, 1, 3)); + + // Check lookup or insert + EXPECT_EQ(3, LookupOrInsert(&m, 0, 2)); + EXPECT_EQ(4, LookupOrInsert(&m, 2, 4)); + EXPECT_EQ(4, FindWithDefault(m, 2, 0)); + EXPECT_EQ(4, FindWithDefault(m, 2)); + + EXPECT_FALSE(InsertOrUpdate(&m, typename Map::value_type(0, 2))); + EXPECT_EQ(2, FindWithDefault(m, 0, 0)); + EXPECT_EQ(2, FindWithDefault(m, 0)); + + // Check InsertOrUpdateMany + std::vector> entries; + entries.push_back(std::make_pair(0, 100)); + entries.push_back(std::make_pair(100, 101)); + entries.push_back(std::make_pair(200, 102)); + + InsertOrUpdateMany(&m, entries.begin(), entries.end()); + EXPECT_EQ(100, FindWithDefault(m, 0, 0)); + EXPECT_EQ(100, FindWithDefault(m, 0)); + EXPECT_EQ(101, FindWithDefault(m, 100, 0)); + EXPECT_EQ(101, FindWithDefault(m, 100)); + EXPECT_EQ(102, FindWithDefault(m, 200, 0)); + EXPECT_EQ(102, FindWithDefault(m, 200)); +} + +TYPED_TEST_P(MapUtilIntIntPtrTest, LookupOrInsertNewTest) +{ + using PtrMap = TypeParam; + PtrMap m; + int *v1, *v2, *v3, *v4; + + // Check inserting one item. + v1 = LookupOrInsertNew(&m, 7); + EXPECT_EQ(0, *v1); + ASSERT_TRUE(v1 != nullptr); + EXPECT_TRUE(ContainsKey(m, 7)); + EXPECT_EQ(m.size(), 1); + + // Check inserting the same item. + v2 = LookupOrInsertNew(&m, 7); + ASSERT_TRUE(v2 != nullptr); + EXPECT_EQ(v1, v2); + EXPECT_EQ(m.size(), 1); + + // Check a couple more items. + v1 = LookupOrInsertNew(&m, 8); + ASSERT_TRUE(v1 != nullptr); + EXPECT_NE(v1, v2); + EXPECT_TRUE(ContainsKey(m, 8)); + EXPECT_EQ(m.size(), 2); + + v2 = LookupOrInsertNew(&m, 8); + EXPECT_EQ(v1, v2); + EXPECT_EQ(m.size(), 2); + + v3 = LookupOrInsertNew(&m, 8, 88); + EXPECT_NE(88, *v3); + EXPECT_EQ(v3, v2); + EXPECT_EQ(m.size(), 2); + + v4 = LookupOrInsertNew(&m, 9, 99); + EXPECT_EQ(99, *v4); + EXPECT_NE(v1, v4); + EXPECT_NE(v2, v4); + EXPECT_NE(v3, v4); + EXPECT_EQ(m.size(), 3); + + // Return by reference, so that the stored value can be modified in the map. + // We check this by verifying the address of the returned value is identical. + EXPECT_EQ(&LookupOrInsertNew(&m, 9), &LookupOrInsertNew(&m, 9, 999)); + + for (auto &kv : m) + delete kv.second; +} + +TYPED_TEST_P(MapUtilIntIntSharedPtrTest, LookupOrInsertNewSharedPtrTest) +{ + using SharedPtrMap = TypeParam; + using SharedPtr = typename TypeParam::value_type::second_type; + SharedPtrMap m; + SharedPtr v1, v2, v3, v4, v5; + + // Check inserting one item. + v1 = LookupOrInsertNew(&m, 7); + ASSERT_TRUE(v1.get() != nullptr); + EXPECT_TRUE(ContainsKey(m, 7)); + EXPECT_EQ(m.size(), 1); + *v1 = 25; + + // Check inserting the same item. + v2 = LookupOrInsertNew(&m, 7); + ASSERT_TRUE(v2.get() != nullptr); + EXPECT_EQ(v1.get(), v2.get()); + EXPECT_EQ(m.size(), 1); + EXPECT_EQ(25, *v2.get()); + + // Check a couple more items. + v2 = LookupOrInsertNew(&m, 8); + ASSERT_TRUE(v2.get() != nullptr); + EXPECT_NE(v1.get(), v2.get()); + EXPECT_TRUE(ContainsKey(m, 8)); + EXPECT_EQ(m.size(), 2); + *v2 = 42; + + v3 = LookupOrInsertNew(&m, 8); + EXPECT_NE(v1.get(), v2.get()); + EXPECT_EQ(v2.get(), v3.get()); + EXPECT_EQ(m.size(), 2); + EXPECT_EQ(25, *v1.get()); + EXPECT_EQ(42, *v2.get()); + EXPECT_EQ(42, *v3.get()); + + m.clear(); + // Since the container does not own the elements and because we still have the + // shared pointers we can still access the old values. + v3 = LookupOrInsertNew(&m, 7); + EXPECT_NE(v1.get(), v3.get()); + EXPECT_NE(v2.get(), v3.get()); + EXPECT_EQ(m.size(), 1); + EXPECT_EQ(25, *v1.get()); + EXPECT_EQ(42, *v2.get()); + EXPECT_EQ(0, *v3.get()); // Also checks for default init of POD elements + + v4 = LookupOrInsertNew(&m, 7, 77); + EXPECT_NE(v1.get(), v4.get()); + EXPECT_NE(v2.get(), v4.get()); + EXPECT_EQ(v3.get(), v4.get()); + EXPECT_EQ(m.size(), 1); + EXPECT_EQ(25, *v1.get()); + EXPECT_EQ(42, *v2.get()); + EXPECT_EQ(0, *v3.get()); + EXPECT_EQ(0, *v4.get()); + + v5 = LookupOrInsertNew(&m, 8, 88); + EXPECT_NE(v1.get(), v5.get()); + EXPECT_NE(v2.get(), v5.get()); + EXPECT_NE(v3.get(), v5.get()); + EXPECT_NE(v4.get(), v5.get()); + EXPECT_EQ(m.size(), 2); + EXPECT_EQ(25, *v1.get()); + EXPECT_EQ(42, *v2.get()); + EXPECT_EQ(0, *v3.get()); + EXPECT_EQ(0, *v4.get()); + EXPECT_EQ(88, *v5.get()); +} + +TYPED_TEST_P(MapUtilIntIntSharedPtrOnlyTest, LookupOrInsertNewSharedPtrSwapTest) +{ + using SharedPtrMap = TypeParam; + using SharedPtr = typename TypeParam::value_type::second_type; + SharedPtrMap m; + SharedPtr v1, v2, v3, v4; + + v1.reset(new int(1)); + LookupOrInsertNew(&m, 11).swap(v1); + EXPECT_TRUE(v1.get() != nullptr); + EXPECT_EQ(0, *v1.get()); // The element created by LookupOrInsertNew + EXPECT_TRUE(ContainsKey(m, 11)); + EXPECT_EQ(1, m.size()); + // If the functions does not correctly return by ref then v2 will contain 0 + // instead of 1 even though v2 still points to the held entry. The tests that + // depend on return by ref use ASSERT_*(). + v2 = LookupOrInsertNew(&m, 11); + ASSERT_EQ(1, *v2.get()); + EXPECT_EQ(v2.get(), LookupOrInsertNew(&m, 11).get()); + + *v2 = 2; + v3 = LookupOrInsertNew(&m, 11); + EXPECT_EQ(2, *v2.get()); + EXPECT_EQ(2, *v3.get()); + ASSERT_NE(v1.get(), v2.get()); + EXPECT_EQ(v2.get(), v3.get()); + ASSERT_NE(v1.get(), LookupOrInsertNew(&m, 11).get()); + EXPECT_EQ(v2.get(), LookupOrInsertNew(&m, 11).get()); + EXPECT_EQ(v3.get(), LookupOrInsertNew(&m, 11).get()); + + v4.reset(new int(4)); + LookupOrInsertNew(&m, 11).swap(v4); + EXPECT_EQ(2, *v4.get()); + ASSERT_EQ(4, *LookupOrInsertNew(&m, 11).get()); + ASSERT_EQ(v3.get(), v4.get()); +} + +TYPED_TEST_P(MapUtilIntIntPtrTest, InsertAndDeleteExistingTest) +{ + using PtrMap = TypeParam; + PtrMap m; + + // Add a few items. + int *v1 = new int; + int *v2 = new int; + int *v3 = new int; + EXPECT_TRUE(InsertAndDeleteExisting(&m, 1, v1)); + EXPECT_TRUE(InsertAndDeleteExisting(&m, 2, v2)); + EXPECT_TRUE(InsertAndDeleteExisting(&m, 3, v3)); + EXPECT_EQ(v1, FindPtrOrNull(m, 1)); + EXPECT_EQ(v2, FindPtrOrNull(m, 2)); + EXPECT_EQ(v3, FindPtrOrNull(m, 3)); + + // Replace a couple. + int *v4 = new int; + int *v5 = new int; + EXPECT_FALSE(InsertAndDeleteExisting(&m, 1, v4)); + EXPECT_FALSE(InsertAndDeleteExisting(&m, 2, v5)); + EXPECT_EQ(v4, FindPtrOrNull(m, 1)); + EXPECT_EQ(v5, FindPtrOrNull(m, 2)); + EXPECT_EQ(v3, FindPtrOrNull(m, 3)); + + // Add one more item. + int *v6 = new int; + EXPECT_TRUE(InsertAndDeleteExisting(&m, 6, v6)); + EXPECT_EQ(v4, FindPtrOrNull(m, 1)); + EXPECT_EQ(v5, FindPtrOrNull(m, 2)); + EXPECT_EQ(v3, FindPtrOrNull(m, 3)); + EXPECT_EQ(v6, FindPtrOrNull(m, 6)); + + // 6 total allocations, this will only delete 4. Heap-check will fail + // here if the existing entries weren't properly deleted. + EXPECT_EQ(4, m.size()); + for (auto &kv : m) + delete kv.second; +} + +TYPED_TEST_P(MapUtilIntIntTest, UpdateReturnCopyTest) +{ + using Map = TypeParam; + Map m; + + int p = 10; + EXPECT_FALSE(UpdateReturnCopy(&m, 0, 5, &p)); + EXPECT_EQ(10, p); + + EXPECT_TRUE(UpdateReturnCopy(&m, 0, 7, &p)); + EXPECT_EQ(5, p); + + // Check UpdateReturnCopy using value_type + p = 10; + EXPECT_FALSE(UpdateReturnCopy(&m, typename Map::value_type(1, 4), &p)); + EXPECT_EQ(10, p); + + EXPECT_TRUE(UpdateReturnCopy(&m, typename Map::value_type(1, 8), &p)); + EXPECT_EQ(4, p); +} + +TYPED_TEST_P(MapUtilIntIntTest, InsertOrReturnExistingTest) +{ + using Map = TypeParam; + Map m; + + EXPECT_EQ(nullptr, InsertOrReturnExisting(&m, 25, 42)); + EXPECT_EQ(42, m[25]); + + int *previous = InsertOrReturnExisting(&m, 25, 666); + EXPECT_EQ(42, *previous); + EXPECT_EQ(42, m[25]); +} + +TYPED_TEST_P(MapUtilIntIntPtrTest, FindPtrOrNullTest) +{ + // Check FindPtrOrNull + using PtrMap = TypeParam; + PtrMap ptr_map; + InsertOrUpdate(&ptr_map, 35, new int(35)); + int *p1 = FindPtrOrNull(ptr_map, 3); + EXPECT_TRUE(nullptr == p1); + const int *p2 = FindPtrOrNull(const_cast(ptr_map), 3); + EXPECT_TRUE(nullptr == p2); + EXPECT_EQ(35, *FindPtrOrNull(ptr_map, 35)); + + for (auto &kv : ptr_map) + delete kv.second; +} + +TYPED_TEST_P(MapUtilIntIntSharedPtrTest, FindPtrOrNullTest) +{ + using SharedPtrMap = TypeParam; + using SharedPtr = typename TypeParam::value_type::second_type; + SharedPtrMap shared_ptr_map; + InsertOrUpdate(&shared_ptr_map, 35, SharedPtr(new int(35))); + const SharedPtr p1 = FindPtrOrNull(shared_ptr_map, 3); + EXPECT_TRUE(nullptr == p1.get()); + const SharedPtr p2 = FindPtrOrNull(const_cast(shared_ptr_map), 3); + EXPECT_TRUE(nullptr == p2.get()); + const SharedPtr p3 = FindPtrOrNull(shared_ptr_map, 35); + const SharedPtr p4 = FindPtrOrNull(shared_ptr_map, 35); + EXPECT_EQ(35, *p3.get()); + EXPECT_EQ(35, *p4.get()); +} + +TYPED_TEST_P(MapUtilIntIntTest, FindOrDieTest) +{ + using Map = TypeParam; + Map m; + m[10] = 15; + EXPECT_EQ(15, FindOrDie(m, 10)); + ASSERT_DEATH(FindOrDie(m, 8), ""); // Map key not found: 8 + EXPECT_EQ(15, FindOrDieNoPrint(m, 10)); + ASSERT_DEATH(FindOrDieNoPrint(m, 8), ""); // Map key not found + + // Make sure the non-const reference returning version works. + FindOrDie(m, 10) = 20; + EXPECT_EQ(20, FindOrDie(m, 10)); + + // Make sure we can lookup values in a const map. + const Map &const_m = m; + EXPECT_EQ(20, FindOrDie(const_m, 10)); +} + +TYPED_TEST_P(MapUtilIntIntTest, InsertOrDieTest) +{ + using Map = TypeParam; + Map m; + InsertOrDie(&m, 1, 2); + EXPECT_EQ(m[1], 2); + ASSERT_DEATH(InsertOrDie(&m, 1, 3), ""); // duplicate +} + +TYPED_TEST_P(MapUtilIntIntTest, InsertKeyOrDieTest) +{ + using Map = TypeParam; + Map m; + int &v = InsertKeyOrDie(&m, 1); + EXPECT_EQ(m[1], 0); + v = 2; + EXPECT_EQ(m[1], 2); + ASSERT_DEATH(InsertKeyOrDie(&m, 1), ""); // duplicate +} + +TYPED_TEST_P(MapUtilIntIntPtrTest, EraseKeyReturnValuePtrTest) +{ + using PtrMap = TypeParam; + PtrMap ptr_map; + int *v = new int(35); + InsertOrUpdate(&ptr_map, 35, v); + EXPECT_TRUE(EraseKeyReturnValuePtr(&ptr_map, 0) == nullptr); // Test no-op. + EXPECT_EQ(ptr_map.size(), 1); + int *retv = EraseKeyReturnValuePtr(&ptr_map, 35); // Successful operation + EXPECT_EQ(ptr_map.size(), 0); + EXPECT_EQ(v, retv); + delete v; + EXPECT_TRUE(EraseKeyReturnValuePtr(&ptr_map, 35) == nullptr); // Empty map. +} + +TYPED_TEST_P(MultiMapUtilIntIntTest, ContainsKeyValuePairTest) +{ + using Map = TypeParam; + + Map m; + + m.insert(std::make_pair(1, 10)); + m.insert(std::make_pair(1, 11)); + m.insert(std::make_pair(1, 12)); + + m.insert(std::make_pair(3, 13)); + + EXPECT_FALSE(ContainsKeyValuePair(m, 0, 0)); + EXPECT_FALSE(ContainsKeyValuePair(m, 1, 0)); + EXPECT_TRUE(ContainsKeyValuePair(m, 1, 10)); + EXPECT_TRUE(ContainsKeyValuePair(m, 1, 11)); + EXPECT_TRUE(ContainsKeyValuePair(m, 1, 12)); + EXPECT_FALSE(ContainsKeyValuePair(m, 1, 13)); +} + +} // namespace gutil diff --git a/src/gutil/test/map_util_unittest.cpp b/src/gutil/test/map_util_unittest.cpp new file mode 100644 index 0000000000..fae676f894 --- /dev/null +++ b/src/gutil/test/map_util_unittest.cpp @@ -0,0 +1,522 @@ +// +// Copyright 2018 Google LLC +// +// Licensed 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 "gutil/map_util.h" + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "gmock/gmock.h" +#include "gtest/gtest.h" +#include "absl/container/flat_hash_map.h" +#include "absl/container/node_hash_map.h" +#include "utils/fmt_logging.h" + +// All of the templates for the tests are defined here. +// This file is critical to understand what is tested. +#include "absl/container/btree_map.h" +#include "absl/container/btree_set.h" +#include "map_util_test.h" + +namespace gutil { + +using ::testing::ElementsAre; +using ::testing::IsEmpty; +using ::testing::Pair; +using ::testing::Pointee; +using ::testing::UnorderedElementsAre; + +TEST(MapUtil, ImplicitTypeConversion) +{ + using Map = absl::btree_map; + Map m; + + // Check that I can use a type that's implicitly convertible to the + // key or value type, such as const char* -> string. + EXPECT_EQ("", FindWithDefault(m, "foo", "")); + EXPECT_EQ("", FindWithDefault(m, "foo")); + EXPECT_TRUE(InsertOrUpdate(&m, "foo", "bar")); + EXPECT_EQ("bar", FindWithDefault(m, "foo", "")); + EXPECT_EQ("bar", FindWithDefault(m, "foo")); + EXPECT_EQ("bar", *FindOrNull(m, "foo")); + std::string str; + EXPECT_TRUE(FindCopy(m, "foo", &str)); + EXPECT_EQ("bar", str); + EXPECT_TRUE(ContainsKey(m, "foo")); +} + +TEST(MapUtil, HeterogeneousLookup) +{ + absl::flat_hash_map m; + const auto &const_m = m; + + // Verify that I can use a key type that's appropriate for heterogeneous + // lookup, such as string_view -> string. + constexpr absl::string_view kLookupKey = "foo"; + EXPECT_EQ(FindWithDefault(m, kLookupKey), ""); + EXPECT_EQ(FindWithDefault(const_m, kLookupKey), ""); + + m["foo"] = "bar"; + + EXPECT_EQ(FindOrDie(m, kLookupKey), "bar"); + EXPECT_EQ(FindOrDie(const_m, kLookupKey), "bar"); + + EXPECT_EQ(FindOrDieNoPrint(m, kLookupKey), "bar"); + EXPECT_EQ(FindOrDieNoPrint(const_m, kLookupKey), "bar"); + + EXPECT_EQ(FindWithDefault(m, kLookupKey), "bar"); + EXPECT_EQ(FindWithDefault(const_m, kLookupKey), "bar"); + + EXPECT_THAT(FindOrNull(m, kLookupKey), Pointee(testing::Eq("bar"))); + EXPECT_THAT(FindOrNull(const_m, kLookupKey), Pointee(testing::Eq("bar"))); + + std::string str; + EXPECT_TRUE(FindCopy(m, kLookupKey, &str)); + EXPECT_EQ(str, "bar"); + + std::string str_from_const; + EXPECT_TRUE(FindCopy(const_m, kLookupKey, &str_from_const)); + EXPECT_EQ(str_from_const, "bar"); + + absl::flat_hash_map ptr_m; + const auto &const_ptr_m = ptr_m; + + // Insert an arbitrary non-null pointer into the map. + ASSERT_TRUE(InsertOrUpdate(&ptr_m, "foo", &ptr_m)); + + EXPECT_EQ(FindPtrOrNull(ptr_m, kLookupKey), &ptr_m); + EXPECT_EQ(FindPtrOrNull(const_ptr_m, kLookupKey), &ptr_m); +} + +TEST(MapUtil, SetOperations) +{ + // Set operations + using Set = std::set; + Set s; + EXPECT_TRUE(InsertIfNotPresent(&s, 0)); + EXPECT_FALSE(InsertIfNotPresent(&s, 0)); + EXPECT_TRUE(ContainsKey(s, 0)); +} + +TEST(MapUtil, ReverseMapWithoutDups) +{ + std::map forward; + forward["1"] = 1; + forward["2"] = 2; + forward["3"] = 3; + forward["4"] = 4; + forward["5"] = 5; + std::map reverse; + EXPECT_TRUE(ReverseMap(forward, &reverse)); + EXPECT_THAT(reverse, + ElementsAre(Pair(1, "1"), Pair(2, "2"), Pair(3, "3"), Pair(4, "4"), Pair(5, "5"))); +} + +TEST(MapUtil, ReverseMapWithDups) +{ + std::map forward; + forward["1"] = 1; + forward["2"] = 2; + forward["3"] = 3; + forward["4"] = 4; + forward["5"] = 5; + forward["6"] = 1; + forward["7"] = 2; + std::map reverse; + EXPECT_FALSE(ReverseMap(forward, &reverse)); + // There are 5 distinct values in forward. + EXPECT_THAT(reverse, + ElementsAre(Pair(1, "6"), Pair(2, "7"), Pair(3, "3"), Pair(4, "4"), Pair(5, "5"))); +} + +TEST(MapUtil, SingleArgumentReverseMapWithoutDups) +{ + std::map forward; + forward["1"] = 1; + forward["2"] = 2; + forward["3"] = 3; + forward["4"] = 4; + forward["5"] = 5; + const std::map reverse = ReverseMap>(forward); + EXPECT_THAT(reverse, + ElementsAre(Pair(1, "1"), Pair(2, "2"), Pair(3, "3"), Pair(4, "4"), Pair(5, "5"))); +} + +TEST(MapUtil, SingleArgumentReverseMapWithDups) +{ + std::map forward; + forward["1"] = 1; + forward["2"] = 2; + forward["3"] = 3; + forward["4"] = 4; + forward["5"] = 5; + forward["6"] = 1; + forward["7"] = 2; + const std::map reverse = ReverseMap>(forward); + // There are 5 distinct values in forward. + EXPECT_THAT(reverse, + ElementsAre(Pair(1, "6"), Pair(2, "7"), Pair(3, "3"), Pair(4, "4"), Pair(5, "5"))); +} + +// Wrapper around an int that we can use to test a key without operator<<. +struct Unprintable +{ + int a; + explicit Unprintable(int a) : a(a) {} + bool operator<(const Unprintable &other) const { return a < other.a; } + bool operator==(const Unprintable &other) const { return a == other.a; } +}; + +TEST(MapUtilDeathTest, FindOrDieNoPrint) +{ + // Test FindOrDieNoPrint with a value with no operator<<. + std::map m; + m[Unprintable(1)] = 8; + EXPECT_EQ(8, FindOrDieNoPrint(m, Unprintable(1))); + ASSERT_DEATH(FindOrDieNoPrint(m, Unprintable(2)), ""); // Map key not found + + // Make sure the non-const reference returning version works. + FindOrDieNoPrint(m, Unprintable(1)) = 20; + EXPECT_EQ(20, FindOrDieNoPrint(m, Unprintable(1))); + + // Make sure we can lookup values in a const std::map. + const std::map &const_m = m; + EXPECT_EQ(20, FindOrDieNoPrint(const_m, Unprintable(1))); +} + +TEST(MapUtilDeathTest, SetInsertOrDieTest) +{ + std::set s; + InsertOrDie(&s, 1); + EXPECT_TRUE(ContainsKey(s, 1)); + ASSERT_DEATH(InsertOrDie(&s, 1), ""); // duplicate +} + +TEST(MapUtilDeathTest, InsertOrDieNoPrint) +{ + std::pair key = std::make_pair(1, 1); + + std::map, int> m; + InsertOrDieNoPrint(&m, key, 2); + EXPECT_EQ(m[key], 2); + ASSERT_DEATH(InsertOrDieNoPrint(&m, key, 3), ""); // duplicate + + std::set> s; + InsertOrDieNoPrint(&s, key); + EXPECT_TRUE(ContainsKey(s, key)); + ASSERT_DEATH(InsertOrDieNoPrint(&s, key), ""); // duplicate +} + +TEST(MapUtil, InsertKeysFromMap) +{ + const std::map empty_map; + std::set keys_as_ints; + InsertKeysFromMap(empty_map, &keys_as_ints); + EXPECT_TRUE(keys_as_ints.empty()); + + std::set keys_as_longs; // NOLINT + InsertKeysFromMap(empty_map, &keys_as_longs); + EXPECT_TRUE(keys_as_longs.empty()); + + const std::pair number_names_array[] = { + std::make_pair("one", 1), std::make_pair("two", 2), std::make_pair("three", 3)}; + std::map number_names_map( + number_names_array, + number_names_array + sizeof number_names_array / sizeof *number_names_array); + absl::btree_set names; + InsertKeysFromMap(number_names_map, &names); + // No two numbers have the same name, so the container sizes must match. + EXPECT_EQ(names.size(), number_names_map.size()); + EXPECT_EQ(names.count("one"), 1); + EXPECT_EQ(names.count("two"), 1); + EXPECT_EQ(names.count("three"), 1); +} + +TEST(MapUtil, AppendKeysFromMap) +{ + const std::map empty_map; + std::vector keys_as_ints; + AppendKeysFromMap(empty_map, &keys_as_ints); + EXPECT_TRUE(keys_as_ints.empty()); + + std::list keys_as_longs; // NOLINT + AppendKeysFromMap(empty_map, &keys_as_longs); + EXPECT_TRUE(keys_as_longs.empty()); + + const std::pair number_names_array[] = { + std::make_pair("one", 1), std::make_pair("two", 2), std::make_pair("three", 3)}; + std::map number_names_map( + number_names_array, + number_names_array + sizeof number_names_array / sizeof *number_names_array); + std::deque names; + AppendKeysFromMap(number_names_map, &names); + // No two numbers have the same name, so the container sizes must match. + EXPECT_EQ(names.size(), number_names_map.size()); + // The names are appended in the order in which they are found in the + // map, i.e., lexicographical order. + EXPECT_EQ(names[0], "one"); + EXPECT_EQ(names[1], "three"); + EXPECT_EQ(names[2], "two"); + + // Appending again should double the size of the std::deque + AppendKeysFromMap(number_names_map, &names); + EXPECT_EQ(names.size(), 2 * number_names_map.size()); +} + +// Vector is a special case. +TEST(MapUtil, AppendKeysFromMapIntoVector) +{ + const std::map empty_map; + std::vector keys_as_ints; + AppendKeysFromMap(empty_map, &keys_as_ints); + EXPECT_TRUE(keys_as_ints.empty()); + + std::vector keys_as_longs; // NOLINT + AppendKeysFromMap(empty_map, &keys_as_longs); + EXPECT_TRUE(keys_as_longs.empty()); + + const std::pair number_names_array[] = { + std::make_pair("one", 1), std::make_pair("two", 2), std::make_pair("three", 3)}; + std::map number_names_map( + number_names_array, + number_names_array + sizeof number_names_array / sizeof *number_names_array); + std::vector names; + AppendKeysFromMap(number_names_map, &names); + // No two numbers have the same name, so the container sizes must match. + EXPECT_EQ(names.size(), number_names_map.size()); + // The names are appended in the order in which they are found in the + // map, i.e., lexicographical order. + EXPECT_EQ(names[0], "one"); + EXPECT_EQ(names[1], "three"); + EXPECT_EQ(names[2], "two"); + + // Appending again should double the size of the std::deque + AppendKeysFromMap(number_names_map, &names); + EXPECT_EQ(names.size(), 2 * number_names_map.size()); +} + +TEST(MapUtil, AppendValuesFromMap) +{ + const std::map empty_map; + std::vector values_as_ints; + AppendValuesFromMap(empty_map, &values_as_ints); + EXPECT_TRUE(values_as_ints.empty()); + + std::list values_as_longs; // NOLINT + AppendValuesFromMap(empty_map, &values_as_longs); + EXPECT_TRUE(values_as_longs.empty()); + + const std::pair number_names_array[] = { + std::make_pair("one", 1), std::make_pair("two", 2), std::make_pair("three", 3)}; + std::map number_names_map( + number_names_array, + number_names_array + sizeof number_names_array / sizeof *number_names_array); + std::deque numbers; + AppendValuesFromMap(number_names_map, &numbers); + // No two numbers have the same name, so the container sizes must match. + EXPECT_EQ(numbers.size(), number_names_map.size()); + // The numbers are appended in the order in which they are found in the + // map, i.e., lexicographical order. + EXPECT_EQ(numbers[0], 1); + EXPECT_EQ(numbers[1], 3); + EXPECT_EQ(numbers[2], 2); + + // Appending again should double the size of the std::deque + AppendValuesFromMap(number_names_map, &numbers); + EXPECT_EQ(numbers.size(), 2 * number_names_map.size()); +} + +TEST(MapUtil, AppendValuesFromMapIntoVector) +{ + const std::map empty_map; + std::vector values_as_ints; + AppendValuesFromMap(empty_map, &values_as_ints); + EXPECT_TRUE(values_as_ints.empty()); + + std::list values_as_longs; // NOLINT + AppendValuesFromMap(empty_map, &values_as_longs); + EXPECT_TRUE(values_as_longs.empty()); + + const std::pair number_names_array[] = { + std::make_pair("one", 1), std::make_pair("two", 2), std::make_pair("three", 3)}; + std::map number_names_map( + number_names_array, + number_names_array + sizeof number_names_array / sizeof *number_names_array); + std::vector numbers; + AppendValuesFromMap(number_names_map, &numbers); + // No two numbers have the same name, so the container sizes must match. + EXPECT_EQ(numbers.size(), number_names_map.size()); + // The numbers are appended in the order in which they are found in the + // map, i.e., lexicographical order. + EXPECT_EQ(numbers[0], 1); + EXPECT_EQ(numbers[1], 3); + EXPECT_EQ(numbers[2], 2); + + // Appending again should double the size of the std::deque + AppendValuesFromMap(number_names_map, &numbers); + EXPECT_EQ(numbers.size(), 2 * number_names_map.size()); +} + +//////////////////////////////////////////////////////////////////////////////// +// Instantiate tests for std::map and std::unordered_map. +//////////////////////////////////////////////////////////////////////////////// + +// Finish setup for MapType +REGISTER_TYPED_TEST_SUITE_P(MapUtilIntIntTest, + ValueMapTests, + UpdateReturnCopyTest, + InsertOrReturnExistingTest, + FindOrDieTest, + InsertOrDieTest, + InsertKeyOrDieTest); +using MapIntIntTypes = ::testing:: + Types, absl::node_hash_map, absl::node_hash_map>; +INSTANTIATE_TYPED_TEST_SUITE_P(MapUtilTest, MapUtilIntIntTest, MapIntIntTypes); + +// Finish setup for MapType +REGISTER_TYPED_TEST_SUITE_P(MapUtilIntIntPtrTest, + LookupOrInsertNewTest, + InsertAndDeleteExistingTest, + FindPtrOrNullTest, + EraseKeyReturnValuePtrTest); +using MapIntIntPtrTypes = ::testing::Types, absl::node_hash_map>; +INSTANTIATE_TYPED_TEST_SUITE_P(MapUtilTest, MapUtilIntIntPtrTest, MapIntIntPtrTypes); + +// Finish setup for MapType > +REGISTER_TYPED_TEST_SUITE_P(MapUtilIntIntSharedPtrTest, + FindPtrOrNullTest, + LookupOrInsertNewSharedPtrTest); +using MapIntIntSharedPtrTypes = ::testing::Types>, + absl::node_hash_map>>; +INSTANTIATE_TYPED_TEST_SUITE_P(MapUtilTest, MapUtilIntIntSharedPtrTest, MapIntIntSharedPtrTypes); + +REGISTER_TYPED_TEST_SUITE_P(MapUtilIntIntSharedPtrOnlyTest, LookupOrInsertNewSharedPtrSwapTest); +typedef ::testing::Types>, + absl::node_hash_map>> + MapIntIntSharedPtrOnlyTypes; +INSTANTIATE_TYPED_TEST_SUITE_P(MapUtilTest, + MapUtilIntIntSharedPtrOnlyTest, + MapIntIntSharedPtrOnlyTypes); + +using AssociateEraseMapTypes = + ::testing::Types, absl::node_hash_map>; + +template +class AssociativeEraseIfTest : public ::testing::Test +{ +}; +TYPED_TEST_SUITE_P(AssociativeEraseIfTest); + +TYPED_TEST_P(AssociativeEraseIfTest, Basic) +{ + using ValueType = std::pair; + TypeParam m; + m["a"] = 1; + m["b"] = 2; + m["c"] = 3; + m["d"] = 4; + + // Test that none of the elements are removed when the predicate always + // returns false. + struct FalseFunc + { + bool operator()(const ValueType &unused) const { return 0; } + }; + AssociativeEraseIf(&m, FalseFunc()); + EXPECT_THAT(m, UnorderedElementsAre(Pair("a", 1), Pair("b", 2), Pair("c", 3), Pair("d", 4))); + + // Test removing a single element. + struct KeyEqualsA + { + bool operator()(const ValueType &pair) const { return pair.first == "a"; } + }; + AssociativeEraseIf(&m, KeyEqualsA()); + EXPECT_THAT(m, UnorderedElementsAre(Pair("b", 2), Pair("c", 3), Pair("d", 4))); + + // Put the element back and test removing a couple elements, + m["a"] = 1; + struct ValueGreaterThanTwo + { + bool operator()(const ValueType &pair) const { return pair.second > 2; } + }; + AssociativeEraseIf(&m, ValueGreaterThanTwo()); + EXPECT_THAT(m, UnorderedElementsAre(Pair("a", 1), Pair("b", 2))); + + // Put the elements back and test removing all of them. + m["c"] = 3; + m["d"] = 4; + struct TrueFunc + { + bool operator()(const ValueType &unused) const { return 1; } + }; + AssociativeEraseIf(&m, TrueFunc()); + EXPECT_THAT(m, IsEmpty()); +} +REGISTER_TYPED_TEST_SUITE_P(AssociativeEraseIfTest, Basic); + +INSTANTIATE_TYPED_TEST_SUITE_P(MapUtilTest, AssociativeEraseIfTest, AssociateEraseMapTypes); + +TEST(MapUtil, InsertKeyOrDie_SmartPtrTest) +{ + absl::node_hash_map> m; + m[1].reset(new int(10)); + m[2].reset(new int(20)); + + EXPECT_THAT( + m, UnorderedElementsAre(Pair(1, ::testing::Pointee(10)), Pair(2, ::testing::Pointee(20)))); + InsertKeyOrDie(&m, 3).reset(new int(30)); + EXPECT_THAT(m, + UnorderedElementsAre(Pair(1, ::testing::Pointee(10)), + Pair(2, ::testing::Pointee(20)), + Pair(3, ::testing::Pointee(30)))); +} + +TEST(MapUtil, EraseKeyReturnValuePtr_SmartPtrTest) +{ + std::map> m; + m[1] = std::unique_ptr(new int(10)); + m[2] = std::unique_ptr(new int(20)); + + std::unique_ptr val1 = EraseKeyReturnValuePtr(&m, 1); + EXPECT_EQ(10, *val1); + EXPECT_THAT(m, ElementsAre(Pair(2, ::testing::Pointee(20)))); + auto val2 = EraseKeyReturnValuePtr(&m, 2); + EXPECT_EQ(20, *val2); +} + +TEST(MapUtil, LookupOrInsertNewVariadicTest) +{ + struct TwoArg + { + TwoArg(int one_in, int two_in) : one(one_in), two(two_in) {} + int one; + int two; + }; + + std::map> m; + TwoArg *val = LookupOrInsertNew(&m, 1, 100, 200).get(); + EXPECT_EQ(100, val->one); + EXPECT_EQ(200, val->two); +} + +} // namespace gutil diff --git a/src/gutil/test/no_destructor_test.cpp b/src/gutil/test/no_destructor_test.cpp new file mode 100644 index 0000000000..3897453380 --- /dev/null +++ b/src/gutil/test/no_destructor_test.cpp @@ -0,0 +1,181 @@ +// +// Copyright 2018 Google LLC +// +// Licensed 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 "gutil/no_destructor.h" + +#include + +#include "gmock/gmock.h" +#include "gtest/gtest.h" +#include "utils/fmt_logging.h" + +namespace gutil { +namespace { + +struct Blob +{ + Blob() : val(42) {} + Blob(int x, int y) : val(x + y) {} + Blob(std::initializer_list xs) + { + val = 0; + for (auto &x : xs) + val += x; + } + + Blob(const Blob & /*b*/) = delete; + Blob(Blob &&b) noexcept : val(b.val) { b.moved_out = true; } // moving is fine + + // no crash: NoDestructor indeed does not destruct (the moved-out Blob + // temporaries do get destroyed though) + ~Blob() { CHECK(moved_out, "~Blob"); } + + int val; + bool moved_out = false; +}; + +struct TypeWithDeletedDestructor +{ + ~TypeWithDeletedDestructor() = delete; +}; + +TEST(NoDestructorTest, DestructorNeverCalled) +{ + NoDestructor a; + (void)a; +} + +TEST(NoDestructorTest, Noncopyable) +{ + using T = NoDestructor; + + EXPECT_FALSE((std::is_constructible::value)); + EXPECT_FALSE((std::is_constructible::value)); + EXPECT_FALSE((std::is_constructible::value)); + EXPECT_FALSE((std::is_constructible::value)); + + EXPECT_FALSE((std::is_assignable::value)); + EXPECT_FALSE((std::is_assignable::value)); + EXPECT_FALSE((std::is_assignable::value)); + EXPECT_FALSE((std::is_assignable::value)); +} + +TEST(NoDestructorTest, Interface) +{ + EXPECT_TRUE(std::is_trivially_destructible>::value); + EXPECT_TRUE(std::is_trivially_destructible>::value); + { + NoDestructor b; // default c-tor + // access: *, ->, get() + EXPECT_EQ(42, (*b).val); + (*b).val = 55; + EXPECT_EQ(55, b->val); + b->val = 66; + EXPECT_EQ(66, b.get()->val); + b.get()->val = 42; + EXPECT_EQ(42, (*b).val); + } + { + NoDestructor b(70, 7); // regular c-tor, const + EXPECT_EQ(77, (*b).val); + EXPECT_EQ(77, b->val); + EXPECT_EQ(77, b.get()->val); + } + { + const NoDestructor b{{20, 28, 40}}; // init-list c-tor, deep const + // This only works in clang, not in gcc: + // const NoDestructor b({20, 28, 40}); + EXPECT_EQ(88, (*b).val); + EXPECT_EQ(88, b->val); + EXPECT_EQ(88, b.get()->val); + } +} + +// ========================================================================= // + +std::string *Str0() +{ + static NoDestructor x; + return x.get(); +} + +extern const std::string &Str2(); + +const char *Str1() +{ + static NoDestructor x(Str2() + "_Str1"); + return x->c_str(); +} + +const std::string &Str2() +{ + static NoDestructor x("Str2"); + return *x; +} + +const std::string &Str2Copy() +{ + static NoDestructor x(Str2()); // exercise copy construction + return *x; +} + +typedef std::array MyArray; +const MyArray &Array() +{ + static NoDestructor x{{{"foo", "bar", "baz"}}}; + // This only works in clang, not in gcc: + // static NoDestructor x({{"foo", "bar", "baz"}}); + return *x; +} + +typedef std::vector MyVector; +const MyVector &Vector() +{ + static NoDestructor x{{1, 2, 3}}; + return *x; +} + +const int &Int() +{ + static NoDestructor x; + return *x; +} + +TEST(NoDestructorTest, StaticPattern) +{ + EXPECT_TRUE(std::is_trivially_destructible>::value); + EXPECT_TRUE(std::is_trivially_destructible>::value); + EXPECT_TRUE(std::is_trivially_destructible>::value); + EXPECT_TRUE(std::is_trivially_destructible>::value); + + EXPECT_EQ(*Str0(), ""); + Str0()->append("foo"); + EXPECT_EQ(*Str0(), "foo"); + + EXPECT_EQ(std::string(Str1()), "Str2_Str1"); + + EXPECT_EQ(Str2(), "Str2"); + EXPECT_EQ(Str2Copy(), "Str2"); + + EXPECT_THAT(Array(), testing::ElementsAre("foo", "bar", "baz")); + + EXPECT_THAT(Vector(), testing::ElementsAre(1, 2, 3)); + + EXPECT_EQ(0, Int()); // should get zero-initialized +} + +} // namespace +} // namespace gutil diff --git a/src/gutil/test/run.sh b/src/gutil/test/run.sh new file mode 100755 index 0000000000..52ad88a6a3 --- /dev/null +++ b/src/gutil/test/run.sh @@ -0,0 +1,19 @@ +#!/bin/sh +# 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. + +./pgs_gutil_test diff --git a/src/http/http_call_registry.h b/src/http/http_call_registry.h index 69eb80fc1c..7decdc9ff3 100644 --- a/src/http/http_call_registry.h +++ b/src/http/http_call_registry.h @@ -20,6 +20,7 @@ #include "utils/fmt_logging.h" #include "http_server.h" #include "utils/errors.h" +#include "gutil/map_util.h" #include "utils/singleton.h" namespace dsn { @@ -35,11 +36,8 @@ class http_call_registry : public utils::singleton std::shared_ptr find(const std::string &path) const { std::lock_guard guard(_mu); - const auto &iter = _call_map.find(path); - if (iter == _call_map.end()) { - return nullptr; - } - return iter->second; + const auto *hc = gutil::FindOrNull(_call_map, path); + return hc == nullptr ? nullptr : *hc; } void remove(const std::string &path) diff --git a/src/http/http_server.cpp b/src/http/http_server.cpp index ce2178bfde..8b3173163b 100644 --- a/src/http/http_server.cpp +++ b/src/http/http_server.cpp @@ -250,8 +250,7 @@ void http_server::serve(message_ex *msg) if (sep + 1 < arg_val.size()) { value = arg_val.substr(sep + 1, arg_val.size() - sep); } - auto iter = ret.query_args.find(name); - if (iter != ret.query_args.end()) { + if (gutil::ContainsKey(ret.query_args, name)) { return FMT_ERR(ERR_INVALID_PARAMETERS, "duplicate parameter: {}", name); } ret.query_args.emplace(std::move(name), std::move(value)); diff --git a/src/meta/app_balance_policy.cpp b/src/meta/app_balance_policy.cpp index 3f7a0a985e..102e4a2557 100644 --- a/src/meta/app_balance_policy.cpp +++ b/src/meta/app_balance_policy.cpp @@ -27,6 +27,7 @@ #include "metadata_types.h" #include "utils/flags.h" #include "utils/fmt_logging.h" +#include "gutil/map_util.h" DSN_DEFINE_bool(meta_server, balancer_in_turn, false, "balance the apps one-by-one/concurrently"); DSN_DEFINE_bool(meta_server, only_primary_balancer, false, "only try to make the primary balanced"); @@ -162,7 +163,7 @@ bool copy_secondary_operation::can_select(gpid pid, migration_list *result) } // if the pid have been used - if (result->find(pid) != result->end()) { + if (gutil::ContainsKey(*result, pid)) { LOG_DEBUG("{}: skip gpid({}.{}) coz it is already copyed", _app->get_logname(), pid.get_app_id(), diff --git a/src/meta/app_env_validator.cpp b/src/meta/app_env_validator.cpp index c27bdb2cd5..06288cb611 100644 --- a/src/meta/app_env_validator.cpp +++ b/src/meta/app_env_validator.cpp @@ -29,6 +29,7 @@ #include "common/replica_envs.h" #include "http/http_status_code.h" #include "utils/fmt_logging.h" +#include "gutil/map_util.h" #include "utils/string_conv.h" #include "utils/strings.h" #include "utils/throttling_controller.h" @@ -52,10 +53,8 @@ bool app_env_validator::validate_app_envs(const std::mapsecond.type) { + switch (func->type) { case ValueType::kBool: { // Check by the default boolean validator. bool result = false; @@ -197,8 +196,7 @@ bool app_env_validator::validate_app_env(const std::string &env_name, } case ValueType::kString: { // Check by the self defined validator. - if (nullptr != func_iter->second.string_validator && - !func_iter->second.string_validator(env_value, hint_message)) { + if (nullptr != func->string_validator && !func->string_validator(env_value, hint_message)) { return false; } break; @@ -208,13 +206,11 @@ bool app_env_validator::validate_app_env(const std::string &env_name, __builtin_unreachable(); } - if (func_iter->second.type == ValueType::kInt32 || - func_iter->second.type == ValueType::kInt64) { + if (func->type == ValueType::kInt32 || func->type == ValueType::kInt64) { // Check by the self defined validator. - if (nullptr != func_iter->second.int_validator && - !func_iter->second.int_validator(int_result)) { - hint_message = fmt::format( - "invalid value '{}', should be '{}'", env_value, func_iter->second.limit_desc); + if (nullptr != func->int_validator && !func->int_validator(int_result)) { + hint_message = + fmt::format("invalid value '{}', should be '{}'", env_value, func->limit_desc); return false; } } @@ -354,10 +350,10 @@ const std::unordered_map nlohmann::json app_env_validator::EnvInfo::to_json() const { - const auto &type_str = ValueType2String.find(type); - CHECK_TRUE(type_str != ValueType2String.end()); + const auto *type_str = gutil::FindOrNull(ValueType2String, type); + CHECK_NOTNULL(type_str, ""); nlohmann::json info; - info["type"] = type_str->second; + info["type"] = *type_str; info["limitation"] = limit_desc; info["sample"] = sample; return info; diff --git a/src/meta/cluster_balance_policy.cpp b/src/meta/cluster_balance_policy.cpp index 2b10e39ece..dd548e193c 100644 --- a/src/meta/cluster_balance_policy.cpp +++ b/src/meta/cluster_balance_policy.cpp @@ -25,6 +25,7 @@ #include #include "dsn.layer2_types.h" +#include "gutil/map_util.h" #include "meta/load_balance_policy.h" #include "runtime/rpc/dns_resolver.h" // IWYU pragma: keep #include "runtime/rpc/rpc_address.h" @@ -258,14 +259,9 @@ void cluster_balance_policy::get_node_migration_info(const node_state &ns, if (!context.get_disk_tag(ns.host_port(), disk_tag)) { continue; } - auto pid = context.pc->pid; - if (info.partitions.find(disk_tag) != info.partitions.end()) { - info.partitions[disk_tag].insert(pid); - } else { - partition_set pset; - pset.insert(pid); - info.partitions.emplace(disk_tag, pset); - } + + auto partitions_of_disk = gutil::LookupOrInsert(&info.partitions, disk_tag, {}); + partitions_of_disk.insert(context.pc->pid); } } } diff --git a/src/meta/duplication/meta_duplication_service.cpp b/src/meta/duplication/meta_duplication_service.cpp index 02562afeb0..f35f76f6c3 100644 --- a/src/meta/duplication/meta_duplication_service.cpp +++ b/src/meta/duplication/meta_duplication_service.cpp @@ -104,13 +104,12 @@ void meta_duplication_service::modify_duplication(duplication_modify_rpc rpc) return; } - auto it = app->duplications.find(dupid); - if (it == app->duplications.end()) { + auto dup = gutil::FindPtrOrNull(app->duplications, dupid); + if (!dup) { response.err = ERR_OBJECT_NOT_FOUND; return; } - duplication_info_s_ptr dup = it->second; auto to_status = request.__isset.status ? request.status : dup->status(); auto to_fail_mode = request.__isset.fail_mode ? request.fail_mode : dup->fail_mode(); response.err = dup->alter_status(to_status, to_fail_mode); diff --git a/src/meta/test/cluster_balance_policy_test.cpp b/src/meta/test/cluster_balance_policy_test.cpp index f8da5a3a14..bef85a072a 100644 --- a/src/meta/test/cluster_balance_policy_test.cpp +++ b/src/meta/test/cluster_balance_policy_test.cpp @@ -186,9 +186,10 @@ TEST(cluster_balance_policy, get_node_migration_info) policy.get_node_migration_info(ns, all_apps, migration_info); ASSERT_EQ(migration_info.hp, hp); - ASSERT_NE(migration_info.partitions.find(disk_tag), migration_info.partitions.end()); - ASSERT_EQ(migration_info.partitions.at(disk_tag).size(), 1); - ASSERT_EQ(*migration_info.partitions.at(disk_tag).begin(), pid); + const auto *ps = gutil::FindOrNull(migration_info.partitions, disk_tag); + ASSERT_NE(ps, nullptr); + ASSERT_EQ(ps->size(), 1); + ASSERT_EQ(*ps->begin(), pid); } TEST(cluster_balance_policy, get_min_max_set) diff --git a/src/meta/test/meta_bulk_load_ingestion_test.cpp b/src/meta/test/meta_bulk_load_ingestion_test.cpp index f6ee7e9fc6..b482be5124 100644 --- a/src/meta/test/meta_bulk_load_ingestion_test.cpp +++ b/src/meta/test/meta_bulk_load_ingestion_test.cpp @@ -30,6 +30,7 @@ #include "runtime/rpc/rpc_address.h" #include "runtime/rpc/rpc_host_port.h" #include "utils/fail_point.h" +#include "gutil/map_util.h" namespace dsn { namespace replication { @@ -64,10 +65,11 @@ class node_context_test : public meta_test_base uint32_t get_disk_count(const std::string &disk_tag) { - if (_context.disk_ingesting_counts.find(disk_tag) == _context.disk_ingesting_counts.end()) { + const auto *count = gutil::FindOrNull(_context.disk_ingesting_counts, disk_tag); + if (count == nullptr) { return -1; } - return _context.disk_ingesting_counts[disk_tag]; + return *count; } void mock_get_max_disk_ingestion_count(const uint32_t node_min_disk_count, @@ -255,8 +257,7 @@ class ingestion_context_test : public meta_test_base bool is_partition_ingesting(const uint32_t pidx) const { - return _context->_running_partitions.find(gpid(APP_ID, pidx)) != - _context->_running_partitions.end(); + return gutil::ContainsKey(_context->_running_partitions, gpid(APP_ID, pidx)); } uint32_t get_app_ingesting_count() const { return _context->get_app_ingesting_count(APP_ID); } diff --git a/src/meta/test/meta_bulk_load_service_test.cpp b/src/meta/test/meta_bulk_load_service_test.cpp index 48477b361c..db73222eeb 100644 --- a/src/meta/test/meta_bulk_load_service_test.cpp +++ b/src/meta/test/meta_bulk_load_service_test.cpp @@ -382,14 +382,11 @@ class bulk_load_service_test : public meta_test_base &partition_bulk_load_info_map, &pinfo_map]() { for (const auto app_id : app_id_set) { - auto app_iter = app_bulk_load_info_map.find(app_id); - auto partition_iter = partition_bulk_load_info_map.find(app_id); - if (app_iter != app_bulk_load_info_map.end()) { + const auto *app = gutil::FindOrNull(app_bulk_load_info_map, app_id); + const auto *partition = gutil::FindOrNull(partition_bulk_load_info_map, app_id); + if (app != nullptr) { mock_app_bulk_load_info_on_remote_storage( - app_iter->second, - partition_iter == partition_bulk_load_info_map.end() - ? pinfo_map - : partition_iter->second); + *app, partition == nullptr ? pinfo_map : *partition); } } }); @@ -492,7 +489,7 @@ class bulk_load_service_test : public meta_test_base bool is_app_bulk_load_states_reset(int32_t app_id) { - return bulk_svc()._bulk_load_app_id.find(app_id) == bulk_svc()._bulk_load_app_id.end(); + return !gutil::ContainsKey(bulk_svc()._bulk_load_app_id, app_id); } meta_op_status get_op_status() { return _ms->get_op_status(); } diff --git a/src/nfs/nfs_server_impl.cpp b/src/nfs/nfs_server_impl.cpp index 21a7f3a8af..bdf2a34fd0 100644 --- a/src/nfs/nfs_server_impl.cpp +++ b/src/nfs/nfs_server_impl.cpp @@ -34,6 +34,7 @@ #include #include "absl/strings/string_view.h" +#include "gutil/map_util.h" #include "nfs/nfs_code_definition.h" #include "nlohmann/json.hpp" #include "runtime/api_layer1.h" @@ -95,24 +96,22 @@ void nfs_service_impl::on_copy(const ::dsn::service::copy_request &request, do { zauto_lock l(_handles_map_lock); - auto it = _handles_map.find(file_path); // find file handle cache first - if (it == _handles_map.end()) { + auto fh = gutil::LookupOrInsert(&_handles_map, file_path, nullptr); + if (!fh) { dfile = file::open(file_path, file::FileOpenType::kReadOnly); - if (dfile == nullptr) { + if (dsn_unlikely(dfile == nullptr)) { + gutil::EraseKeyReturnValuePtr(&_handles_map, file_path); LOG_ERROR("[nfs_service] open file {} failed", file_path); ::dsn::service::copy_response resp; resp.error = ERR_OBJECT_NOT_FOUND; reply(resp); return; } - - auto fh = std::make_shared(); fh->file_handle = dfile; - it = _handles_map.insert(std::make_pair(file_path, std::move(fh))).first; } - dfile = it->second->file_handle; - it->second->file_access_count++; - it->second->last_access_time = dsn_now_ms(); + dfile = fh->file_handle; + fh->file_access_count++; + fh->last_access_time = dsn_now_ms(); } while (false); CHECK_NOTNULL(dfile, ""); diff --git a/src/replica/replica_config.cpp b/src/replica/replica_config.cpp index 23767986b6..7abeaf4549 100644 --- a/src/replica/replica_config.cpp +++ b/src/replica/replica_config.cpp @@ -74,6 +74,7 @@ #include "utils/fail_point.h" #include "utils/flags.h" #include "utils/fmt_logging.h" +#include "gutil/map_util.h" #include "utils/string_conv.h" #include "utils/strings.h" #include "utils/thread_access_checker.h" @@ -89,9 +90,9 @@ bool get_bool_envs(const std::map &envs, const std::string &name, bool &value) { - auto iter = envs.find(name); - if (iter != envs.end()) { - if (!buf2bool(iter->second, value)) { + const auto *value_ptr = gutil::FindOrNull(envs, name); + if (value_ptr != nullptr) { + if (!buf2bool(*value_ptr, value)) { return false; } } @@ -202,7 +203,7 @@ void replica::add_potential_secondary(const configuration_update_request &propos _primary_states.pc.hp_secondaries.size() + _primary_states.learners.size(); if (potential_secondaries_count >= _primary_states.pc.max_replica_count - 1) { if (proposal.type == config_type::CT_ADD_SECONDARY) { - if (_primary_states.learners.find(node) == _primary_states.learners.end()) { + if (!gutil::ContainsKey(_primary_states.learners, node)) { LOG_INFO_PREFIX( "already have enough secondaries or potential secondaries, ignore new " "potential secondary proposal"); @@ -225,9 +226,9 @@ void replica::add_potential_secondary(const configuration_update_request &propos state.prepare_start_decree = invalid_decree; state.timeout_task = nullptr; // TODO: add timer for learner task - auto it = _primary_states.learners.find(node); - if (it != _primary_states.learners.end()) { - state.signature = it->second.signature; + const auto *rls = gutil::FindOrNull(_primary_states.learners, node); + if (rls != nullptr) { + state.signature = rls->signature; } else { state.signature = ++_primary_states.next_learning_version; _primary_states.learners[node] = state; @@ -585,9 +586,10 @@ void replica::update_bool_envs(const std::map &envs, void replica::update_ac_allowed_users(const std::map &envs) { std::string allowed_users; - auto iter = envs.find(replica_envs::REPLICA_ACCESS_CONTROLLER_ALLOWED_USERS); - if (iter != envs.end()) { - allowed_users = iter->second; + const auto *env = + gutil::FindOrNull(envs, replica_envs::REPLICA_ACCESS_CONTROLLER_ALLOWED_USERS); + if (env != nullptr) { + allowed_users = *env; } _access_controller->update_allowed_users(allowed_users); @@ -595,9 +597,10 @@ void replica::update_ac_allowed_users(const std::map & void replica::update_ac_ranger_policies(const std::map &envs) { - auto iter = envs.find(replica_envs::REPLICA_ACCESS_CONTROLLER_RANGER_POLICIES); - if (iter != envs.end()) { - _access_controller->update_ranger_policies(iter->second); + const auto *env = + gutil::FindOrNull(envs, replica_envs::REPLICA_ACCESS_CONTROLLER_RANGER_POLICIES); + if (env != nullptr) { + _access_controller->update_ranger_policies(*env); } } @@ -623,14 +626,14 @@ void replica::update_allow_ingest_behind(const std::map &envs) { - auto env_iter = envs.find(replica_envs::DENY_CLIENT_REQUEST); - if (env_iter == envs.end()) { + const auto *env = gutil::FindOrNull(envs, replica_envs::DENY_CLIENT_REQUEST); + if (env == nullptr) { _deny_client.reset(); return; } std::vector sub_sargs; - utils::split_args(env_iter->second.c_str(), sub_sargs, '*', true); + utils::split_args(env->c_str(), sub_sargs, '*', true); CHECK_EQ_PREFIX(sub_sargs.size(), 2); _deny_client.reconfig = (sub_sargs[0] == "reconfig"); diff --git a/src/utils/metrics.h b/src/utils/metrics.h index 2990537203..199d11d483 100644 --- a/src/utils/metrics.h +++ b/src/utils/metrics.h @@ -52,6 +52,7 @@ #include "utils/fmt_logging.h" #include "utils/long_adder.h" #include "utils/macros.h" +#include "gutil/map_util.h" #include "utils/nth_element.h" #include "utils/ports.h" #include "utils/singleton.h" @@ -1746,13 +1747,13 @@ inline error_s parse_metric_attribute(const metric_entity::attr_map &attrs, const std::string &name, TAttrValue &value) { - const auto &iter = attrs.find(name); - if (dsn_unlikely(iter == attrs.end())) { + const auto *value_ptr = gutil::FindOrNull(attrs, name); + if (dsn_unlikely(value_ptr == nullptr)) { return FMT_ERR(dsn::ERR_INVALID_DATA, "{} field was not found", name); } - if (dsn_unlikely(!dsn::buf2numeric(iter->second, value))) { - return FMT_ERR(dsn::ERR_INVALID_DATA, "invalid {}: {}", name, iter->second); + if (dsn_unlikely(!dsn::buf2numeric(*value_ptr, value))) { + return FMT_ERR(dsn::ERR_INVALID_DATA, "invalid {}: {}", name, *value_ptr); } return dsn::error_s::ok(); diff --git a/src/utils/test/logger.cpp b/src/utils/test/logger.cpp index 0ef7bcb220..04ecb056f3 100644 --- a/src/utils/test/logger.cpp +++ b/src/utils/test/logger.cpp @@ -35,6 +35,7 @@ #include #include "gtest/gtest.h" +#include "gutil/map_util.h" #include "utils/api_utilities.h" #include "utils/filesystem.h" #include "utils/flags.h" @@ -85,7 +86,7 @@ class simple_logger_test : public logger_test for (const auto &path : sub_list) { std::string name(utils::filesystem::get_file_name(path)); if (std::regex_match(name, pattern)) { - ASSERT_TRUE(file_names.insert(name).second); + ASSERT_TRUE(gutil::InsertIfNotPresent(&file_names, name)); } } } diff --git a/src/utils/test/metrics_test.cpp b/src/utils/test/metrics_test.cpp index 9f3a4d1909..d21ff30983 100644 --- a/src/utils/test/metrics_test.cpp +++ b/src/utils/test/metrics_test.cpp @@ -39,6 +39,7 @@ #include "runtime/rpc/rpc_message.h" #include "utils/errors.h" #include "utils/flags.h" +#include "gutil/map_util.h" #include "utils/rand.h" #include "utils/strings.h" #include "utils/test/nth_element_utils.h" @@ -246,7 +247,7 @@ TEST(metrics_test, create_entity) auto attrs = entity->attributes(); ASSERT_EQ(attrs, test.entity_attrs); - ASSERT_EQ(entities.find(test.entity_id), entities.end()); + ASSERT_TRUE(!gutil::ContainsKey(entities, test.entity_id)); entities[test.entity_id] = entity; } @@ -315,18 +316,14 @@ TEST(metrics_test, create_metric) ASSERT_EQ(my_metric->value(), test.value); - auto iter = expected_entities.find(test.entity.get()); - if (iter == expected_entities.end()) { - expected_entities[test.entity.get()] = {{test.prototype, my_metric}}; - } else { - iter->second[test.prototype] = my_metric; - } + auto iter = gutil::LookupOrInsert(&expected_entities, test.entity.get(), {}); + iter.emplace(test.prototype, my_metric); } entity_map actual_entities; auto entities = metric_registry::instance().entities(); for (const auto &entity : entities) { - if (expected_entities.find(entity.second.get()) != expected_entities.end()) { + if (gutil::ContainsKey(expected_entities, entity.second.get())) { actual_entities[entity.second.get()] = entity.second->metrics(); } } @@ -778,7 +775,7 @@ void run_percentile(const metric_entity_ptr &my_entity, std::vector actual_elements; for (const auto &kth : kAllKthPercentileTypes) { T value; - if (kth_percentiles.find(kth) == kth_percentiles.end()) { + if (!gutil::ContainsKey(kth_percentiles, kth)) { ASSERT_FALSE(my_metric->get(kth, value)); checker(value, 0); } else { @@ -1099,8 +1096,7 @@ void compare_floating_metric_value_map(const metric_value_map &actual_value_m filters.with_metric_fields = metric_fields; \ \ metric_value_map expected_value_map; \ - if (expected_metric_fields.find(kMetricSingleValueField) != \ - expected_metric_fields.end()) { \ + if (gutil::ContainsKey(expected_metric_fields, kMetricSingleValueField)) { \ expected_value_map[kMetricSingleValueField] = test.expected_value; \ } \ \ @@ -1283,7 +1279,7 @@ void generate_metric_value_map(MetricType *my_metric, for (const auto &type : kth_percentiles) { auto name = kth_percentile_to_name(type); // Only add the chosen fields to the expected value map. - if (expected_metric_fields.find(name) != expected_metric_fields.end()) { + if (gutil::ContainsKey(expected_metric_fields, name)) { value_map[name] = *value; } ++value; @@ -2871,13 +2867,11 @@ TEST(metrics_test, http_get_metrics) for (const auto &test : tests) { entity_container expected_entities; for (const auto &entity_pair : test.expected_entity_metrics) { - const auto &iter = entities.find(entity_pair.first); - ASSERT_NE(entities.end(), iter); - - const auto &entity = iter->second; - expected_entities.emplace(entity->id(), - entity_properties{entity->prototype()->name(), - entity->attributes(), + const auto *entity = gutil::FindOrNull(entities, entity_pair.first); + ASSERT_NE(entity, nullptr); + expected_entities.emplace((*entity)->id(), + entity_properties{(*entity)->prototype()->name(), + (*entity)->attributes(), entity_pair.second}); } @@ -3131,11 +3125,11 @@ void scoped_entity::test_survival_immediately_after_initialization() const // Use internal member directly instead of calling entities(). We don't want to have // any reference which may affect the test results. const auto &entities = metric_registry::instance()._entities; - const auto &iter = entities.find(_my_entity_id); - ASSERT_NE(entities.end(), iter); - ASSERT_EQ(_expected_my_entity_raw_ptr, iter->second.get()); + const auto *entity = gutil::FindOrNull(entities, _my_entity_id); + ASSERT_NE(entity, nullptr); + ASSERT_EQ(_expected_my_entity_raw_ptr, entity->get()); - const auto &actual_surviving_metrics = get_actual_surviving_metrics(iter->second); + const auto &actual_surviving_metrics = get_actual_surviving_metrics(*entity); ASSERT_EQ(_expected_all_metrics, actual_surviving_metrics); } @@ -3149,18 +3143,18 @@ void scoped_entity::test_survival_after_retirement() const // Use internal member directly instead of calling entities(). We don't want to have // any reference which may affect the test results. const auto &entities = metric_registry::instance()._entities; - const auto &iter = entities.find(_my_entity_id); + const auto *iter = gutil::FindOrNull(entities, _my_entity_id); if (_my_entity == nullptr) { // The entity has been retired. - ASSERT_EQ(entities.end(), iter); + ASSERT_EQ(iter, nullptr); ASSERT_TRUE(_expected_surviving_metrics.empty()); return; } - ASSERT_NE(entities.end(), iter); - ASSERT_EQ(_expected_my_entity_raw_ptr, iter->second.get()); + ASSERT_NE(iter, nullptr); + ASSERT_EQ(_expected_my_entity_raw_ptr, iter->get()); - const auto &actual_surviving_metrics = get_actual_surviving_metrics(iter->second); + const auto &actual_surviving_metrics = get_actual_surviving_metrics(*iter); ASSERT_EQ(_expected_surviving_metrics, actual_surviving_metrics); } From 6aa5681f267eb510f61d5e8d2a7b7f7b1e56a552 Mon Sep 17 00:00:00 2001 From: Yingchun Lai Date: Sun, 21 Jul 2024 23:25:24 +0800 Subject: [PATCH 2/4] fix ut --- src/gutil/test/CMakeLists.txt | 4 +- src/gutil/test/config.ini | 75 ---------------------------- src/gutil/test/main.cpp | 4 ++ src/gutil/test/map_util_unittest.cpp | 9 ++-- src/utils/simple_logger.cpp | 15 ++++-- 5 files changed, 22 insertions(+), 85 deletions(-) delete mode 100644 src/gutil/test/config.ini diff --git a/src/gutil/test/CMakeLists.txt b/src/gutil/test/CMakeLists.txt index af9b3ca69c..df75977b5f 100644 --- a/src/gutil/test/CMakeLists.txt +++ b/src/gutil/test/CMakeLists.txt @@ -27,6 +27,6 @@ set(MY_PROJ_LIBS gmock gtest) set(MY_BINPLACES - "${CMAKE_CURRENT_SOURCE_DIR}/run.sh" -) + run.sh + config.ini) dsn_add_test() diff --git a/src/gutil/test/config.ini b/src/gutil/test/config.ini deleted file mode 100644 index 744d8d412a..0000000000 --- a/src/gutil/test/config.ini +++ /dev/null @@ -1,75 +0,0 @@ -; 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. - -[apps..default] -run = true -count = 1 -network.client.RPC_CHANNEL_TCP = dsn::tools::asio_network_provider, 65536 -network.client.RPC_CHANNEL_UDP = dsn::tools::asio_udp_provider, 65536 -network.server.0.RPC_CHANNEL_TCP = dsn::tools::asio_network_provider, 65536 -network.server.0.RPC_CHANNEL_UDP = dsn::tools::asio_udp_provider, 65536 - -[apps.test] -type = test -arguments = -run = true -ports = 20001 -count = 1 -pools = THREAD_POOL_DEFAULT - -[core] -tool = nativerun - -toollets = tracer, profiler -pause_on_start = false - -logging_start_level = LOG_LEVEL_DEBUG -logging_factory_name = dsn::tools::simple_logger - -[tools.simple_logger] -fast_flush = true -short_header = false -stderr_start_level = LOG_LEVEL_DEBUG - -[network] -; how many network threads for network library (used by asio) -io_service_worker_count = 2 - -[task..default] -is_trace = true -is_profile = true -allow_inline = false -rpc_call_channel = RPC_CHANNEL_TCP -rpc_message_header_format = dsn -rpc_timeout_milliseconds = 1000 - -[task.LPC_RPC_TIMEOUT] -is_trace = false -is_profile = false - -[task.RPC_TEST_UDP] -rpc_call_channel = RPC_CHANNEL_UDP -rpc_message_crc_required = true - -; specification for each thread pool -[threadpool..default] -worker_count = 2 - -[threadpool.THREAD_POOL_DEFAULT] -partitioned = false -worker_priority = THREAD_xPRIORITY_NORMAL - diff --git a/src/gutil/test/main.cpp b/src/gutil/test/main.cpp index 7473ada88a..317872d4f1 100644 --- a/src/gutil/test/main.cpp +++ b/src/gutil/test/main.cpp @@ -19,10 +19,14 @@ #include #include "runtime/app_model.h" +#include "utils/flags.h" + +DSN_DECLARE_string(stderr_start_level); GTEST_API_ int main(int argc, char **argv) { testing::InitGoogleTest(&argc, argv); + FLAGS_stderr_start_level = "LOG_LEVEL_ERROR"; int ret = RUN_ALL_TESTS(); dsn_exit(ret); } diff --git a/src/gutil/test/map_util_unittest.cpp b/src/gutil/test/map_util_unittest.cpp index fae676f894..78a00d16a1 100644 --- a/src/gutil/test/map_util_unittest.cpp +++ b/src/gutil/test/map_util_unittest.cpp @@ -37,6 +37,7 @@ #include "absl/container/btree_map.h" #include "absl/container/btree_set.h" #include "map_util_test.h" +#include "utils/flags.h" namespace gutil { @@ -193,7 +194,7 @@ TEST(MapUtilDeathTest, FindOrDieNoPrint) std::map m; m[Unprintable(1)] = 8; EXPECT_EQ(8, FindOrDieNoPrint(m, Unprintable(1))); - ASSERT_DEATH(FindOrDieNoPrint(m, Unprintable(2)), ""); // Map key not found + ASSERT_DEATH(FindOrDieNoPrint(m, Unprintable(2)), "Map key not found"); // Make sure the non-const reference returning version works. FindOrDieNoPrint(m, Unprintable(1)) = 20; @@ -209,7 +210,7 @@ TEST(MapUtilDeathTest, SetInsertOrDieTest) std::set s; InsertOrDie(&s, 1); EXPECT_TRUE(ContainsKey(s, 1)); - ASSERT_DEATH(InsertOrDie(&s, 1), ""); // duplicate + ASSERT_DEATH(InsertOrDie(&s, 1), "duplicate"); } TEST(MapUtilDeathTest, InsertOrDieNoPrint) @@ -219,12 +220,12 @@ TEST(MapUtilDeathTest, InsertOrDieNoPrint) std::map, int> m; InsertOrDieNoPrint(&m, key, 2); EXPECT_EQ(m[key], 2); - ASSERT_DEATH(InsertOrDieNoPrint(&m, key, 3), ""); // duplicate + ASSERT_DEATH(InsertOrDieNoPrint(&m, key, 3), "duplicate"); std::set> s; InsertOrDieNoPrint(&s, key); EXPECT_TRUE(ContainsKey(s, key)); - ASSERT_DEATH(InsertOrDieNoPrint(&s, key), ""); // duplicate + ASSERT_DEATH(InsertOrDieNoPrint(&s, key), "duplicate"); } TEST(MapUtil, InsertKeysFromMap) diff --git a/src/utils/simple_logger.cpp b/src/utils/simple_logger.cpp index 86145d4a8e..ee4ca37b1b 100644 --- a/src/utils/simple_logger.cpp +++ b/src/utils/simple_logger.cpp @@ -171,7 +171,8 @@ inline void process_fatal_log(log_level_t log_level) void screen_logger::print_header(log_level_t log_level) { - ::dsn::tools::print_header(stdout, LOG_LEVEL_COUNT, log_level); + ::dsn::tools::print_header( + stdout, enum_from_string(FLAGS_stderr_start_level, LOG_LEVEL_INVALID), log_level); } void screen_logger::print_long_header(const char *file, @@ -179,13 +180,19 @@ void screen_logger::print_long_header(const char *file, const int line, log_level_t log_level) { - ::dsn::tools::print_long_header( - stdout, file, function, line, _short_header, LOG_LEVEL_COUNT, log_level); + ::dsn::tools::print_long_header(stdout, + file, + function, + line, + _short_header, + enum_from_string(FLAGS_stderr_start_level, LOG_LEVEL_INVALID), + log_level); } void screen_logger::print_body(const char *body, log_level_t log_level) { - ::dsn::tools::print_body(stdout, body, LOG_LEVEL_COUNT, log_level); + ::dsn::tools::print_body( + stdout, body, enum_from_string(FLAGS_stderr_start_level, LOG_LEVEL_INVALID), log_level); } void screen_logger::log( From 08b84472e1d484bf5e0e3c22ca2a4f1ad4bd7bf3 Mon Sep 17 00:00:00 2001 From: Yingchun Lai Date: Sun, 21 Jul 2024 23:26:59 +0800 Subject: [PATCH 3/4] 1 --- src/gutil/test/CMakeLists.txt | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/gutil/test/CMakeLists.txt b/src/gutil/test/CMakeLists.txt index df75977b5f..4e3c97a540 100644 --- a/src/gutil/test/CMakeLists.txt +++ b/src/gutil/test/CMakeLists.txt @@ -27,6 +27,5 @@ set(MY_PROJ_LIBS gmock gtest) set(MY_BINPLACES - run.sh - config.ini) + run.sh) dsn_add_test() From 60b375cf2fb1ddf185388cc20444b3a619665d13 Mon Sep 17 00:00:00 2001 From: Yingchun Lai Date: Sun, 21 Jul 2024 23:28:47 +0800 Subject: [PATCH 4/4] 1 --- src/gutil/test/main.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/gutil/test/main.cpp b/src/gutil/test/main.cpp index 317872d4f1..f63bdb435e 100644 --- a/src/gutil/test/main.cpp +++ b/src/gutil/test/main.cpp @@ -26,7 +26,5 @@ DSN_DECLARE_string(stderr_start_level); GTEST_API_ int main(int argc, char **argv) { testing::InitGoogleTest(&argc, argv); - FLAGS_stderr_start_level = "LOG_LEVEL_ERROR"; - int ret = RUN_ALL_TESTS(); - dsn_exit(ret); + dsn_exit(RUN_ALL_TESTS()); }