summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorcleeland <cleeland@ae88bc3d-4319-0410-8dbf-d08b4c9d3795>2010-02-25 22:10:39 +0000
committercleeland <cleeland@ae88bc3d-4319-0410-8dbf-d08b4c9d3795>2010-02-25 22:10:39 +0000
commit5975f972c39f9f32a4adf184d00a7a85d820f8df (patch)
tree874756808d2d189440bb18af43a9bd9987cc194d
parent52010d19b71b69212e86691d567d8803f3cb6a29 (diff)
downloadATCD-5975f972c39f9f32a4adf184d00a7a85d820f8df.tar.gz
ChangeLogTag: Thu Feb 25 22:07:25 UTC 2010 Chris Cleeland <cleeland@ociweb.com>\nBug 3104
-rw-r--r--ACE/ChangeLog29
-rw-r--r--ACE/ace/Timer_Hash_T.cpp5
-rw-r--r--ACE/ace/Timer_Queue_T.cpp65
-rw-r--r--ACE/ace/Timer_Queue_T.h4
-rw-r--r--ACE/ace/Timer_Wheel_T.cpp5
-rw-r--r--ACE/tests/Timer_Queue_Test.cpp51
6 files changed, 147 insertions, 12 deletions
diff --git a/ACE/ChangeLog b/ACE/ChangeLog
index 2a6d312fd7c..bf1f26996c7 100644
--- a/ACE/ChangeLog
+++ b/ACE/ChangeLog
@@ -1,3 +1,32 @@
+Thu Feb 25 22:07:25 UTC 2010 Chris Cleeland <cleeland@ociweb.com>
+
+ * tests/Timer_Queue_Test.cpp (test_interval_timer):
+
+ Added new function to test interval timing functionality in
+ preparation for merging in a fix for bug 3104. Note that while
+ bug 3104 focuses on performance issues, this test doesn't
+ specifically test performance, but rather tests that the
+ interval timer feature works properly so that we know that
+ any performance-related changes made in fixing bug 3104 don't
+ cause a regression.
+
+ * ace/Timer_Queue_T.h:
+ * ace/Timer_Queue_T.cpp:
+
+ Integrate code proposed to fix bug 3104, but factored out into
+ a new method, recompute_next_abs_interval_time, so that the
+ more efficient computation can be re-used in Timer_Hash and
+ Timer_Wheel.
+
+ * ace/Timer_Queue_T.cpp (dispatch_info_i):
+
+ Change to use recompute_next_abs_interval_time.
+
+ * ace/Timer_Hash_T.cpp (expire):
+ * ace/Timer_Wheel_T.cpp (expire):
+
+ Change to use recompute_next_abs_interval_time.
+
Thu Feb 25 20:33:27 UTC 2010 Steve Huston <shuston@riverace.com>
* tests/Bug_2609_Regression_Test.cpp: Correct to use documented ACE
diff --git a/ACE/ace/Timer_Hash_T.cpp b/ACE/ace/Timer_Hash_T.cpp
index a2a81fc3773..86dec8fe378 100644
--- a/ACE/ace/Timer_Hash_T.cpp
+++ b/ACE/ace/Timer_Hash_T.cpp
@@ -823,10 +823,7 @@ ACE_Timer_Hash_T<TYPE, FUNCTOR, ACE_LOCK, BUCKET>::expire (const ACE_Time_Value
{
// Make sure that we skip past values that have already
// "expired".
- do
- expired->set_timer_value (expired->get_timer_value ()
- + expired->get_interval ());
- while (expired->get_timer_value () <= cur_time);
+ this->recompute_next_abs_interval_time (expired, cur_time);
// Since this is an interval timer, we need to
// reschedule it.
diff --git a/ACE/ace/Timer_Queue_T.cpp b/ACE/ace/Timer_Queue_T.cpp
index 2ac9a29e9aa..23d2628970d 100644
--- a/ACE/ace/Timer_Queue_T.cpp
+++ b/ACE/ace/Timer_Queue_T.cpp
@@ -283,6 +283,66 @@ ACE_Timer_Queue_T<TYPE, FUNCTOR, ACE_LOCK>::expire (const ACE_Time_Value &cur_ti
return number_of_timers_expired;
}
+template <class TYPE, class FUNCTOR, class ACE_LOCK> void
+ACE_Timer_Queue_T<TYPE, FUNCTOR, ACE_LOCK>::recompute_next_abs_interval_time
+ (ACE_Timer_Node_T<TYPE> *expired,
+ const ACE_Time_Value &cur_time)
+{
+ if ( expired->get_timer_value () <= cur_time )
+ {
+ /*
+ * Somehow the current time is past when this time was
+ * supposed to expire (e.g., timer took too long,
+ * somebody changed system time, etc.). There used to
+ * be a simple loop here that skipped ahead one timer
+ * interval at a time, but that was horribly inefficient
+ * (an O(n) algorithm) when the timer duration was small
+ * relative to the amount of time skipped.
+ *
+ * So, we replace the loop with a simple computation,
+ * which also happens to be O(1). All times get
+ * normalized in the computation to microseconds.
+ *
+ * For reference, the loop looked like this:
+ *
+ * do
+ * expired->set_timer_value (expired->get_timer_value () +
+ * expired->get_interval ());
+ * while (expired->get_timer_value () <= cur_time);
+ *
+ */
+
+ // Compute the duration of the timer's interval
+ ACE_UINT64 interval_usec;
+ expired->get_interval ().to_usec (interval_usec);
+
+ // Compute the span between the current time and when
+ // the timer would have expired in the past (and
+ // normalize to microseconds).
+ ACE_Time_Value old_diff = cur_time - expired->get_timer_value ();
+ ACE_UINT64 old_diff_usec;
+ old_diff.to_usec (old_diff_usec);
+
+ // Compute the delta time in the future when the timer
+ // should fire as if it had advanced incrementally. The
+ // modulo arithmetic accomodates the likely case that
+ // the current time doesn't fall precisely on a timer
+ // firing interval.
+ ACE_UINT64 new_timer_usec =
+ interval_usec - (old_diff_usec % interval_usec);
+
+ // Compute the absolute time in the future when this
+ // interval timer should expire.
+ ACE_Time_Value new_timer_value
+ (cur_time.sec ()
+ + static_cast<time_t>(new_timer_usec / ACE_ONE_SECOND_IN_USECS),
+ cur_time.usec ()
+ + static_cast<suseconds_t>(new_timer_usec % ACE_ONE_SECOND_IN_USECS));
+
+ expired->set_timer_value (new_timer_value);
+ }
+}
+
template <class TYPE, class FUNCTOR, class ACE_LOCK> int
ACE_Timer_Queue_T<TYPE, FUNCTOR, ACE_LOCK>::dispatch_info_i (const ACE_Time_Value &cur_time,
ACE_Timer_Node_Dispatch_Info_T<TYPE> &info)
@@ -306,10 +366,7 @@ ACE_Timer_Queue_T<TYPE, FUNCTOR, ACE_LOCK>::dispatch_info_i (const ACE_Time_Valu
{
// Make sure that we skip past values that have already
// "expired".
- do
- expired->set_timer_value (expired->get_timer_value () +
- expired->get_interval ());
- while (expired->get_timer_value () <= cur_time);
+ this->recompute_next_abs_interval_time (expired, cur_time);
// Since this is an interval timer, we need to reschedule
// it.
diff --git a/ACE/ace/Timer_Queue_T.h b/ACE/ace/Timer_Queue_T.h
index 8aef637579e..4ed6fd4a30f 100644
--- a/ACE/ace/Timer_Queue_T.h
+++ b/ACE/ace/Timer_Queue_T.h
@@ -431,6 +431,10 @@ protected:
virtual int dispatch_info_i (const ACE_Time_Value &current_time,
ACE_Timer_Node_Dispatch_Info_T<TYPE> &info);
+ /// Recompute when the next time is that this interval timer should fire.
+ void recompute_next_abs_interval_time (ACE_Timer_Node_T<TYPE>* expired,
+ const ACE_Time_Value &cur_time);
+
/// Synchronization variable for ACE_Timer_Queue.
/// @note The right name would be lock_, but HP/C++ will choke on that!
ACE_LOCK mutex_;
diff --git a/ACE/ace/Timer_Wheel_T.cpp b/ACE/ace/Timer_Wheel_T.cpp
index f306905365c..d7734e6b120 100644
--- a/ACE/ace/Timer_Wheel_T.cpp
+++ b/ACE/ace/Timer_Wheel_T.cpp
@@ -834,10 +834,7 @@ ACE_Timer_Wheel_T<TYPE, FUNCTOR, ACE_LOCK>::expire (const ACE_Time_Value& cur_ti
{
// Make sure that we skip past values that have already
// "expired".
- do
- n->set_timer_value (n->get_timer_value () +
- n->get_interval ());
- while (n->get_timer_value () <= cur_time);
+ this->recompute_next_abs_interval_time (n, cur_time);
this->reschedule (n);
}
diff --git a/ACE/tests/Timer_Queue_Test.cpp b/ACE/tests/Timer_Queue_Test.cpp
index f6a76ee3465..0ea1eb23fa4 100644
--- a/ACE/tests/Timer_Queue_Test.cpp
+++ b/ACE/tests/Timer_Queue_Test.cpp
@@ -87,6 +87,56 @@ public:
// Keeps track of the number of times that <handle_close> is called.
};
+
+struct Interval_Handler : public ACE_Event_Handler
+{
+ Interval_Handler (void) : trip_count_ (0) { }
+
+ virtual int handle_timeout (const ACE_Time_Value & , const void *arg)
+ {
+ ++trip_count_;
+ return 0;
+ }
+
+ unsigned trip_count_; // number of times handle_timeout has been tripped.
+};
+
+static void
+test_interval_timer (ACE_Timer_Queue *tq)
+{
+ /*
+ The strategy:
+
+ Set up a timer to fire on a 50ms interval.
+ */
+ Interval_Handler ih;
+ ACE_Time_Value interval (0, 50 * 1000 /* number of usec in millisecond */);
+ const unsigned NUM_INTERVAL_FIRINGS = 50;
+ ACE_Time_Value loop_stop_time =
+ tq->gettimeofday () + (NUM_INTERVAL_FIRINGS * interval);
+ const unsigned EXPECTED_TRIP_COUNT =
+ NUM_INTERVAL_FIRINGS + 1 /* for the first immediate firing */;
+
+ long id = tq->schedule (&ih, 0 /* no act */, ACE_Time_Value::zero, interval);
+ ACE_ASSERT (id != -1);
+
+ do
+ {
+ tq->expire ();
+ }
+ while (tq->gettimeofday () < loop_stop_time);
+
+ ACE_DEBUG((LM_DEBUG,
+ ACE_TEXT("after interval loop, timer fired %d ")
+ ACE_TEXT("times out of %d expected: %s\n"),
+ ih.trip_count_, EXPECTED_TRIP_COUNT,
+ ih.trip_count_ == EXPECTED_TRIP_COUNT
+ ? ACE_TEXT ("success") : ACE_TEXT ("FAIL")
+ ));
+
+ tq->cancel (id);
+}
+
static void
test_functionality (ACE_Timer_Queue *tq)
{
@@ -646,6 +696,7 @@ run_main (int argc, ACE_TCHAR *argv[])
ACE_DEBUG ((LM_DEBUG,
ACE_TEXT ("**** starting test of %s\n"),
tq_ptr->name_));
+ test_interval_timer (tq_ptr->queue_);
test_functionality (tq_ptr->queue_);
test_performance (tq_ptr->queue_,
tq_ptr->name_);