diff --git a/src/mlpack/core/util/timers.cpp b/src/mlpack/core/util/timers.cpp index 146a5ff14a..aa21e60f8e 100644 --- a/src/mlpack/core/util/timers.cpp +++ b/src/mlpack/core/util/timers.cpp @@ -66,25 +66,34 @@ void Timer::ResetAll() // Reset a Timers object. void Timers::Reset() { + lock_guard lock(timersMutex); timers.clear(); timerStartTime.clear(); - timerState.clear(); } -map& Timers::GetAllTimers() +map Timers::GetAllTimers() { + // Make a copy of the timer. + lock_guard lock(timersMutex); return timers; } microseconds Timers::GetTimer(const string& timerName) { + if (!enabled) + return microseconds(0); + + lock_guard lock(timersMutex); return timers[timerName]; } bool Timers::GetState(const string& timerName, const thread::id& threadId) { - return timerState[threadId][timerName]; + lock_guard lock(timersMutex); + if (timerStartTime.count(threadId) == 0) + return 0; + return (timerStartTime[threadId].count(timerName) > 0); } void Timers::PrintTimer(const string& timerName) @@ -150,27 +159,32 @@ void Timers::PrintTimer(const string& timerName) void Timers::StopAllTimers() { - // Terminate the program timers. - for (auto it : timerState) - { - for (auto it2 : it.second) - { - if (timerState[it.first][it2.first] == 1) - StopTimer(it2.first, it.first); - } - } -} + // Terminate the program timers. Don't use StopTimer() since that modifies + // the map and would invalidate our iterators. + lock_guard lock(timersMutex); -high_resolution_clock::time_point Timers::GetTime() -{ - return high_resolution_clock::now(); + high_resolution_clock::time_point currTime = high_resolution_clock::now(); + for (auto it : timerStartTime) + for (auto it2 : it.second) + timers[it2.first] += duration_cast(currTime - it2.second); + + // If all timers are stopped, we can clear the maps. + timerStartTime.clear(); } void Timers::StartTimer(const string& timerName, const thread::id& threadId) { - timersMutex.lock(); - if ((timerState[threadId][timerName]) && (timerName != "total_time")) + // Don't do anything if we aren't timing. + if (!enabled) + return; + if (timerName == "total_time") + return; // Ignore that timer. + + lock_guard lock(timersMutex); + + if ((timerStartTime.count(threadId) > 0) && + (timerStartTime[threadId].count(timerName))) { ostringstream error; error << "Timer::Start(): timer '" << timerName @@ -178,7 +192,7 @@ void Timers::StartTimer(const string& timerName, throw runtime_error(error.str()); } - high_resolution_clock::time_point currTime = GetTime(); + high_resolution_clock::time_point currTime = high_resolution_clock::now(); // If the timer is added for the first time. if (timers.count(timerName) == 0) @@ -186,36 +200,37 @@ void Timers::StartTimer(const string& timerName, timers[timerName] = (microseconds) 0; } - timerState[threadId][timerName] = true; timerStartTime[threadId][timerName] = currTime; - timersMutex.unlock(); } void Timers::StopTimer(const string& timerName, const thread::id& threadId) { - timersMutex.lock(); - if ((!timerState[threadId][timerName]) && (timerName != "total_time")) + // Don't do anything if we aren't timing. + if (!enabled) + return; + if (timerName == "total_time") + return; // Ignore that timer. + + lock_guard lock(timersMutex); + + if ((timerStartTime.count(threadId) == 0) || + (timerStartTime[threadId].count(timerName) == 0)) { ostringstream error; - error << "Timer::Stop(): timer '" << timerName - << "' has already been stopped"; + error << "Timer::Stop(): no timer with name '" << timerName + << "' currently running"; throw runtime_error(error.str()); } - high_resolution_clock::time_point currTime = GetTime(); + high_resolution_clock::time_point currTime = high_resolution_clock::now(); // Calculate the delta time. - timerState[threadId][timerName] = false; timers[timerName] += duration_cast(currTime - timerStartTime[threadId][timerName]); // Remove the entries. - timerState[threadId].erase(timerName); timerStartTime[threadId].erase(timerName); - if (timerState[threadId].empty()) - timerState.erase(threadId); if (timerStartTime[threadId].empty()) timerStartTime.erase(threadId); - timersMutex.unlock(); } diff --git a/src/mlpack/core/util/timers.hpp b/src/mlpack/core/util/timers.hpp index af2655f208..5fe2f94764 100644 --- a/src/mlpack/core/util/timers.hpp +++ b/src/mlpack/core/util/timers.hpp @@ -19,6 +19,7 @@ #include // std::thread is used for thread safety. #include #include +#include #if defined(_WIN32) // uint64_t isn't defined on every windows. @@ -101,7 +102,7 @@ class Timers /** * Returns a copy of all the timers used via this interface. */ - std::map& GetAllTimers(); + std::map GetAllTimers(); /** * Reset the timers. This stops all running timers and removes them. Whether @@ -161,7 +162,7 @@ class Timers void StopAllTimers(); //! Modify whether or not timing is enabled. - bool& Enabled() { return enabled; } + std::atomic& Enabled() { return enabled; } //! Get whether or not timing is enabled. bool Enabled() const { return enabled; } @@ -170,16 +171,12 @@ class Timers std::map timers; //! A mutex for modifying the timers. std::mutex timersMutex; - //! A map that contains whether or not each timer is currently running. - std::map> timerState; //! A map for the starting values of the timers. std::map> timerStartTime; - std::chrono::high_resolution_clock::time_point GetTime(); - //! Whether or not timing is enabled. - bool enabled; + std::atomic enabled; }; } // namespace mlpack diff --git a/src/mlpack/tests/timer_test.cpp b/src/mlpack/tests/timer_test.cpp index f24c8d7c4e..d50479bb53 100644 --- a/src/mlpack/tests/timer_test.cpp +++ b/src/mlpack/tests/timer_test.cpp @@ -98,7 +98,6 @@ BOOST_AUTO_TEST_CASE(TwiceStartTimerTest) Timer::DisableTiming(); } -#include BOOST_AUTO_TEST_CASE(MultithreadTimerTest) { Timer::EnableTiming(); @@ -132,4 +131,20 @@ BOOST_AUTO_TEST_CASE(MultithreadTimerTest) BOOST_REQUIRE(Timer::Get("thread_timer") > std::chrono::microseconds(50000)); } +BOOST_AUTO_TEST_CASE(DisabledTimingTest) +{ + // It should be disabled by default but let's be paranoid. + Timer::DisableTiming(); + + Timer::Start("test_timer"); + #ifdef _WIN32 + Sleep(20); + #else + usleep(20000); + #endif + Timer::Stop("test_timer"); + + BOOST_REQUIRE(Timer::Get("test_timer") == std::chrono::microseconds(0)); +} + BOOST_AUTO_TEST_SUITE_END();