ChangeLogTag:Fri Mar 26 14:07:55 UTC 2010 Steve Huston <shuston@riverace.com>

5 files changed, 32 insertions, 50 deletions

diff --git a/ACE/ChangeLog b/ACE/ChangeLog
index 5fb2daa388f..f0d41711d7d 100644
--- a/ACE/ChangeLog
+++ b/ACE/ChangeLog
@@ -1,3 +1,13 @@
+Fri Mar 26 14:07:55 UTC 2010  Steve Huston  <shuston@riverace.com>
+
+        * ace/Dev_Poll_Reactor.{h inl cpp}: Refinements to the way notifies
+          are handled:
+            1. The notify handler was being dispatched multiple times per
+               notify; this has been fixed, which increases performance.
+            2. Notifications are now dispatched in only one thread at a time.
+               This brings more parity to the way notifies are handled in
+               other reactor implementations.
+
 Fri Mar 26 13:43:08 UTC 2010  Johnny Willemsen  <jwillemsen@remedy.nl>
 
         * debianbuild/tao-utils.install:
diff --git a/ACE/NEWS b/ACE/NEWS
index ac4a5e70f9e..5a71dd54ad0 100644
--- a/ACE/NEWS
+++ b/ACE/NEWS
@@ -14,6 +14,10 @@ USER VISIBLE CHANGES BETWEEN ACE-5.7.7 and ACE-5.7.8
   directives then the directives from svc.conf can not override
   results of the user provided command-line directives.
 
+. ACE_Dev_Poll_Reactor now dispatches notifications in only one thread at
+  a time. This brings notification handling more in line with behavior in
+  other Reactor implementations.
+
 USER VISIBLE CHANGES BETWEEN ACE-5.7.6 and ACE-5.7.7
 ====================================================
 
diff --git a/ACE/ace/Dev_Poll_Reactor.cpp b/ACE/ace/Dev_Poll_Reactor.cpp
index 70ff3451be4..1c26947fb7c 100644
--- a/ACE/ace/Dev_Poll_Reactor.cpp
+++ b/ACE/ace/Dev_Poll_Reactor.cpp
@@ -53,6 +53,7 @@ ACE_Dev_Poll_Reactor_Notify::ACE_Dev_Poll_Reactor_Notify (void)
 #if defined (ACE_HAS_REACTOR_NOTIFICATION_QUEUE)
   , notification_queue_ ()
 #endif  /* ACE_HAS_REACTOR_NOTIFICATION_QUEUE */
+  , dispatching_ (false)
 {
 }
 
@@ -279,12 +280,12 @@ ACE_Dev_Poll_Reactor_Notify::handle_input (ACE_HANDLE handle)
 {
   ACE_TRACE ("ACE_Dev_Poll_Reactor_Notify::handle_input");
 
-  // @@ We may end up dispatching this event handler twice:  once when
-  //    performing the speculative read on the notification pipe
-  //    handle, and once more when dispatching the IO events.
-
-  // Precondition: this->select_reactor_.token_.current_owner () ==
-  // ACE_Thread::self ();
+  {
+    ACE_MT (ACE_GUARD_RETURN (ACE_SYNCH_MUTEX, mon, this->dispatching_lock_, -1));
+    if (this->dispatching_)
+      return 0;
+    this->dispatching_ = true;
+  }
 
   int number_dispatched = 0;
   int result = 0;
@@ -311,11 +312,7 @@ ACE_Dev_Poll_Reactor_Notify::handle_input (ACE_HANDLE handle)
       number_dispatched = -1;
     }
 
-  // Enqueue ourselves into the list of waiting threads.  When we
-  // reacquire the token we'll be off and running again with ownership
-  // of the token.  The postcondition of this call is that
-  // <select_reactor_.token_.current_owner> == <ACE_Thread::self>.
-  //this->select_reactor_->renew ();
+  this->dispatching_ = false;
 
   return number_dispatched;
 }
@@ -1336,11 +1333,14 @@ ACE_Dev_Poll_Reactor::dispatch_io_event (Token_Guard &guard)
         guard.release_token ();
 
         // Dispatch the detected event; will do the repeated upcalls
-        // if callback returns > 0. We come back with either 0 or < 0.
+        // if callback returns > 0, unless it's the notify handler (which
+        // returns the number of notfies dispatched, not an indication of
+        // re-callback requested). If anything other than the notify, come
+        // back with either 0 or < 0.
         status = this->upcall (eh, callback, handle);
 
         if (eh == this->notify_handler_)
-          return status == 0 ? 1 : -1;
+          return status;
 
         // If the callback returned 0, epoll-based needs to resume the
         // suspended handler but dev/poll doesn't.
diff --git a/ACE/ace/Dev_Poll_Reactor.h b/ACE/ace/Dev_Poll_Reactor.h
index 53b8befb7cb..b2f331ece56 100644
--- a/ACE/ace/Dev_Poll_Reactor.h
+++ b/ACE/ace/Dev_Poll_Reactor.h
@@ -246,6 +246,10 @@ protected:
   ACE_Notification_Queue notification_queue_;
 #endif /* ACE_HAS_REACTOR_NOTIFICATION_QUEUE */
 
+  /// Lock and flag to say whether we're already dispatching notifies.
+  /// Purpose is to only dispatch notifies from one thread at a time.
+  ACE_SYNCH_MUTEX dispatching_lock_;
+  bool dispatching_;
 };
 
 // ---------------------------------------------------------------------
diff --git a/ACE/ace/Dev_Poll_Reactor.inl b/ACE/ace/Dev_Poll_Reactor.inl
index d031e06179f..b8392e7bb02 100644
--- a/ACE/ace/Dev_Poll_Reactor.inl
+++ b/ACE/ace/Dev_Poll_Reactor.inl
@@ -57,33 +57,6 @@ ACE_Dev_Poll_Handler_Guard::ACE_Dev_Poll_Handler_Guard
 
   if (do_incr && this->refcounted_)
     eh->add_reference ();
-
-  /**
-   * The below comments were here when I replaced the old refcount
-   * scheme was replaced. They may still need addressing.   -Steve Huston
-   */
-
-  /**
-   * @todo Suspend the handler so that other threads will not cause
-   *       an event that is already in an upcall from being dispatched
-   *       again.
-   *
-   * @note The naive approach would be to simply call
-   *       suspend_handler_i() on the reactor.  However, that would
-   *       cause a system call (write()) to occur.  Obviously this
-   *       can potentially have an adverse affect on performance.
-   *       Ideally, the handler would only be marked as "suspended" in
-   *       the handler repository.  If an event arrives for a
-   *       suspended handler that event can be "queued" in a
-   *       "handle readiness queue."  "Queued" is quoted since a real
-   *       queue need not be used since duplicate events can be
-   *       coalesced, thus avoiding unbounded queue growth.  Event
-   *       coalescing is already done by Linux's event poll driver
-   *       (/dev/epoll) so Solaris' poll driver (/dev/poll) is the
-   *       main concern here.  The largest the queue can be is the
-   *       same size as the number of handlers stored in the handler
-   *       repository.
-   */
 }
 
 ACE_INLINE
@@ -91,15 +64,6 @@ ACE_Dev_Poll_Handler_Guard::~ACE_Dev_Poll_Handler_Guard (void)
 {
   if (this->refcounted_ && this->eh_ != 0)
     this->eh_->remove_reference ();
-
-  /**
-   * The below comments were here when I replaced the old refcount
-   * scheme was replaced. They may still need addressing.   -Steve Huston
-   */
-  /**
-   * @todo Resume the handler so that other threads will be allowed to
-   *       dispatch the handler.
-   */
 }
 
 ACE_INLINE void
@@ -125,7 +89,7 @@ ACE_Dev_Poll_Reactor::upcall (ACE_Event_Handler *event_handler,
     {
       status = (event_handler->*callback) (handle);
     }
-  while (status > 0);
+  while (status > 0 && event_handler != this->notify_handler_);
 
   return status;
 }