From ac1734d06b953f7cf55f7297c699e1720188383e Mon Sep 17 00:00:00 2001 From: Peter Dillinger Date: Wed, 23 Sep 2020 11:32:44 -0700 Subject: [PATCH] Fix/minimize mock_time_env.h dependencies (#7426) Summary: (a) own copy of kMicrosInSecond (b) out-of-line sync point code Pull Request resolved: https://github.com/facebook/rocksdb/pull/7426 Test Plan: FB internal Reviewed By: ajkr Differential Revision: D23861363 Pulled By: pdillinger fbshipit-source-id: de6b1621dca2f7391c5ff72bad04a7613dc27527 --- CMakeLists.txt | 1 + TARGETS | 1 + src.mk | 1 + test_util/mock_time_env.cc | 38 ++++++++++++++++++++++++++++++++++++++ test_util/mock_time_env.h | 24 ++++-------------------- 5 files changed, 45 insertions(+), 20 deletions(-) create mode 100644 test_util/mock_time_env.cc diff --git a/CMakeLists.txt b/CMakeLists.txt index 91625bd30..52e47e453 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1019,6 +1019,7 @@ option(WITH_ALL_TESTS "Build all test, rather than a small subset" ON) if(WITH_TESTS OR WITH_BENCHMARK_TOOLS) add_subdirectory(third-party/gtest-1.8.1/fused-src/gtest) add_library(testharness STATIC + test_util/mock_time_env.cc test_util/testharness.cc) target_link_libraries(testharness gtest) endif() diff --git a/TARGETS b/TARGETS index 3e2f556f7..ac811dc1b 100644 --- a/TARGETS +++ b/TARGETS @@ -396,6 +396,7 @@ cpp_library( srcs = [ "db/db_test_util.cc", "table/mock_table.cc", + "test_util/mock_time_env.cc", "test_util/testharness.cc", "test_util/testutil.cc", "tools/block_cache_analyzer/block_cache_trace_analyzer.cc", diff --git a/src.mk b/src.mk index 8da6f5efc..69dd75748 100644 --- a/src.mk +++ b/src.mk @@ -305,6 +305,7 @@ STRESS_LIB_SOURCES = \ TEST_LIB_SOURCES = \ db/db_test_util.cc \ + test_util/mock_time_env.cc \ test_util/testharness.cc \ test_util/testutil.cc \ utilities/cassandra/test_utils.cc \ diff --git a/test_util/mock_time_env.cc b/test_util/mock_time_env.cc new file mode 100644 index 000000000..8316406ec --- /dev/null +++ b/test_util/mock_time_env.cc @@ -0,0 +1,38 @@ +// Copyright (c) 2011-present, Facebook, Inc. All rights reserved. +// This source code is licensed under both the GPLv2 (found in the +// COPYING file in the root directory) and Apache 2.0 License +// (found in the LICENSE.Apache file in the root directory). + +#include "test_util/mock_time_env.h" + +#include "test_util/sync_point.h" + +namespace ROCKSDB_NAMESPACE { + +// TODO: this is a workaround for the different behavior on different platform +// for timedwait timeout. Ideally timedwait API should be moved to env. +// details: PR #7101. +void MockTimeEnv::InstallTimedWaitFixCallback() { +#ifndef NDEBUG + SyncPoint::GetInstance()->DisableProcessing(); + SyncPoint::GetInstance()->ClearAllCallBacks(); +#ifdef OS_MACOSX + // This is an alternate way (vs. SpecialEnv) of dealing with the fact + // that on some platforms, pthread_cond_timedwait does not appear to + // release the lock for other threads to operate if the deadline time + // is already passed. (TimedWait calls are currently a bad abstraction + // because the deadline parameter is usually computed from Env time, + // but is interpreted in real clock time.) + SyncPoint::GetInstance()->SetCallBack( + "InstrumentedCondVar::TimedWaitInternal", [&](void* arg) { + uint64_t time_us = *reinterpret_cast(arg); + if (time_us < this->RealNowMicros()) { + *reinterpret_cast(arg) = this->RealNowMicros() + 1000; + } + }); +#endif // OS_MACOSX + SyncPoint::GetInstance()->EnableProcessing(); +#endif // !NDEBUG +} + +} // namespace ROCKSDB_NAMESPACE diff --git a/test_util/mock_time_env.h b/test_util/mock_time_env.h index 4316007f3..1f454144a 100644 --- a/test_util/mock_time_env.h +++ b/test_util/mock_time_env.h @@ -5,6 +5,8 @@ #pragma once +#include + #include "rocksdb/env.h" namespace ROCKSDB_NAMESPACE { @@ -62,29 +64,11 @@ class MockTimeEnv : public EnvWrapper { // TODO: this is a workaround for the different behavior on different platform // for timedwait timeout. Ideally timedwait API should be moved to env. // details: PR #7101. - void InstallTimedWaitFixCallback() { - SyncPoint::GetInstance()->DisableProcessing(); - SyncPoint::GetInstance()->ClearAllCallBacks(); -#if defined(OS_MACOSX) && !defined(NDEBUG) - // This is an alternate way (vs. SpecialEnv) of dealing with the fact - // that on some platforms, pthread_cond_timedwait does not appear to - // release the lock for other threads to operate if the deadline time - // is already passed. (TimedWait calls are currently a bad abstraction - // because the deadline parameter is usually computed from Env time, - // but is interpreted in real clock time.) - SyncPoint::GetInstance()->SetCallBack( - "InstrumentedCondVar::TimedWaitInternal", [&](void* arg) { - uint64_t time_us = *reinterpret_cast(arg); - if (time_us < this->RealNowMicros()) { - *reinterpret_cast(arg) = this->RealNowMicros() + 1000; - } - }); -#endif // OS_MACOSX && !NDEBUG - SyncPoint::GetInstance()->EnableProcessing(); - } + void InstallTimedWaitFixCallback(); private: std::atomic current_time_us_{0}; + static constexpr uint64_t kMicrosInSecond = 1000U * 1000U; }; } // namespace ROCKSDB_NAMESPACE