Mon Feb 19 21:29:32 UTC 2007 Carlos O'Ryan <coryan@atdesk.com>

5 files changed, 62 insertions, 22 deletions

diff --git a/ChangeLog b/ChangeLog
index 54afef3c33e..227a7d242b3 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,30 @@
+Mon Feb 19 21:29:32 UTC 2007  Carlos O'Ryan  <coryan@atdesk.com>
+
+        * ace/Notification_Queue.cpp:
+          Modified the purging algorithm.  The algorithm used a temporary
+          list to store the elements not purged, so basically all nodes
+          were either moved to the free list or to the temporary list.
+          The new algorithm only removes the nodes that needs purging.
+          In the vast majority of the cases this is more efficient because
+          most nodes are not purged.
+          This resulted in a factor of 2 improvement for
+          tests/Bug_2815_Regression_Test, keep in mind that in this
+          program 1/16th of the nodes are purged.  So in practice I would
+          expect even better results.
+          I also changed the order in which the free nodes are re-used, I
+          think LIFO order has a better chance of re-using the cache, but
+          I have no evidence or experiments to prove this.
+
+        * ace/Intrusive_List.h:
+        * ace/Intrusive_List.cpp:
+          Renamed the remove_i() function to unsafe_remove() and promoted
+          its access to "public".  The function is not safe to use in
+          general, thus the name, but it resulted in a factor of 2
+          performance improvement for ACE_Notification_Queue.
+
+        * tests/Bug_2815_Regression_Test.cpp:
+          Fixed memory leaks (in the test not the library)
+
 Mon Feb 19 20:32:34 UTC 2007  Carlos O'Ryan  <coryan@atdesk.com>
 
         * ace/Notification_Queue.h:
diff --git a/ace/Intrusive_List.cpp b/ace/Intrusive_List.cpp
index 4a374c9b045..c0006792ce4 100644
--- a/ace/Intrusive_List.cpp
+++ b/ace/Intrusive_List.cpp
@@ -69,7 +69,7 @@ ACE_Intrusive_List<T>::pop_front (void)
   T *node = this->head_;
   if (node == 0)
     return 0;
-  this->remove_i (node);
+  this->unsafe_remove (node);
   return node;
 }
 
@@ -79,7 +79,7 @@ ACE_Intrusive_List<T>::pop_back (void)
   T *node = this->tail_;
   if (node == 0)
     return 0;
-  this->remove_i (node);
+  this->unsafe_remove (node);
   return node;
 }
 
@@ -90,14 +90,14 @@ ACE_Intrusive_List<T>::remove (T *node)
     {
       if (node == i)
         {
-          this->remove_i (node);
+          this->unsafe_remove (node);
           return;
         }
     }
 }
 
 template<class T> void
-ACE_Intrusive_List<T>::remove_i (T *node)
+ACE_Intrusive_List<T>::unsafe_remove (T *node)
 {
   if (node->prev () != 0)
     node->prev ()->next (node->next ());
diff --git a/ace/Intrusive_List.h b/ace/Intrusive_List.h
index 350899ef04c..59d0c9054a8 100644
--- a/ace/Intrusive_List.h
+++ b/ace/Intrusive_List.h
@@ -102,14 +102,14 @@ public:
   /// Swap two lists
   void swap(ACE_Intrusive_List<T> & rhs);
 
-private:
-  /// Remove a element from the list
+  /// Remove a element from the list without checking
   /**
    * No attempts are performed to check if T* really belongs to the
    * list.  The effects of removing an invalid element are unspecified
    */
-  void remove_i (T *node);
+  void unsafe_remove (T *node);
 
+private:
   /** @name Disallow copying
    *
    */
diff --git a/ace/Notification_Queue.cpp b/ace/Notification_Queue.cpp
index ee700a81ff7..0b2ca59e831 100644
--- a/ace/Notification_Queue.cpp
+++ b/ace/Notification_Queue.cpp
@@ -96,35 +96,45 @@ purge_pending_notifications(ACE_Event_Handler * eh,
   if (this->notify_queue_.is_empty ())
     return 0;
 
-  Buffer_List local_queue;
-
   int number_purged = 0;
-  while(!notify_queue_.is_empty())
+  ACE_Notification_Queue_Node * node = notify_queue_.head();
+  while(node != 0)
     {
-      ACE_Notification_Queue_Node * node = notify_queue_.pop_front();
-
       if (!node->matches_for_purging(eh))
 	{
-	  // Easy case, save the node and continue;
-	  local_queue.push_back(node);
+	  // Easy case, skip to the next node
+          node = node->next();
 	  continue;
 	}
 
       if (!node->mask_disables_all_notifications(mask))
 	{
+          // ... another easy case, skip this node too, but clear the
+          // mask first ...
 	  node->clear_mask(mask);
-	  local_queue.push_back(node);
+          node = node->next();
 	  continue;
 	}
 
-      free_queue_.push_back(node);
+      // ... this is the more complicated case, we want to remove the
+      // node from the notify_queue_ list.  First save the next node
+      // on the list:
+      ACE_Notification_Queue_Node * next = node->next();
+
+      // ... then remove it ...
+      notify_queue_.unsafe_remove(node);
+      ++number_purged;
+
+      // ... release resources ...
       ACE_Event_Handler *event_handler = node->get().eh_;
       event_handler->remove_reference ();
-      ++number_purged;
-    }
+      
+      // ... now this is a free node ...
+      free_queue_.push_front(node);
 
-  // now put it back in the notify queue
-  local_queue.swap(notify_queue_);
+      // ... go to the next node, if there is one ...
+      node = next;
+    }
 
   return number_purged;
 }
@@ -188,7 +198,7 @@ ACE_Notification_Queue::pop_next_notification(
     notify_queue_.pop_front();
 
   current = node->get();
-  free_queue_.push_back(node);
+  free_queue_.push_front(node);
 
   if(!this->notify_queue_.is_empty())
     {
diff --git a/tests/Bug_2815_Regression_Test.cpp b/tests/Bug_2815_Regression_Test.cpp
index e6c7765f532..9600c530922 100644
--- a/tests/Bug_2815_Regression_Test.cpp
+++ b/tests/Bug_2815_Regression_Test.cpp
@@ -486,11 +486,14 @@ handle_exception(ACE_HANDLE)
   if (r >= 0)
     {
       master_handler_->notifications_skipped(r);
+      delete this;
       return 0;
     }
 	
   ACE_ERROR((LM_ERROR,
 	     ACE_TEXT ("Cannot remove handler %d in %C test\n"),
 	     id_, test_name_));
-  return -1;
+
+  delete this;
+  return 0;
 }