From 7670ea50c166bc15c9e2ac0a8e500f3f6d1eda66 Mon Sep 17 00:00:00 2001 From: Mike Kot Date: Tue, 28 Sep 2021 23:01:16 +0200 Subject: [PATCH 1/7] Replacing std::function to Fn in some places to avoid dynamic allocating while keeping desired function signature clear. Simplifying SimpleCache (CachedFn) using C++20 --- base/common/CachedFn.h | 74 +++++++++++++++++ base/common/FnTraits.h | 37 +++++++++ base/common/SimpleCache.h | 82 ------------------- base/common/function_traits.h | 16 ---- src/Access/AllowedClientHosts.cpp | 1 - src/Access/IAccessStorage.cpp | 3 +- src/Access/RoleCache.cpp | 5 +- src/Access/UsersConfigAccessStorage.cpp | 7 +- src/Common/DNSResolver.cpp | 6 +- src/Common/StackTrace.cpp | 18 ++-- src/Common/examples/simple_cache.cpp | 4 +- src/Databases/MySQL/MaterializeMetadata.cpp | 3 +- .../fetchPostgreSQLTableStructure.cpp | 11 ++- src/Disks/HDFS/DiskHDFS.cpp | 3 +- src/Disks/S3/DiskS3.cpp | 3 +- src/Interpreters/Cluster.cpp | 1 - 16 files changed, 150 insertions(+), 124 deletions(-) create mode 100644 base/common/CachedFn.h create mode 100644 base/common/FnTraits.h delete mode 100644 base/common/SimpleCache.h delete mode 100644 base/common/function_traits.h diff --git a/base/common/CachedFn.h b/base/common/CachedFn.h new file mode 100644 index 00000000000..6ecae0b88b3 --- /dev/null +++ b/base/common/CachedFn.h @@ -0,0 +1,74 @@ +#pragma once + +#include +#include +#include "FnTraits.h" + +/** + * Cache for a functor that decays to a pointer-to-function. + * The size is unlimited. Values are stored permanently and never evicted. + * Suitable only for simplest cases. + * + * Invocation/update is O(log(saved cache values)). + * + * @example CachedFn<&my_func> cached; cached(arg); + */ +template +struct CachedFn +{ +private: + using Traits = FnTraits; + using Key = typename Traits::DecayedArgs; + using Result = typename Traits::Ret; + + std::map cache; // Can't use hashmap as tuples are unhashable by default + mutable std::mutex mutex; + +public: + template + Result operator()(Args && ...args) + { + Key key{std::forward(args)...}; + + { + std::lock_guard lock(mutex); + + if (auto it = cache.find(key); it != cache.end()) + return it->second; + } + + /// The calculations themselves are not done under mutex. + Result res = std::apply(Func, key); + + { + std::lock_guard lock(mutex); + cache.emplace(std::move(key), res); + } + + return res; + } + + template + void update(Args && ...args) + { + Key key{std::forward(args)...}; + Result res = std::apply(Func, key); + + { + std::lock_guard lock(mutex); + cache.emplace(std::move(key), std::move(res)); + } + } + + size_t size() const + { + std::lock_guard lock(mutex); + return cache.size(); + } + + void drop() + { + std::lock_guard lock(mutex); + cache.clear(); + } +}; diff --git a/base/common/FnTraits.h b/base/common/FnTraits.h new file mode 100644 index 00000000000..d90cb530a09 --- /dev/null +++ b/base/common/FnTraits.h @@ -0,0 +1,37 @@ +#pragma once + +#include +#include +#include + +namespace detail +{ +template +struct FnTraits { template static constexpr bool value = false; }; + +template +struct FnTraits +{ + template + static constexpr bool value = std::is_invocable_r_v; + + using Ret = R; + using Args = std::tuple; + using DecayedArgs = std::tuple::type...>; +}; + +template +struct FnTraits : FnTraits {}; +} + +template using FnTraits = detail::FnTraits; + +/** + * A less-typing alias for std::is_invokable_r_v. + * @example void foo(Fn auto && functor) + */ +template +concept Fn = FnTraits::template value; + +template +using Constant = std::integral_constant; diff --git a/base/common/SimpleCache.h b/base/common/SimpleCache.h deleted file mode 100644 index c3bf019c226..00000000000 --- a/base/common/SimpleCache.h +++ /dev/null @@ -1,82 +0,0 @@ -#pragma once - -#include -#include -#include -#include - - -/** The simplest cache for a free function. - * You can also pass a static class method or lambda without captures. - * The size is unlimited. Values are stored permanently and never evicted. - * But single record or all cache can be manually dropped. - * Mutex is used for synchronization. - * Suitable only for the simplest cases. - * - * Usage - * - * SimpleCache func_cached; - * std::cerr << func_cached(args...); - */ -template -class SimpleCache -{ -private: - using Key = typename function_traits::arguments_decay; - using Result = typename function_traits::result; - - std::map cache; - mutable std::mutex mutex; - -public: - template - Result operator() (Args &&... args) - { - Key key{std::forward(args)...}; - - { - std::lock_guard lock(mutex); - - auto it = cache.find(key); - - if (cache.end() != it) - return it->second; - } - - /// The calculations themselves are not done under mutex. - Result res = std::apply(f, key); - - { - std::lock_guard lock(mutex); - - cache.emplace(std::forward_as_tuple(args...), res); - } - - return res; - } - - template - void update(Args &&... args) - { - Key key{std::forward(args)...}; - - Result res = std::apply(f, key); - - { - std::lock_guard lock(mutex); - cache[key] = std::move(res); - } - } - - size_t size() const - { - std::lock_guard lock(mutex); - return cache.size(); - } - - void drop() - { - std::lock_guard lock(mutex); - cache.clear(); - } -}; diff --git a/base/common/function_traits.h b/base/common/function_traits.h deleted file mode 100644 index 9cd104925a7..00000000000 --- a/base/common/function_traits.h +++ /dev/null @@ -1,16 +0,0 @@ -#pragma once - -#include -#include - - -template -struct function_traits; - -template -struct function_traits -{ - using result = ReturnType; - using arguments = std::tuple; - using arguments_decay = std::tuple::type...>; -}; diff --git a/src/Access/AllowedClientHosts.cpp b/src/Access/AllowedClientHosts.cpp index 5ac05bea88d..88061213c9a 100644 --- a/src/Access/AllowedClientHosts.cpp +++ b/src/Access/AllowedClientHosts.cpp @@ -1,6 +1,5 @@ #include #include -#include #include #include #include diff --git a/src/Access/IAccessStorage.cpp b/src/Access/IAccessStorage.cpp index f0fbb95ff4e..ce05cb7da0f 100644 --- a/src/Access/IAccessStorage.cpp +++ b/src/Access/IAccessStorage.cpp @@ -6,6 +6,7 @@ #include #include #include +#include namespace DB @@ -96,7 +97,7 @@ namespace bool errors() const { return exception.has_value(); } - void showErrors(const char * format, const std::function & get_name_function) + void showErrors(const char * format, Fn auto && get_name_function) { if (!exception) return; diff --git a/src/Access/RoleCache.cpp b/src/Access/RoleCache.cpp index d264ec2d953..db0d5863268 100644 --- a/src/Access/RoleCache.cpp +++ b/src/Access/RoleCache.cpp @@ -3,6 +3,7 @@ #include #include #include +#include namespace DB @@ -11,7 +12,7 @@ namespace { void collectRoles(EnabledRolesInfo & roles_info, boost::container::flat_set & skip_ids, - const std::function & get_role_function, + Fn auto && get_role_function, const UUID & role_id, bool is_current_role, bool with_admin_option) @@ -113,7 +114,9 @@ void RoleCache::collectEnabledRoles(EnabledRoles & enabled, scope_guard & notifi /// Collect enabled roles. That includes the current roles, the roles granted to the current roles, and so on. auto new_info = std::make_shared(); boost::container::flat_set skip_ids; + auto get_role_function = [this](const UUID & id) { return getRole(id); }; + for (const auto & current_role : enabled.params.current_roles) collectRoles(*new_info, skip_ids, get_role_function, current_role, true, false); diff --git a/src/Access/UsersConfigAccessStorage.cpp b/src/Access/UsersConfigAccessStorage.cpp index 68333ed45da..9c6ebf5c643 100644 --- a/src/Access/UsersConfigAccessStorage.cpp +++ b/src/Access/UsersConfigAccessStorage.cpp @@ -18,6 +18,7 @@ #include #include #include +#include namespace DB @@ -362,7 +363,7 @@ namespace SettingsProfileElements parseSettingsConstraints(const Poco::Util::AbstractConfiguration & config, const String & path_to_constraints, - const std::function & check_setting_name_function) + Fn auto && check_setting_name_function) { SettingsProfileElements profile_elements; Poco::Util::AbstractConfiguration::Keys keys; @@ -399,7 +400,7 @@ namespace std::shared_ptr parseSettingsProfile( const Poco::Util::AbstractConfiguration & config, const String & profile_name, - const std::function & check_setting_name_function) + Fn auto && check_setting_name_function) { auto profile = std::make_shared(); profile->setName(profile_name); @@ -441,7 +442,7 @@ namespace std::vector parseSettingsProfiles( const Poco::Util::AbstractConfiguration & config, - const std::function & check_setting_name_function) + Fn auto && check_setting_name_function) { std::vector profiles; Poco::Util::AbstractConfiguration::Keys profile_names; diff --git a/src/Common/DNSResolver.cpp b/src/Common/DNSResolver.cpp index 4fe0f0bb8c8..2aa62b879a3 100644 --- a/src/Common/DNSResolver.cpp +++ b/src/Common/DNSResolver.cpp @@ -1,5 +1,5 @@ #include "DNSResolver.h" -#include +#include #include #include #include @@ -146,8 +146,8 @@ static String reverseResolveImpl(const Poco::Net::IPAddress & address) struct DNSResolver::Impl { - SimpleCache cache_host; - SimpleCache cache_address; + CachedFn<&resolveIPAddressImpl> cache_host; + CachedFn<&reverseResolveImpl> cache_address; std::mutex drop_mutex; std::mutex update_mutex; diff --git a/src/Common/StackTrace.cpp b/src/Common/StackTrace.cpp index a9d4700490e..2d83895a618 100644 --- a/src/Common/StackTrace.cpp +++ b/src/Common/StackTrace.cpp @@ -5,7 +5,7 @@ #include #include #include -#include +#include #include #include @@ -21,7 +21,7 @@ # include #endif -std::string signalToErrorMessage(int sig, const siginfo_t & info, const ucontext_t & context) +std::string signalToErrorMessage(int sig, const siginfo_t & info, [[maybe_unused]] const ucontext_t & context) { std::stringstream error; // STYLE_CHECK_ALLOW_STD_STRING_STREAM error.exceptions(std::ios::failbit); @@ -41,8 +41,6 @@ std::string signalToErrorMessage(int sig, const siginfo_t & info, const ucontext error << " Access: write."; else error << " Access: read."; -#else - UNUSED(context); #endif switch (info.si_code) @@ -197,7 +195,9 @@ static void * getCallerAddress(const ucontext_t & context) #endif } -void StackTrace::symbolize(const StackTrace::FramePointers & frame_pointers, size_t offset, size_t size, StackTrace::Frames & frames) +void StackTrace::symbolize( + const StackTrace::FramePointers & frame_pointers, [[maybe_unused]] size_t offset, + size_t size, StackTrace::Frames & frames) { #if defined(__ELF__) && !defined(__FreeBSD__) && !defined(ARCADIA_BUILD) @@ -256,7 +256,6 @@ void StackTrace::symbolize(const StackTrace::FramePointers & frame_pointers, siz { frames[i].virtual_addr = frame_pointers[i]; } - UNUSED(offset); #endif } @@ -322,7 +321,7 @@ const StackTrace::FramePointers & StackTrace::getFramePointers() const } static void toStringEveryLineImpl( - bool fatal, + [[maybe_unused]] bool fatal, const StackTrace::FramePointers & frame_pointers, size_t offset, size_t size, @@ -386,7 +385,6 @@ static void toStringEveryLineImpl( out.str({}); } #else - UNUSED(fatal); std::stringstream out; // STYLE_CHECK_ALLOW_STD_STRING_STREAM out.exceptions(std::ios::failbit); @@ -431,9 +429,9 @@ std::string StackTrace::toString(void ** frame_pointers_, size_t offset, size_t return toStringStatic(frame_pointers_copy, offset, size); } -static SimpleCache & cacheInstance() +static CachedFn<&toStringImpl> & cacheInstance() { - static SimpleCache cache; + static CachedFn<&toStringImpl> cache; return cache; } diff --git a/src/Common/examples/simple_cache.cpp b/src/Common/examples/simple_cache.cpp index 8fe18c5594c..b6bcfc0b932 100644 --- a/src/Common/examples/simple_cache.cpp +++ b/src/Common/examples/simple_cache.cpp @@ -1,5 +1,5 @@ #include -#include +#include static int func(int x, int y) @@ -11,7 +11,7 @@ static int func(int x, int y) int main(int, char **) { - SimpleCache func_cached; + CachedFn<&func> func_cached; std::cerr << func_cached(1, 2) << "\n"; std::cerr << func_cached(1, 2) << "\n"; diff --git a/src/Databases/MySQL/MaterializeMetadata.cpp b/src/Databases/MySQL/MaterializeMetadata.cpp index f684797c675..380a68521ca 100644 --- a/src/Databases/MySQL/MaterializeMetadata.cpp +++ b/src/Databases/MySQL/MaterializeMetadata.cpp @@ -15,6 +15,7 @@ #include #include #include +#include namespace fs = std::filesystem; @@ -221,7 +222,7 @@ bool MaterializeMetadata::checkBinlogFileExists(const mysqlxx::PoolWithFailover: return false; } -void commitMetadata(const std::function & function, const String & persistent_tmp_path, const String & persistent_path) +void commitMetadata(Fn auto && function, const String & persistent_tmp_path, const String & persistent_path) { try { diff --git a/src/Databases/PostgreSQL/fetchPostgreSQLTableStructure.cpp b/src/Databases/PostgreSQL/fetchPostgreSQLTableStructure.cpp index 1b77947264e..731e8f639d8 100644 --- a/src/Databases/PostgreSQL/fetchPostgreSQLTableStructure.cpp +++ b/src/Databases/PostgreSQL/fetchPostgreSQLTableStructure.cpp @@ -14,6 +14,7 @@ #include #include #include +#include namespace DB @@ -41,7 +42,7 @@ std::unordered_set fetchPostgreSQLTablesList(T & tx, const String & } -static DataTypePtr convertPostgreSQLDataType(String & type, const std::function & recheck_array, bool is_nullable = false, uint16_t dimensions = 0) +static DataTypePtr convertPostgreSQLDataType(String & type, Fn auto && recheck_array, bool is_nullable = false, uint16_t dimensions = 0) { DataTypePtr res; bool is_array = false; @@ -290,12 +291,20 @@ PostgreSQLTableStructure fetchPostgreSQLTableStructure( pqxx::ReplicationTransaction & tx, const String & postgres_table_name, bool use_nulls, bool with_primary_key, bool with_replica_identity_index); +template +PostgreSQLTableStructure fetchPostgreSQLTableStructure( + pqxx::nontransaction & tx, const String & postgres_table_name, bool use_nulls, + bool with_primary_key, bool with_replica_identity_index); + template std::unordered_set fetchPostgreSQLTablesList(pqxx::work & tx, const String & postgres_schema); template std::unordered_set fetchPostgreSQLTablesList(pqxx::ReadTransaction & tx, const String & postgres_schema); +template +std::unordered_set fetchPostgreSQLTablesList(pqxx::nontransaction & tx, const String & postgres_schema); + } #endif diff --git a/src/Disks/HDFS/DiskHDFS.cpp b/src/Disks/HDFS/DiskHDFS.cpp index 5b15ee69cb8..293a8ec35cc 100644 --- a/src/Disks/HDFS/DiskHDFS.cpp +++ b/src/Disks/HDFS/DiskHDFS.cpp @@ -6,6 +6,7 @@ #include #include #include +#include namespace DB @@ -36,7 +37,7 @@ public: chunks.back().push_back(path.data()); } - void removePaths(const std::function & remove_chunk_func) + void removePaths(Fn auto && remove_chunk_func) { for (auto & chunk : chunks) remove_chunk_func(std::move(chunk)); diff --git a/src/Disks/S3/DiskS3.cpp b/src/Disks/S3/DiskS3.cpp index 30cd314702b..d66b51c7e13 100644 --- a/src/Disks/S3/DiskS3.cpp +++ b/src/Disks/S3/DiskS3.cpp @@ -10,6 +10,7 @@ #include #include +#include #include #include @@ -74,7 +75,7 @@ public: chunks.back().push_back(obj); } - void removePaths(const std::function & remove_chunk_func) + void removePaths(Fn auto && remove_chunk_func) { for (auto & chunk : chunks) remove_chunk_func(std::move(chunk)); diff --git a/src/Interpreters/Cluster.cpp b/src/Interpreters/Cluster.cpp index 1a7554a8c27..625785d18c9 100644 --- a/src/Interpreters/Cluster.cpp +++ b/src/Interpreters/Cluster.cpp @@ -1,5 +1,4 @@ #include -#include #include #include #include From 5e560ff9d795587fd1ecfb0c8f60d4d73039cb67 Mon Sep 17 00:00:00 2001 From: Mike Kot Date: Fri, 1 Oct 2021 16:55:01 +0200 Subject: [PATCH 2/7] fix --- CMakeLists.txt | 13 ++++---- base/common/CachedFn.h | 9 +++-- src/Common/examples/CMakeLists.txt | 3 -- src/Common/examples/simple_cache.cpp | 22 ------------ src/Common/tests/gtest_cached_fn.cpp | 50 ++++++++++++++++++++++++++++ 5 files changed, 60 insertions(+), 37 deletions(-) delete mode 100644 src/Common/examples/simple_cache.cpp create mode 100644 src/Common/tests/gtest_cached_fn.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index e0fa6d8e197..9c8903d853c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -152,6 +152,7 @@ if (CMAKE_GENERATOR STREQUAL "Ninja" AND NOT DISABLE_COLORED_BUILD) set (CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fdiagnostics-color=always") endif () +include (cmake/check_flags.cmake) include (cmake/add_warning.cmake) if (NOT MSVC) @@ -166,7 +167,8 @@ if (COMPILER_CLANG) set(COMPILER_FLAGS "${COMPILER_FLAGS} -gdwarf-aranges") endif () - if (CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL 12.0.0) + if (HAS_USE_CTOR_HOMING) + # For more info see https://blog.llvm.org/posts/2021-04-05-constructor-homing-for-debug-info/ if (CMAKE_BUILD_TYPE_UC STREQUAL "DEBUG" OR CMAKE_BUILD_TYPE_UC STREQUAL "RELWITHDEBINFO") set (CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Xclang -fuse-ctor-homing") set (CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Xclang -fuse-ctor-homing") @@ -192,7 +194,7 @@ endif () # Make sure the final executable has symbols exported set (CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -rdynamic") -find_program (OBJCOPY_PATH NAMES "llvm-objcopy" "llvm-objcopy-12" "llvm-objcopy-11" "llvm-objcopy-10" "llvm-objcopy-9" "llvm-objcopy-8" "objcopy") +find_program (OBJCOPY_PATH NAMES "llvm-objcopy" "llvm-objcopy-13" "llvm-objcopy-12" "llvm-objcopy-11" "llvm-objcopy-10" "llvm-objcopy-9" "llvm-objcopy-8" "objcopy") if (NOT OBJCOPY_PATH AND OS_DARWIN) find_program (BREW_PATH NAMES "brew") @@ -379,7 +381,7 @@ if (COMPILER_CLANG) endif () # Always prefer llvm tools when using clang. For instance, we cannot use GNU ar when llvm LTO is enabled - find_program (LLVM_AR_PATH NAMES "llvm-ar" "llvm-ar-12" "llvm-ar-11" "llvm-ar-10" "llvm-ar-9" "llvm-ar-8") + find_program (LLVM_AR_PATH NAMES "llvm-ar" "llvm-ar-13" "llvm-ar-12" "llvm-ar-11" "llvm-ar-10" "llvm-ar-9" "llvm-ar-8") if (LLVM_AR_PATH) message(STATUS "Using llvm-ar: ${LLVM_AR_PATH}.") @@ -388,7 +390,7 @@ if (COMPILER_CLANG) message(WARNING "Cannot find llvm-ar. System ar will be used instead. It does not work with ThinLTO.") endif () - find_program (LLVM_RANLIB_PATH NAMES "llvm-ranlib" "llvm-ranlib-12" "llvm-ranlib-11" "llvm-ranlib-10" "llvm-ranlib-9" "llvm-ranlib-8") + find_program (LLVM_RANLIB_PATH NAMES "llvm-ranlib" "llvm-ranlib-13" "llvm-ranlib-12" "llvm-ranlib-11" "llvm-ranlib-10" "llvm-ranlib-9" "llvm-ranlib-8") if (LLVM_RANLIB_PATH) message(STATUS "Using llvm-ranlib: ${LLVM_RANLIB_PATH}.") @@ -629,9 +631,6 @@ include_directories(${ConfigIncludePath}) # Add as many warnings as possible for our own code. include (cmake/warnings.cmake) -# Check if needed compiler flags are supported -include (cmake/check_flags.cmake) - add_subdirectory (base) add_subdirectory (src) add_subdirectory (programs) diff --git a/base/common/CachedFn.h b/base/common/CachedFn.h index 6ecae0b88b3..7c3af0443d8 100644 --- a/base/common/CachedFn.h +++ b/base/common/CachedFn.h @@ -5,13 +5,12 @@ #include "FnTraits.h" /** - * Cache for a functor that decays to a pointer-to-function. - * The size is unlimited. Values are stored permanently and never evicted. - * Suitable only for simplest cases. - * + * Caching proxy for a functor that decays to a pointer-to-function. + * Saves pairs (func args, func result on args). + * Cache size is unlimited. Cache items are evicted only on manual drop. * Invocation/update is O(log(saved cache values)). * - * @example CachedFn<&my_func> cached; cached(arg); + * See Common/tests/cached_fn.cpp for examples. */ template struct CachedFn diff --git a/src/Common/examples/CMakeLists.txt b/src/Common/examples/CMakeLists.txt index 64d28fec5c2..e72681621cb 100644 --- a/src/Common/examples/CMakeLists.txt +++ b/src/Common/examples/CMakeLists.txt @@ -19,9 +19,6 @@ target_link_libraries (parallel_aggregation2 PRIVATE dbms) add_executable (int_hashes_perf int_hashes_perf.cpp) target_link_libraries (int_hashes_perf PRIVATE clickhouse_common_io) -add_executable (simple_cache simple_cache.cpp) -target_link_libraries (simple_cache PRIVATE common) - add_executable (compact_array compact_array.cpp) target_link_libraries (compact_array PRIVATE clickhouse_common_io) diff --git a/src/Common/examples/simple_cache.cpp b/src/Common/examples/simple_cache.cpp deleted file mode 100644 index b6bcfc0b932..00000000000 --- a/src/Common/examples/simple_cache.cpp +++ /dev/null @@ -1,22 +0,0 @@ -#include -#include - - -static int func(int x, int y) -{ - std::cerr << x << " + " << y << "\n"; - return x + y; -} - - -int main(int, char **) -{ - CachedFn<&func> func_cached; - - std::cerr << func_cached(1, 2) << "\n"; - std::cerr << func_cached(1, 2) << "\n"; - std::cerr << func_cached(1, 2) << "\n"; - std::cerr << func_cached(3, 4) << "\n"; - std::cerr << func_cached(3, 4) << "\n"; - std::cerr << func_cached(3, 4) << "\n"; -} diff --git a/src/Common/tests/gtest_cached_fn.cpp b/src/Common/tests/gtest_cached_fn.cpp new file mode 100644 index 00000000000..51e77018038 --- /dev/null +++ b/src/Common/tests/gtest_cached_fn.cpp @@ -0,0 +1,50 @@ +#include +#include +#include +#include + +using namespace std::chrono_literals; + +constexpr int add(int x, int y) +{ + return x + y; +} + +static int longFunction(int x, int y) +{ + std::this_thread::sleep_for(1s); + return x + y; +} + +TEST(CachedFn, Basic) +{ + CachedFn<&add> fn; + + const int res = fn(1, 2); + EXPECT_EQ(fn(1, 2), res); + + auto f = [](int x, int y) { return x - y; }; + CachedFn<+f> fn2; + + const int res2 = fn2(1, 2); + EXPECT_EQ(fn2(1, 2), res2); +} + +TEST(CachedFn, CachingResults) +{ + CachedFn<&longFunction> fn; + + for (int x = 0; x < 2; ++x) + { + for (int y = 0; y < 2; ++y) + { + const int res = fn(x, y); + const time_t start = time(nullptr); + + for (int count = 0; count < 1000; ++count) + EXPECT_EQ(fn(x, y), res); + + EXPECT_LT(time(nullptr) - start, 10); + } + } +} From e8625e85a466c874247e67dc539f027575404e8c Mon Sep 17 00:00:00 2001 From: Mike Kot Date: Fri, 1 Oct 2021 19:13:14 +0200 Subject: [PATCH 3/7] Fixing integration tests --- base/common/CachedFn.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/base/common/CachedFn.h b/base/common/CachedFn.h index 7c3af0443d8..83e02073e1d 100644 --- a/base/common/CachedFn.h +++ b/base/common/CachedFn.h @@ -36,7 +36,6 @@ public: return it->second; } - /// The calculations themselves are not done under mutex. Result res = std::apply(Func, key); { @@ -55,7 +54,8 @@ public: { std::lock_guard lock(mutex); - cache.emplace(std::move(key), std::move(res)); + // TODO Can't use emplace(std::move(key), ..), causes test_host_ip_change errors. + cache[key] = std::move(res); } } From 5d38930a3c54d54485087fa66542870c43cd6a01 Mon Sep 17 00:00:00 2001 From: Mike Kot Date: Sat, 2 Oct 2021 14:27:52 +0200 Subject: [PATCH 4/7] Fixing gcc build due to a bug in compiler --- src/Common/tests/gtest_cached_fn.cpp | 8 ++++++-- src/Compression/CompressionCodecEncrypted.cpp | 2 +- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/Common/tests/gtest_cached_fn.cpp b/src/Common/tests/gtest_cached_fn.cpp index 51e77018038..c490e635f0e 100644 --- a/src/Common/tests/gtest_cached_fn.cpp +++ b/src/Common/tests/gtest_cached_fn.cpp @@ -10,12 +10,14 @@ constexpr int add(int x, int y) return x + y; } -static int longFunction(int x, int y) +int longFunction(int x, int y) { std::this_thread::sleep_for(1s); return x + y; } +auto f = [](int x, int y) { return x - y; }; + TEST(CachedFn, Basic) { CachedFn<&add> fn; @@ -23,7 +25,9 @@ TEST(CachedFn, Basic) const int res = fn(1, 2); EXPECT_EQ(fn(1, 2), res); - auto f = [](int x, int y) { return x - y; }; + /// In GCC, lambda can't be placed in TEST, producing " has no linkage". + /// Assuming http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2014/n4268.html, + /// this is a GCC bug. CachedFn<+f> fn2; const int res2 = fn2(1, 2); diff --git a/src/Compression/CompressionCodecEncrypted.cpp b/src/Compression/CompressionCodecEncrypted.cpp index d35411d6266..8f2a33f282f 100644 --- a/src/Compression/CompressionCodecEncrypted.cpp +++ b/src/Compression/CompressionCodecEncrypted.cpp @@ -123,7 +123,7 @@ UInt64 methodKeySize(EncryptionMethod Method) std::string lastErrorString() { - std::array buffer; + std::array buffer = {}; ERR_error_string_n(ERR_get_error(), buffer.data(), buffer.size()); return std::string(buffer.data()); } From 38232c286d9e9c4a1a10191642a39a2665d0b635 Mon Sep 17 00:00:00 2001 From: Mike Kot Date: Sat, 2 Oct 2021 21:45:05 +0200 Subject: [PATCH 5/7] fixing paths --- base/{common => base}/CachedFn.h | 0 base/{common => base}/FnTraits.h | 0 src/Common/DNSResolver.cpp | 2 +- src/Common/StackTrace.cpp | 2 +- src/Disks/HDFS/DiskHDFS.cpp | 2 +- src/Disks/S3/DiskS3.cpp | 2 +- 6 files changed, 4 insertions(+), 4 deletions(-) rename base/{common => base}/CachedFn.h (100%) rename base/{common => base}/FnTraits.h (100%) diff --git a/base/common/CachedFn.h b/base/base/CachedFn.h similarity index 100% rename from base/common/CachedFn.h rename to base/base/CachedFn.h diff --git a/base/common/FnTraits.h b/base/base/FnTraits.h similarity index 100% rename from base/common/FnTraits.h rename to base/base/FnTraits.h diff --git a/src/Common/DNSResolver.cpp b/src/Common/DNSResolver.cpp index 6a5f7b872fe..d69291b79c0 100644 --- a/src/Common/DNSResolver.cpp +++ b/src/Common/DNSResolver.cpp @@ -1,5 +1,5 @@ #include "DNSResolver.h" -#include +#include #include #include #include diff --git a/src/Common/StackTrace.cpp b/src/Common/StackTrace.cpp index 22d1b17b8ea..1406cf03fe4 100644 --- a/src/Common/StackTrace.cpp +++ b/src/Common/StackTrace.cpp @@ -5,7 +5,7 @@ #include #include #include -#include +#include #include #include diff --git a/src/Disks/HDFS/DiskHDFS.cpp b/src/Disks/HDFS/DiskHDFS.cpp index 94c2a23bc1a..7b37c2ae8fa 100644 --- a/src/Disks/HDFS/DiskHDFS.cpp +++ b/src/Disks/HDFS/DiskHDFS.cpp @@ -6,7 +6,7 @@ #include #include #include -#include +#include namespace DB diff --git a/src/Disks/S3/DiskS3.cpp b/src/Disks/S3/DiskS3.cpp index 77af47aaec1..be07f036d47 100644 --- a/src/Disks/S3/DiskS3.cpp +++ b/src/Disks/S3/DiskS3.cpp @@ -10,7 +10,7 @@ #include #include -#include +#include #include #include From 57e2744264a31a1ae7d189cc2b1e28f857a935d7 Mon Sep 17 00:00:00 2001 From: Mike Kot Date: Sat, 2 Oct 2021 21:47:35 +0200 Subject: [PATCH 6/7] Fixing other imports --- src/Access/IAccessStorage.cpp | 2 +- src/Access/RoleCache.cpp | 2 +- src/Access/UsersConfigAccessStorage.cpp | 2 +- src/Databases/MySQL/MaterializeMetadata.cpp | 2 +- src/Databases/PostgreSQL/fetchPostgreSQLTableStructure.cpp | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Access/IAccessStorage.cpp b/src/Access/IAccessStorage.cpp index ce05cb7da0f..aade1fdd6f1 100644 --- a/src/Access/IAccessStorage.cpp +++ b/src/Access/IAccessStorage.cpp @@ -6,7 +6,7 @@ #include #include #include -#include +#include namespace DB diff --git a/src/Access/RoleCache.cpp b/src/Access/RoleCache.cpp index db0d5863268..e9c88868e8c 100644 --- a/src/Access/RoleCache.cpp +++ b/src/Access/RoleCache.cpp @@ -3,7 +3,7 @@ #include #include #include -#include +#include namespace DB diff --git a/src/Access/UsersConfigAccessStorage.cpp b/src/Access/UsersConfigAccessStorage.cpp index 559aa2198e6..7c5baa92b27 100644 --- a/src/Access/UsersConfigAccessStorage.cpp +++ b/src/Access/UsersConfigAccessStorage.cpp @@ -18,7 +18,7 @@ #include #include #include -#include +#include namespace DB diff --git a/src/Databases/MySQL/MaterializeMetadata.cpp b/src/Databases/MySQL/MaterializeMetadata.cpp index efcbf8939b5..514978f2456 100644 --- a/src/Databases/MySQL/MaterializeMetadata.cpp +++ b/src/Databases/MySQL/MaterializeMetadata.cpp @@ -15,7 +15,7 @@ #include #include #include -#include +#include namespace fs = std::filesystem; diff --git a/src/Databases/PostgreSQL/fetchPostgreSQLTableStructure.cpp b/src/Databases/PostgreSQL/fetchPostgreSQLTableStructure.cpp index 731e8f639d8..16ba4196dd2 100644 --- a/src/Databases/PostgreSQL/fetchPostgreSQLTableStructure.cpp +++ b/src/Databases/PostgreSQL/fetchPostgreSQLTableStructure.cpp @@ -14,7 +14,7 @@ #include #include #include -#include +#include namespace DB From 585e6d53ff58926c52857172a21ecde34ace71f9 Mon Sep 17 00:00:00 2001 From: Mike Kot Date: Sat, 2 Oct 2021 22:55:26 +0200 Subject: [PATCH 7/7] fix --- src/Common/tests/gtest_cached_fn.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Common/tests/gtest_cached_fn.cpp b/src/Common/tests/gtest_cached_fn.cpp index c490e635f0e..ab15a1ee5e1 100644 --- a/src/Common/tests/gtest_cached_fn.cpp +++ b/src/Common/tests/gtest_cached_fn.cpp @@ -1,7 +1,7 @@ #include #include #include -#include +#include using namespace std::chrono_literals;