ChangeLogTag:Wed Jul 28 13:53:36 1999 Carlos O'Ryan <coryan@cs.wustl.edu>

6 files changed, 41 insertions, 36 deletions

diff --git a/TAO/ChangeLog-99c b/TAO/ChangeLog-99c
index c9198ebacf3..d0cbb885ddd 100644
--- a/TAO/ChangeLog-99c
+++ b/TAO/ChangeLog-99c
@@ -4,6 +4,15 @@ Wed Jul 28 13:53:36 1999  Carlos O'Ryan  <coryan@cs.wustl.edu>
 	  The test was mistakenly not using the octet sequence
 	  optimizations.
 
+	* tao/IIOP_Connect.cpp:
+	* tao/IIOP_Connector.cpp:
+	* tao/IIOP_Transport.cpp:
+	* tao/UIOP_Connect.cpp:
+	* tao/UIOP_Transport.cpp:
+	  Added a bunch of comments about code that could be refactored
+	  and things that were not very clear, thanks to James Megquier
+	  <jmegq@bbn.com> for helping out with this.
+
 Wed Jul 28 13:44:14 1999  Carlos O'Ryan  <coryan@cs.wustl.edu>
 
 	* orbsvcs/examples/RtEC/Makefile:
diff --git a/TAO/tao/IIOP_Connect.cpp b/TAO/tao/IIOP_Connect.cpp
index 90410bf0249..f2126c0ad0a 100644
--- a/TAO/tao/IIOP_Connect.cpp
+++ b/TAO/tao/IIOP_Connect.cpp
@@ -241,6 +241,10 @@ TAO_IIOP_Server_Connection_Handler::handle_input (ACE_HANDLE)
   // it so that nested upcall on the same transport can be handled.
   //
 
+  // Notice that the message_state is only modified in one thread at a 
+  // time because the reactor does not call handle_input() for the
+  // same Event_Handler in two threads at the same time.
+
   // Copy message type.
   CORBA::Octet message_type = this->transport_.message_state_.message_type;
 
diff --git a/TAO/tao/IIOP_Connector.cpp b/TAO/tao/IIOP_Connector.cpp
index 41dfc3175a3..d34eb5d31f0 100644
--- a/TAO/tao/IIOP_Connector.cpp
+++ b/TAO/tao/IIOP_Connector.cpp
@@ -62,6 +62,7 @@ TAO_IIOP_Connector::open (TAO_ORB_Core *orb_core)
   if (result == -1)
     return -1;
 
+  // @@ We should use ACE_NEW here
   this->cached_connect_strategy_ =
     new CACHED_CONNECT_STRATEGY (*this->caching_strategy_,
         new TAO_IIOP_Connect_Creation_Strategy (this->orb_core_->thr_mgr (),
diff --git a/TAO/tao/IIOP_Transport.cpp b/TAO/tao/IIOP_Transport.cpp
index 1d0de12047e..caa49ba61cd 100644
--- a/TAO/tao/IIOP_Transport.cpp
+++ b/TAO/tao/IIOP_Transport.cpp
@@ -225,17 +225,10 @@ int
 TAO_IIOP_Client_Transport::handle_client_input (int /* block */,
                                                 ACE_Time_Value *max_wait_time)
 {
-  // When we multiplex several invocations over a connection we need
-  // to allocate the CDR stream *here*, but when there is a single
-  // request over a connection the CDR stream can be pre-allocated on
-  // the stack of the thread that sent the request!
-  // Can we preserve this optimization on the new architecture?
-  //
-  // here is how:
-  //
-  // Use an "factory" to obtain the CDR stream, in the Muxed case the
-  // factory simply allocates a new one, in the Exclusive case the
-  // factory returns a pointer to the pre-allocated CDR.
+
+  // Notice that the message_state is only modified in one thread at a 
+  // time because the reactor does not call handle_input() for the
+  // same Event_Handler in two threads at the same time.
 
   // Get the message state from the Transport Mux Strategy. 
   TAO_GIOP_Message_State* message_state =
@@ -321,6 +314,8 @@ TAO_IIOP_Client_Transport::handle_client_input (int /* block */,
 int
 TAO_IIOP_Client_Transport::register_handler (void)
 {
+  // @@ It seems like this method should go away, the right reactor is 
+  //    picked at object creation time.
   ACE_Reactor *r = this->orb_core ()->reactor ();
   if (r == this->client_handler ()->reactor ())
     return 0;
@@ -332,6 +327,8 @@ TAO_IIOP_Client_Transport::register_handler (void)
 int
 TAO_IIOP_Client_Transport::check_unexpected_data (void)
 {
+  // @@ It seems like this method should go away, all the error
+  //    detection code is in GIOP::handle_input() now.
   // @@ Alex: This should *not* be part of the client connection
   //    handler, we should treat any incoming data as a GIOP message.
   //    The server can always send the "CloseConnection" message and
@@ -393,6 +390,9 @@ TAO_IIOP_Transport::send (const ACE_Message_Block *mblk,
 {
   TAO_FUNCTION_PP_TIMEPROBE (TAO_IIOP_TRANSPORT_SEND_START);
 
+  // @@ This code should be refactored into ACE.cpp or something
+  // similar!
+
   // For the most part this was copied from GIOP::send_request and
   // friends.
 
diff --git a/TAO/tao/UIOP_Connect.cpp b/TAO/tao/UIOP_Connect.cpp
index f80672866ed..a0216a15d7d 100644
--- a/TAO/tao/UIOP_Connect.cpp
+++ b/TAO/tao/UIOP_Connect.cpp
@@ -234,6 +234,10 @@ TAO_UIOP_Server_Connection_Handler::handle_input (ACE_HANDLE)
   // it so that nested upcall on the same transport can be handled.
   //
 
+  // Notice that the message_state is only modified in one thread at a 
+  // time because the reactor does not call handle_input() for the
+  // same Event_Handler in two threads at the same time.
+
   // Copy message type.
   CORBA::Octet message_type = this->transport_.message_state_.message_type;
 
diff --git a/TAO/tao/UIOP_Transport.cpp b/TAO/tao/UIOP_Transport.cpp
index 7de6447d1ce..54188bd2b21 100644
--- a/TAO/tao/UIOP_Transport.cpp
+++ b/TAO/tao/UIOP_Transport.cpp
@@ -226,32 +226,12 @@ int
 TAO_UIOP_Client_Transport::handle_client_input (int /* block */,
                                                 ACE_Time_Value *max_wait_time)
 {
-  // When we multiplex several invocations over a connection we need
-  // to allocate the CDR stream *here*, but when there is a single
-  // request over a connection the CDR stream can be pre-allocated on
-  // the stack of the thread that sent the request!
-  // Can we preserve this optimization on the new architecture?
-  //
-  // here is how:
-  //
-  // Use an "factory" to obtain the CDR stream, in the Muxed case the
-  // factory simply allocates a new one, in the Exclusive case the
-  // factory returns a pointer to the pre-allocated CDR.
-  //
-  // @@ Alex: I thought some more about this, and here is how i would
-  //    like to do it: this class keeps a CDR stream for the "current"
-  //    message beign received. Initially the CDR is 0, when the
-  //    handle_client_input() is called the first time then we go to
-  //    the muxer to obtain the CDR stream.
-  //    - The exclusive Muxer returns the CDR stream pre-allocated by
-  //      the invocation.
-  //    - The shared Muxer returns a new CDR stream.
-  //    Once all the data has been received the reply handler takes
-  //    charge of the CDR stream, or actually of its message block,
-  //    which is referenced counted and thus can be efficiently
-  //    removed.
-  //    Do I make any sense?
 
+  // Notice that the message_state is only modified in one thread at a 
+  // time because the reactor does not call handle_input() for the
+  // same Event_Handler in two threads at the same time.
+
+  // Get the message state from the Transport Mux Strategy. 
   TAO_GIOP_Message_State* message_state =
     this->tms_->get_message_state ();
 
@@ -336,6 +316,8 @@ TAO_UIOP_Client_Transport::handle_client_input (int /* block */,
 int
 TAO_UIOP_Client_Transport::register_handler (void)
 {
+  // @@ It seems like this method should go away, the right reactor is 
+  //    picked at object creation time.
   ACE_Reactor *r = this->orb_core ()->reactor ();
   if (r == this->client_handler ()->reactor ())
     return 0;
@@ -347,6 +329,8 @@ TAO_UIOP_Client_Transport::register_handler (void)
 int
 TAO_UIOP_Client_Transport::check_unexpected_data (void)
 {
+  // @@ It seems like this method should go away, all the error
+  //    detection code is in GIOP::handle_input() now.
   // @@ Alex: This should *not* be part of the client connection
   //    handler, we should treat any incoming data as a GIOP message.
   //    The server can always send the "CloseConnection" message and
@@ -408,6 +392,9 @@ TAO_UIOP_Transport::send (const ACE_Message_Block *mblk,
 {
   TAO_FUNCTION_PP_TIMEPROBE (TAO_UIOP_TRANSPORT_SEND_START);
 
+  // @@ This code should be refactored into ACE.cpp or something
+  // similar!
+
   // For the most part this was copied from GIOP::send_request and
   // friends.