diff --git a/test_util/sync_point.cc b/test_util/sync_point.cc index 4c71fc6bc..067fc8234 100644 --- a/test_util/sync_point.cc +++ b/test_util/sync_point.cc @@ -60,7 +60,7 @@ void SyncPoint::ClearTrace() { impl_->ClearTrace(); } -void SyncPoint::Process(const std::string& point, void* cb_arg) { +void SyncPoint::Process(const Slice& point, void* cb_arg) { impl_->Process(point, cb_arg); } diff --git a/test_util/sync_point.h b/test_util/sync_point.h index 775fd5c36..410176dc8 100644 --- a/test_util/sync_point.h +++ b/test_util/sync_point.h @@ -5,6 +5,7 @@ #pragma once #include + #include #include #include @@ -12,6 +13,7 @@ #include #include "rocksdb/rocksdb_namespace.h" +#include "rocksdb/slice.h" #ifdef NDEBUG // empty in release build @@ -117,7 +119,16 @@ class SyncPoint { // triggered by TEST_SYNC_POINT, blocking execution until all predecessors // are executed. // And/or call registered callback function, with argument `cb_arg` - void Process(const std::string& point, void* cb_arg = nullptr); + void Process(const Slice& point, void* cb_arg = nullptr); + + // template gets length of const string at compile time, + // avoiding strlen() at runtime + template + void Process(const char (&point)[kLen], void* cb_arg = nullptr) { + static_assert(kLen > 0, "Must not be empty"); + assert(point[kLen - 1] == '\0'); + Process(Slice(point, kLen - 1), cb_arg); + } // TODO: it might be useful to provide a function that blocks until all // sync points are cleared. diff --git a/test_util/sync_point_impl.cc b/test_util/sync_point_impl.cc index 1d87a05fe..4add49889 100644 --- a/test_util/sync_point_impl.cc +++ b/test_util/sync_point_impl.cc @@ -101,19 +101,23 @@ void SyncPoint::Data::ClearAllCallBacks() { callbacks_.clear(); } -void SyncPoint::Data::Process(const std::string& point, void* cb_arg) { +void SyncPoint::Data::Process(const Slice& point, void* cb_arg) { if (!enabled_) { return; } + // Use a filter to prevent mutex lock if possible. if (!point_filter_.MayContain(point)) { return; } + // Must convert to std::string for remaining work. Take + // heap hit. + std::string point_string(point.ToString()); std::unique_lock lock(mutex_); auto thread_id = std::this_thread::get_id(); - auto marker_iter = markers_.find(point); + auto marker_iter = markers_.find(point_string); if (marker_iter != markers_.end()) { for (auto& marked_point : marker_iter->second) { marked_thread_id_.emplace(marked_point, thread_id); @@ -121,18 +125,18 @@ void SyncPoint::Data::Process(const std::string& point, void* cb_arg) { } } - if (DisabledByMarker(point, thread_id)) { + if (DisabledByMarker(point_string, thread_id)) { return; } - while (!PredecessorsAllCleared(point)) { + while (!PredecessorsAllCleared(point_string)) { cv_.wait(lock); - if (DisabledByMarker(point, thread_id)) { + if (DisabledByMarker(point_string, thread_id)) { return; } } - auto callback_pair = callbacks_.find(point); + auto callback_pair = callbacks_.find(point_string); if (callback_pair != callbacks_.end()) { num_callbacks_running_++; mutex_.unlock(); @@ -140,7 +144,7 @@ void SyncPoint::Data::Process(const std::string& point, void* cb_arg) { mutex_.lock(); num_callbacks_running_--; } - cleared_points_.insert(point); + cleared_points_.insert(point_string); cv_.notify_all(); } } // namespace ROCKSDB_NAMESPACE diff --git a/test_util/sync_point_impl.h b/test_util/sync_point_impl.h index ba818e381..526018040 100644 --- a/test_util/sync_point_impl.h +++ b/test_util/sync_point_impl.h @@ -95,7 +95,7 @@ struct SyncPoint::Data { return marked_point_iter != marked_thread_id_.end() && thread_id != marked_point_iter->second; } - void Process(const std::string& point, void* cb_arg); + void Process(const Slice& point, void* cb_arg); }; } // namespace ROCKSDB_NAMESPACE #endif // NDEBUG