From 03627c0ed5bed05941cd4898a55c2ca0b739c249 Mon Sep 17 00:00:00 2001 From: Denes Matetelki Date: Sat, 9 Apr 2011 15:37:24 +0200 Subject: [PATCH] TimerThread had too much overhead, trying a different approach with Timer --- CMakeLists.txt | 1 + build/CMakeLists.txt | 4 +- include/Logger.hpp | 43 +++++++------- include/Thread.hpp | 4 +- include/Timer.hpp | 40 +++++++++++++ {src => other}/TimerThread.cpp | 2 + {include => other}/TimerThread.hpp | 0 {test => other}/test_TimerThread.hpp | 55 +++++++++++------ src/Thread.cpp | 19 +----- src/Timer.cpp | 88 ++++++++++++++++++++++++++++ test/CMakeLists.txt | 8 ++- test/run_test.sh | 4 ++ test/test_Timer.hpp | 79 +++++++++++++++++++++++++ test/valgrind.supp | 32 +++++++++- 14 files changed, 317 insertions(+), 62 deletions(-) create mode 100644 include/Timer.hpp rename {src => other}/TimerThread.cpp (99%) rename {include => other}/TimerThread.hpp (100%) rename {test => other}/test_TimerThread.hpp (70%) create mode 100644 src/Timer.cpp create mode 100644 test/test_Timer.hpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 589664e..817b343 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -10,6 +10,7 @@ if(DOXYGEN_FOUND) endif(DOXYGEN_FOUND) + add_custom_target(reset COMMAND rm -rf html diff --git a/build/CMakeLists.txt b/build/CMakeLists.txt index 468109e..5be6579 100644 --- a/build/CMakeLists.txt +++ b/build/CMakeLists.txt @@ -1,8 +1,10 @@ cmake_minimum_required (VERSION 2.6) project (CPP_UTILS_LIB) -set (CXX_FLAGS "-Wall -Wextra -pedantic -Weffc++ -Wshadow -ggdb -fprofile-arcs -ftest-coverage") +set (CXX_FLAGS "-Wall -Wextra -pedantic -Weffc++ -Wshadow " + "-ggdb -fprofile-arcs -ftest-coverage") add_definitions( ${CXX_FLAGS} ) +# add_definitions( -DNO_TRACE ) include_directories (../include) aux_source_directory(../src CPP_UTILS_LIB_SOURCES) diff --git a/include/Logger.hpp b/include/Logger.hpp index ac8c627..2fcb7c4 100644 --- a/include/Logger.hpp +++ b/include/Logger.hpp @@ -57,33 +57,32 @@ private: #ifdef NO_TRACE - #define MAX_LOFLEVEL Logger::DEBUG -#else - #define MAX_LOFLEVEL Logger::FINEST -#endif + #define TRACE (void)0 + #define TRACE_STATIC (void)0 + #define LOG (void)0 -#define TRACE \ -if(MAX_LOFLEVEL >= Logger::FINEST && \ - Logger::getInstance()->getLoglevel() >= Logger::FINEST ) \ -Logger::getInstance()->log_pointer( \ - this, __FILE__, __LINE__, __PRETTY_FUNCTION__); \ -else (void)0 +#else -#define TRACE_STATIC \ -if(MAX_LOFLEVEL >= Logger::FINEST && \ - Logger::getInstance()->getLoglevel() >= Logger::FINEST ) \ -Logger::getInstance()->log_pointer( \ - 0, __FILE__, __LINE__, __PRETTY_FUNCTION__); \ -else (void)0 + #define TRACE \ + if ( Logger::getInstance()->getLoglevel() >= Logger::FINEST ) \ + Logger::getInstance()->log_pointer( \ + this, __FILE__, __LINE__, __PRETTY_FUNCTION__); \ + else (void)0 + #define TRACE_STATIC \ + if ( Logger::getInstance()->getLoglevel() >= Logger::FINEST ) \ + Logger::getInstance()->log_pointer( \ + 0, __FILE__, __LINE__, __PRETTY_FUNCTION__); \ + else (void)0 -#define LOG(level, msg) \ -if (MAX_LOFLEVEL >= level && \ - Logger::getInstance()->getLoglevel() >= Logger::FINEST ) \ -Logger::getInstance()->log_string( \ - msg, __FILE__, __LINE__, __PRETTY_FUNCTION__); \ -else (void)0 + #define LOG(level, msg) \ + if ( Logger::getInstance()->getLoglevel() >= Logger::FINEST ) \ + Logger::getInstance()->log_string( \ + msg, __FILE__, __LINE__, __PRETTY_FUNCTION__); \ + else (void)0 + +#endif /// @todo remove this diff --git a/include/Thread.hpp b/include/Thread.hpp index f322f59..69d70d7 100644 --- a/include/Thread.hpp +++ b/include/Thread.hpp @@ -8,7 +8,7 @@ class Thread { public: - Thread( const bool softRealTime = false ); + Thread(); virtual ~Thread(); void start(); @@ -28,7 +28,7 @@ protected: private: pthread_t m_threadHandler; - bool m_softRealTime; + }; diff --git a/include/Timer.hpp b/include/Timer.hpp new file mode 100644 index 0000000..68cf062 --- /dev/null +++ b/include/Timer.hpp @@ -0,0 +1,40 @@ +#ifndef TIMER_HPP +#define TIMER_HPP + +#include // sigset_t +#include // timer_t + +class Timer +{ + +public: + + Timer(const int signal = SIGALRM ); + + + virtual void timerExpired() {} + + virtual void periodicTimerExpired() {} + + + void createTimer(const time_t interval_sec, + const long interval_nsec = 0, + const time_t initExpr_sec = 0, + const long initExpr_nsec = 0); + + void wait(); + + void stopTimer(); + +private: + + int m_signal; + struct sigaction m_sigAction; + timer_t m_timerId; + bool m_periodic; + bool m_running; + +}; // class Timer + + +#endif // TIMER_HPP diff --git a/src/TimerThread.cpp b/other/TimerThread.cpp similarity index 99% rename from src/TimerThread.cpp rename to other/TimerThread.cpp index 82cd71f..87a9cbf 100644 --- a/src/TimerThread.cpp +++ b/other/TimerThread.cpp @@ -84,6 +84,8 @@ bool TimerThread::removeTimerUser ( void* timerUser ) m_users.erase(tmp); m_condVar.signal(); found = true; // one user can be registered multiple times + } else { + it++; } } return found; diff --git a/include/TimerThread.hpp b/other/TimerThread.hpp similarity index 100% rename from include/TimerThread.hpp rename to other/TimerThread.hpp diff --git a/test/test_TimerThread.hpp b/other/test_TimerThread.hpp similarity index 70% rename from test/test_TimerThread.hpp rename to other/test_TimerThread.hpp index 59932ef..899536b 100644 --- a/test/test_TimerThread.hpp +++ b/other/test_TimerThread.hpp @@ -34,7 +34,7 @@ private: public: - void testBasic( void ) + void nemtestBasic( void ) { TEST_HEADER; TimerThread* tt = new TimerThread(); @@ -54,7 +54,7 @@ public: delete user; } - void testBasicTimeSpec( void ) + void nemtestBasicTimeSpec( void ) { TEST_HEADER; TimerThread* tt = new TimerThread(); @@ -75,7 +75,7 @@ public: delete user; } - void testPeriodic( void ) + void nemtestPeriodic( void ) { TEST_HEADER; TimerThread* tt = new TimerThread(); @@ -95,35 +95,39 @@ public: delete user; } - void testPeriodicTimeSpec( void ) + void testPeriodicTimeSpec( void ) { TEST_HEADER; + +// Logger::getInstance()->setLogLevel(Logger::DEBUG); + TimerThread* tt = new TimerThread(); tt->start(); sleep(1); DummyTimerUser *user = new DummyTimerUser(); - timespec ts = { 2, 3 }; - timespec tsperiod = { 1, 2 }; + timespec ts = { 2, 0 }; + timespec tsperiod = { 1, 0 }; tt->addTimerUser( user, ts, tsperiod ); /// @bug What is wrong here? -// DummyTimerUser *user2 = new DummyTimerUser(); -// timespec ts2 = { 0, 100000000 }; -// tt->addTimerUser( user2, ts, ts2 ); + DummyTimerUser *user2 = new DummyTimerUser(); + int perMinute = 2000; + timespec ts2 = { 0, 1000000000 / perMinute }; + tt->addTimerUser( user2, ts, ts2 ); sleep(6); tt->stop(); sleep(1); - TS_ASSERT_EQUALS( user->m_counter, 104 ); -// TS_ASSERT_EQUALS( user2->m_counter, 110 ); + TS_ASSERT_EQUALS( user->m_counter, 100 + 4 ); // 4 times + TS_ASSERT_EQUALS( user2->m_counter, 100 + perMinute*4 ); delete tt; delete user; } - void testDestroyed( void ) + void nemtestDestroyed( void ) { TEST_HEADER; TimerThread* tt = new TimerThread(); @@ -143,7 +147,7 @@ public: delete user; } - void testRemoved( void ) + void nemtestRemoved( void ) { TEST_HEADER; TimerThread* tt = new TimerThread(); @@ -151,25 +155,42 @@ public: sleep(1); DummyTimerUser *user = new DummyTimerUser(); + DummyTimerUser *user2 = new DummyTimerUser(); + DummyTimerUser *user3 = new DummyTimerUser(); tt->addTimerUser( user, 10 ); + tt->addTimerUser( user2, 13 ); + tt->addTimerUser( user3, 15 ); sleep(2); - TS_ASSERT_EQUALS( tt->m_users.size(), 1); + TS_ASSERT_EQUALS( tt->m_users.size(), 3 ); - tt->removeTimerUser( user ); + tt->removeTimerUser( user2 ); + sleep(1); + TS_ASSERT_EQUALS( tt->m_users.size(), 2 ); + tt->removeTimerUser( user3 ); sleep(1); - TS_ASSERT_EQUALS( tt->m_users.size(), 0); + TS_ASSERT_EQUALS( tt->m_users.size(), 1 ); + + tt->removeTimerUser( user2 ); + sleep(1); + TS_ASSERT_EQUALS( tt->m_users.size(), 1 ); + + tt->removeTimerUser( user ); + sleep(1); + TS_ASSERT_EQUALS( tt->m_users.size(), 0 ); tt->stop(); sleep(1); delete tt; delete user; + delete user2; + delete user3; } - void testRemovedMultiple( void ) + void nemtestRemovedMultiple( void ) { TEST_HEADER; TimerThread* tt = new TimerThread(); diff --git a/src/Thread.cpp b/src/Thread.cpp index 8ec624b..ceeb773 100644 --- a/src/Thread.cpp +++ b/src/Thread.cpp @@ -8,10 +8,9 @@ #include // sched_param -Thread::Thread(const bool softRealTime) +Thread::Thread() : m_isRunning(false) , m_threadHandler( 0 ) - , m_softRealTime(softRealTime) { TRACE; } @@ -27,21 +26,7 @@ void Thread::start() { TRACE; m_isRunning = true; - - if ( m_softRealTime ) { - pthread_attr_t attr; - sched_param param; - - pthread_attr_init(&attr); - pthread_attr_setschedpolicy( &attr, SCHED_RR ); - - param.sched_priority = 50; - pthread_attr_setschedparam( &attr, ¶m ); - - pthread_create( &m_threadHandler, &attr, threadStarter, ( void* )this ); - } else { - pthread_create( &m_threadHandler, 0, threadStarter, ( void* )this ); - } + pthread_create( &m_threadHandler, 0, threadStarter, ( void* )this ); } diff --git a/src/Timer.cpp b/src/Timer.cpp new file mode 100644 index 0000000..15d1b1e --- /dev/null +++ b/src/Timer.cpp @@ -0,0 +1,88 @@ +#include "Timer.hpp" + +#include "Common.hpp" + +#include // sigset_t +// #include // assert +#include // timer_t +// #include // EINVAL, EAGAIN, EINTR +// #include +#include // strerror +// #include +// #include + +Timer::Timer(const int signal) + : m_signal(signal) + , m_periodic(false) + , m_running(true) +{ + TRACE; + + m_sigAction.sa_flags = SA_SIGINFO; + sigemptyset(&m_sigAction.sa_mask); + sigaddset( &m_sigAction.sa_mask, m_signal ); + sigaction( m_signal, &m_sigAction, 0 ); +} + + +void Timer::createTimer(const time_t interval_sec, + const long interval_nsec, + const time_t initExpr_sec, + const long initExpr_nsec) +{ + TRACE; + + // create timer + struct sigevent sigev; + sigev.sigev_notify = SIGEV_SIGNAL; + sigev.sigev_signo = m_signal; + sigev.sigev_value.sival_ptr = &m_timerId; + if ( timer_create( CLOCK_MONOTONIC, &sigev, &m_timerId ) == -1 ) { +// std::cout << "Error from timer_create: " << strerror(errno) << std::endl; + LOG ( Logger::FINEST, "Error from timer_create: " /*strerror(errno)*/ ); + return; + } + + // arm it + struct itimerspec its; + its.it_value.tv_sec = interval_sec; + its.it_value.tv_nsec = interval_nsec; + its.it_interval.tv_sec = initExpr_sec; + its.it_interval.tv_nsec = initExpr_nsec; + + if ( initExpr_sec != 0 or initExpr_nsec != 0 ) m_periodic = true; + + if ( timer_settime( m_timerId, 0, &its, 0 ) == -1 ) { +// std::cout << "Error from timer_settime: " << strerror(errno) << std::endl; + LOG ( Logger::FINEST, "Error from timer_settime: " /*strerror(errno)*/ ); + return; + } +} + + +void Timer::wait() +{ + TRACE; + + int sig; + sigwait( &m_sigAction.sa_mask, &sig ); + timerExpired(); + if ( m_periodic ) { + while ( m_running ) { + sigwait( &m_sigAction.sa_mask, &sig ); + periodicTimerExpired(); + } + } +} + + +void Timer::stopTimer() +{ + TRACE; + + struct itimerspec its; + its.it_value.tv_sec = 0; + its.it_value.tv_nsec = 0; + timer_settime( m_timerId, 0, &its, 0 ); + m_running = false; +} diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index e9a96d1..b20f580 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -1,5 +1,9 @@ -set (CXX_FLAGS "-Wall -Wextra -pedantic -Weffc++ -Wshadow -ggdb -fprofile-arcs -ftest-coverage") +set (CXX_FLAGS "-Wall -Wextra -pedantic -Weffc++ -Wshadow " + "-ggdb -fprofile-arcs -ftest-coverage " ) + add_definitions( ${CXX_FLAGS} ) +# add_definitions( -DNO_TRACE ) + find_package(CxxTest) if(CXXTEST_FOUND) @@ -19,7 +23,7 @@ if(CXXTEST_FOUND) test_Thread.hpp test_ThreadPool.hpp test_Semaphore.hpp - test_TimerThread.hpp + test_Timer.hpp test_Common.hpp ) target_link_libraries(test CppUtils gcov) diff --git a/test/run_test.sh b/test/run_test.sh index 47bde77..38fc693 100755 --- a/test/run_test.sh +++ b/test/run_test.sh @@ -61,6 +61,10 @@ valgrind \ --suppressions=valgrind.supp \ $test | tee $test.out; retval=$PIPESTATUS +# NOTE to gen suppressions run: +# valgrind --leak-check=full --show-reachable=yes --show-below-main=no --track-origins=yes --num-callers=30 --malloc-fill=0xaa --free-fill=0xdd --gen-suppressions=yes ./test + + # retval is 0 on success # or the number of failed cases # or 137 if segfault happens diff --git a/test/test_Timer.hpp b/test/test_Timer.hpp new file mode 100644 index 0000000..ce19782 --- /dev/null +++ b/test/test_Timer.hpp @@ -0,0 +1,79 @@ +#include + +#include "Fixture.hpp" + +// #define private public // reach TimerThread's private multimap + +#include "Timer.hpp" + +// #include + + + + +class TestTimer : public CxxTest::TestSuite +{ + +private: + + class DummyTimer : public Timer + { + public: + + DummyTimer(int maxPeriodicCount = 5) + : m_counter(0) + , m_maxPeriodicCount(maxPeriodicCount) + { + TRACE; + } + + void timerExpired() + { + TRACE; + m_counter += 100; + } + + void periodicTimerExpired() + { + TRACE; + static int count = 0; + m_counter++; + count++; + if ( count >= m_maxPeriodicCount ) { + stopTimer(); + } + } + + int m_counter; + + private: + + int m_maxPeriodicCount; + + }; + + +public: + + void testBasic( void ) + { + TEST_HEADER; + + DummyTimer t; + t.createTimer(2); + t.wait(); + TS_ASSERT_EQUALS( t.m_counter, 100 ); + } + + void testBasicPeriodic( void ) + { + TEST_HEADER; + + DummyTimer t; + t.createTimer(2,0,1); + t.wait(); + TS_ASSERT_EQUALS( t.m_counter, 105 ); + } + + +}; diff --git a/test/valgrind.supp b/test/valgrind.supp index dbcfa5a..26c2f66 100644 --- a/test/valgrind.supp +++ b/test/valgrind.supp @@ -76,6 +76,36 @@ fun:calloc fun:allocate_dtv fun:_dl_allocate_tls - fun:pthread_create@@GLIBC_2.1 + fun:pthread_create@@GLIBC_*.*.* fun:_ZN6Thread5startEv + ... + fun:_ZN7CxxTest19RealTestDescription3runEv + fun:_ZN7CxxTest10TestRunner7runTestERNS_15TestDescriptionE + fun:_ZN7CxxTest10TestRunner8runSuiteERNS_16SuiteDescriptionE + fun:_ZN7CxxTest10TestRunner8runWorldEv + fun:_ZN7CxxTest10TestRunner11runAllTestsERNS_12TestListenerE + fun:_ZN7CxxTest14ErrorFormatter3runEv + fun:main } + +{ + sigaction handling + Memcheck:Param + rt_sigaction(act->sa_handler) + fun:__libc_sigaction +} + +{ + timer create + Memcheck:Param + timer_create(evp) + fun:timer_create@@GLIBC_2.3.3 +} + +{ + timer create 2 + Memcheck:Leak + fun:malloc + fun:timer_create@@GLIBC_2.3.3 +} +