diff options
| -rw-r--r-- | ChangeLog | 21 | ||||
| -rw-r--r-- | ChangeLogs/ChangeLog-02a | 21 | ||||
| -rw-r--r-- | ChangeLogs/ChangeLog-03a | 21 | ||||
| -rw-r--r-- | ace/Timer_Hash_T.cpp | 15 | ||||
| -rw-r--r-- | ace/Timer_Heap_T.cpp | 75 | ||||
| -rw-r--r-- | ace/Timer_Heap_T.h | 6 | ||||
| -rw-r--r-- | tests/Timer_Queue_Test.cpp | 1 |
7 files changed, 118 insertions, 42 deletions
diff --git a/ChangeLog b/ChangeLog index f5fb8e81c65..27712d93c62 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,11 +1,28 @@ +Thu May 24 20:26:39 2001 Steve Huston <shuston@riverace.com> + + * ace/Timer_Heap_T.{h cpp}: Fixed some problems in managing the + free timer ID list. + + * ace/Timer_Hash_T.cpp (expire): Changed this to peek at the first + timer node from each table, do reschedule iterative timers, and + do the upcall, then _cancel_ the original, rather than try to + manage a remove_first()/reschedule-or-free sequence. The ID + assigned by the underlying table is not important, since its not + seen by the ACE_Timer_Hash_T caller anyway, and that's the main + impetus for using remove_first/reschedule (or free_node). + + * tests/Timer_Queue_Test.cpp (test_functionality): Added a test + to verify empty timer queue after adding/removing two timers + right at the start. + Thu May 24 12:15:35 2001 Douglas C. Schmidt <schmidt@tango.doc.wustl.edu> * ace/OS.h: Added #defines for RTLD_LAZY, RTLD_NOW, RTLD_GLOBAL. Thanks to Kobi Cohen-Arazi <kobic@bvr.co.il> for suggesting this. - * ace/OS.h: Updated the ACE_Time_Value class to point out that the values - are in secs and usecs. Thanks to Israel Illescas Gomez + * ace/OS.h: Updated the ACE_Time_Value class to point out that the + values are in secs and usecs. Thanks to Israel Illescas Gomez <illescas@dycsa.es> for motivating this. Thu May 24 18:08:27 2001 Steve Huston <shuston@riverace.com> diff --git a/ChangeLogs/ChangeLog-02a b/ChangeLogs/ChangeLog-02a index f5fb8e81c65..27712d93c62 100644 --- a/ChangeLogs/ChangeLog-02a +++ b/ChangeLogs/ChangeLog-02a @@ -1,11 +1,28 @@ +Thu May 24 20:26:39 2001 Steve Huston <shuston@riverace.com> + + * ace/Timer_Heap_T.{h cpp}: Fixed some problems in managing the + free timer ID list. + + * ace/Timer_Hash_T.cpp (expire): Changed this to peek at the first + timer node from each table, do reschedule iterative timers, and + do the upcall, then _cancel_ the original, rather than try to + manage a remove_first()/reschedule-or-free sequence. The ID + assigned by the underlying table is not important, since its not + seen by the ACE_Timer_Hash_T caller anyway, and that's the main + impetus for using remove_first/reschedule (or free_node). + + * tests/Timer_Queue_Test.cpp (test_functionality): Added a test + to verify empty timer queue after adding/removing two timers + right at the start. + Thu May 24 12:15:35 2001 Douglas C. Schmidt <schmidt@tango.doc.wustl.edu> * ace/OS.h: Added #defines for RTLD_LAZY, RTLD_NOW, RTLD_GLOBAL. Thanks to Kobi Cohen-Arazi <kobic@bvr.co.il> for suggesting this. - * ace/OS.h: Updated the ACE_Time_Value class to point out that the values - are in secs and usecs. Thanks to Israel Illescas Gomez + * ace/OS.h: Updated the ACE_Time_Value class to point out that the + values are in secs and usecs. Thanks to Israel Illescas Gomez <illescas@dycsa.es> for motivating this. Thu May 24 18:08:27 2001 Steve Huston <shuston@riverace.com> diff --git a/ChangeLogs/ChangeLog-03a b/ChangeLogs/ChangeLog-03a index f5fb8e81c65..27712d93c62 100644 --- a/ChangeLogs/ChangeLog-03a +++ b/ChangeLogs/ChangeLog-03a @@ -1,11 +1,28 @@ +Thu May 24 20:26:39 2001 Steve Huston <shuston@riverace.com> + + * ace/Timer_Heap_T.{h cpp}: Fixed some problems in managing the + free timer ID list. + + * ace/Timer_Hash_T.cpp (expire): Changed this to peek at the first + timer node from each table, do reschedule iterative timers, and + do the upcall, then _cancel_ the original, rather than try to + manage a remove_first()/reschedule-or-free sequence. The ID + assigned by the underlying table is not important, since its not + seen by the ACE_Timer_Hash_T caller anyway, and that's the main + impetus for using remove_first/reschedule (or free_node). + + * tests/Timer_Queue_Test.cpp (test_functionality): Added a test + to verify empty timer queue after adding/removing two timers + right at the start. + Thu May 24 12:15:35 2001 Douglas C. Schmidt <schmidt@tango.doc.wustl.edu> * ace/OS.h: Added #defines for RTLD_LAZY, RTLD_NOW, RTLD_GLOBAL. Thanks to Kobi Cohen-Arazi <kobic@bvr.co.il> for suggesting this. - * ace/OS.h: Updated the ACE_Time_Value class to point out that the values - are in secs and usecs. Thanks to Israel Illescas Gomez + * ace/OS.h: Updated the ACE_Time_Value class to point out that the + values are in secs and usecs. Thanks to Israel Illescas Gomez <illescas@dycsa.es> for motivating this. Thu May 24 18:08:27 2001 Steve Huston <shuston@riverace.com> diff --git a/ace/Timer_Hash_T.cpp b/ace/Timer_Hash_T.cpp index 07e65bb791a..16c6074c595 100644 --- a/ace/Timer_Hash_T.cpp +++ b/ace/Timer_Hash_T.cpp @@ -565,9 +565,8 @@ ACE_Timer_Hash_T<TYPE, FUNCTOR, ACE_LOCK, BUCKET>::expire (const ACE_Time_Value while (!this->table_[i]->is_empty () && this->table_[i]->earliest_time () <= cur_time) { - expired = this->table_[i]->remove_first (); - --this->size_; - TYPE &type = expired->get_type (); + expired = this->table_[i]->get_first (); + TYPE type = expired->get_type (); const void *act = expired->get_act (); int reclaim = 1; @@ -587,6 +586,13 @@ ACE_Timer_Hash_T<TYPE, FUNCTOR, ACE_LOCK, BUCKET>::expire (const ACE_Time_Value reclaim = 0; } + // Now remove the timer from the original table... if + // it's a simple, non-recurring timer, it's got to be + // removed anyway. If it was rescheduled, it's been + // scheduled into the correct table (regardless of whether + // it's the same one or not) already. + this->table_[i]->cancel (expired->get_timer_id ()); + Hash_Token *h = ACE_reinterpret_cast (Hash_Token *, ACE_const_cast (void *, act)); @@ -596,8 +602,7 @@ ACE_Timer_Hash_T<TYPE, FUNCTOR, ACE_LOCK, BUCKET>::expire (const ACE_Time_Value cur_time); if (reclaim) { - // Free up the node and the token - this->free_node (expired); + --this->size_; delete h; } number_of_timers_expired++; diff --git a/ace/Timer_Heap_T.cpp b/ace/Timer_Heap_T.cpp index f35380576d1..f86c7563776 100644 --- a/ace/Timer_Heap_T.cpp +++ b/ace/Timer_Heap_T.cpp @@ -67,7 +67,10 @@ ACE_Timer_Heap_Iterator_T<TYPE, FUNCTOR, ACE_LOCK>::item (void) } // Constructor - +// Note that timer_ids_curr_ and timer_ids_min_free_ both start at 0. +// Since timer IDs are assigned by first incrementing the timer_ids_curr_ +// value, the first ID assigned will be 1 (just as in the previous design). +// When it's time to wrap, the next ID given out will be 0. template <class TYPE, class FUNCTOR, class ACE_LOCK> ACE_Timer_Heap_T<TYPE, FUNCTOR, ACE_LOCK>::ACE_Timer_Heap_T (size_t size, int preallocate, @@ -76,8 +79,8 @@ ACE_Timer_Heap_T<TYPE, FUNCTOR, ACE_LOCK>::ACE_Timer_Heap_T (size_t size, : ACE_Timer_Queue_T<TYPE,FUNCTOR,ACE_LOCK> (upcall_functor, freelist), max_size_ (size), cur_size_ (0), - timer_ids_next_ (0), - timer_ids_min_free_ (size), + timer_ids_curr_ (0), + timer_ids_min_free_ (0), preallocated_nodes_ (0), preallocated_nodes_freelist_ (0) { @@ -122,14 +125,18 @@ ACE_Timer_Heap_T<TYPE, FUNCTOR, ACE_LOCK>::ACE_Timer_Heap_T (size_t size, HEAP_ITERATOR (*this)); } +// Note that timer_ids_curr_ and timer_ids_min_free_ both start at 0. +// Since timer IDs are assigned by first incrementing the timer_ids_curr_ +// value, the first ID assigned will be 1 (just as in the previous design). +// When it's time to wrap, the next ID given out will be 0. template <class TYPE, class FUNCTOR, class ACE_LOCK> ACE_Timer_Heap_T<TYPE, FUNCTOR, ACE_LOCK>::ACE_Timer_Heap_T (FUNCTOR *upcall_functor, ACE_Free_List<ACE_Timer_Node_T <TYPE> > *freelist) : ACE_Timer_Queue_T<TYPE,FUNCTOR,ACE_LOCK> (upcall_functor, freelist), max_size_ (ACE_DEFAULT_TIMERS), cur_size_ (0), - timer_ids_next_ (0), - timer_ids_min_free_ (ACE_DEFAULT_TIMERS), + timer_ids_curr_ (0), + timer_ids_min_free_ (0), preallocated_nodes_ (0), preallocated_nodes_freelist_ (0) { @@ -197,28 +204,28 @@ ACE_Timer_Heap_T<TYPE, FUNCTOR, ACE_LOCK>::pop_freelist (void) // Scan for a free timer ID. Note that since this function is called // _after_ the check for a full timer heap, we are guaranteed to find - // a free ID. The next_ index was left at the previous allocated ID - // and we pick up where we left off last time. - ++this->timer_ids_next_; - if (this->timer_ids_next_ == this->max_size_) + // a free ID, even if we need to wrap around and start reusing freed IDs. + // On entry, the curr_ index is at the previous ID given out; start + // up where we left off last time. + ++this->timer_ids_curr_; + while (this->timer_ids_curr_ < this->max_size_ && + this->timer_ids_[this->timer_ids_curr_] >= 0) + ++this->timer_ids_curr_; + if (this->timer_ids_curr_ == this->max_size_) { ACE_ASSERT (this->timer_ids_min_free_ < this->max_size_); - this->timer_ids_next_ = this->timer_ids_min_free_; + this->timer_ids_curr_ = this->timer_ids_min_free_; // We restarted the free search at min. Since min won't be - // free anymore, find the next lowest free entry, recognizing - // that there may not be any more; in that case, min_free_ - // gets left saying "none free" and will be reset next time - // an entry is freed. - while (++this->timer_ids_min_free_ < this->max_size_) - if (this->timer_ids_[this->timer_ids_min_free_] < 0) - break; + // free anymore, and curr_ will just keep marching up the list + // on each successive need for an ID, reset min_free_ to the + // size of the list until an ID is freed that curr_ has already + // gone past (see push_freelist). + this->timer_ids_min_free_ = this->max_size_; } - while (this->timer_ids_[this->timer_ids_next_] >= 0) - ++this->timer_ids_next_; // We need to truncate this to <int> for backwards compatibility. int new_id = ACE_static_cast (int, - this->timer_ids_next_); + this->timer_ids_curr_); return new_id; } @@ -227,16 +234,18 @@ ACE_Timer_Heap_T<TYPE, FUNCTOR, ACE_LOCK>::push_freelist (int old_id) { ACE_TRACE ("ACE_Timer_Heap_T::push_freelist"); - // Since this ID has alreayd been checked by one of the public + // Since this ID has already been checked by one of the public // functions, it's safe to cast it here. size_t oldid = size_t (old_id); // The freelist values in the <timer_ids_> are negative, so set the // freed entry back to 'free'. If this is the new lowest value free - // timer ID, remember it. + // timer ID that curr_ won't see on it's normal march through the list, + // remember it. this->timer_ids_[oldid] = -1; - if (oldid < this->timer_ids_min_free_) + if (oldid < this->timer_ids_min_free_ && oldid <= this->timer_ids_curr_) this->timer_ids_min_free_ = oldid; + return; } template <class TYPE, class FUNCTOR, class ACE_LOCK> int @@ -281,8 +290,8 @@ ACE_Timer_Heap_T<TYPE, FUNCTOR, ACE_LOCK>::dump (void) const ACE_DEBUG ((LM_DEBUG, ACE_LIB_TEXT ("\nmax_size_ = %d"), this->max_size_)); ACE_DEBUG ((LM_DEBUG, ACE_LIB_TEXT ("\ncur_size_ = %d"), this->cur_size_)); - ACE_DEBUG ((LM_DEBUG, ACE_LIB_TEXT ("\nids_next_ = %d"), - this->timer_ids_next_)); + ACE_DEBUG ((LM_DEBUG, ACE_LIB_TEXT ("\nids_curr_ = %d"), + this->timer_ids_curr_)); ACE_DEBUG ((LM_DEBUG, ACE_LIB_TEXT ("\nmin_free_ = %d"), this->timer_ids_min_free_)); @@ -321,15 +330,23 @@ ACE_Timer_Heap_T<TYPE, FUNCTOR, ACE_LOCK>::copy (int slot, this->timer_ids_[moved_node->get_timer_id ()] = slot; } +// Remove the slot'th timer node from the heap, but do not reclaim its +// timer ID or change the size of this timer heap object. The caller of +// this function must call either free_node (to reclaim the timer ID +// and the timer node memory, as well as decrement the size of the queue) +// or reschedule (to reinsert the node in the heap at a new time). template <class TYPE, class FUNCTOR, class ACE_LOCK> ACE_Timer_Node_T<TYPE> * ACE_Timer_Heap_T<TYPE, FUNCTOR, ACE_LOCK>::remove (size_t slot) { ACE_Timer_Node_T<TYPE> *removed_node = this->heap_[slot]; - // Decrement the size of the heap by one since we're removing the - // "slot"th node. - this->cur_size_--; + // NOTE - this is a temporary decrement of cur_size_, since other + // member functions that will be called from here use it without + // wanting the removed node included. However, we will bump it back up + // before return to be sure that the removed node is still included in + // the size of this timer heap. + --this->cur_size_; // Only try to reheapify if we're not deleting the last entry. @@ -357,6 +374,7 @@ ACE_Timer_Heap_T<TYPE, FUNCTOR, ACE_LOCK>::remove (size_t slot) parent); } + ++this->cur_size_; return removed_node; } @@ -565,6 +583,7 @@ ACE_Timer_Heap_T<TYPE, FUNCTOR, ACE_LOCK>::free_node (ACE_Timer_Node_T<TYPE> *no // Return this timer id to the freelist. this->push_freelist (node->get_timer_id ()); + --this->cur_size_; // Only free up a node if we are *not* using the preallocated heap. if (this->preallocated_nodes_ == 0) diff --git a/ace/Timer_Heap_T.h b/ace/Timer_Heap_T.h index 0ad14588f98..20e3ce9e408 100644 --- a/ace/Timer_Heap_T.h +++ b/ace/Timer_Heap_T.h @@ -283,9 +283,9 @@ private: */ long *timer_ids_; - /// "Pointer" to the next element in the <timer_ids_> array that will - /// be checked for use when a new timer ID is needed. - size_t timer_ids_next_; + /// "Pointer" to the element in the <timer_ids_> array that was + /// last given out as a timer ID. + size_t timer_ids_curr_; /// Index representing the lowest timer ID that has been freed. When /// the timer_ids_next_ value wraps around, it starts back at this diff --git a/tests/Timer_Queue_Test.cpp b/tests/Timer_Queue_Test.cpp index a9b03aadfd4..e37a0b1d2a9 100644 --- a/tests/Timer_Queue_Test.cpp +++ b/tests/Timer_Queue_Test.cpp @@ -115,6 +115,7 @@ test_functionality (ACE_Timer_Queue *tq) tq->cancel (timer_id); tq->cancel (timer_id2); + ACE_ASSERT (tq->is_empty () == 1); timer_id = tq->schedule (&eh, (const void *) 1, |