ChangeLogTag:Tue Mar 2 19:50:50 2004 Balachandran Natarajan <bala@dre.vanderbilt.edu>

2 files changed, 38 insertions, 21 deletions

diff --git a/TAO/CIAO/ChangeLog b/TAO/CIAO/ChangeLog
index a1148bce18b..0d2a27b2657 100644
--- a/TAO/CIAO/ChangeLog
+++ b/TAO/CIAO/ChangeLog
@@ -1,3 +1,9 @@
+Tue Mar  2 19:50:50 2004  Balachandran Natarajan  <bala@dre.vanderbilt.edu>
+
+	* DnC/Node/NodeApplicationManager_Impl.h:
+
+	  Added @@Tao comments. 
+
 Tue Mar  2 15:08:39 2004  Jeff Parsons <j.parsons@vanderbilt.edu>
 
         * CIDLC/ServantSourceGenerator.cpp:
diff --git a/TAO/CIAO/DnC/Node/NodeApplicationManager_Impl.h b/TAO/CIAO/DnC/Node/NodeApplicationManager_Impl.h
index 7690391c434..74445c038d3 100644
--- a/TAO/CIAO/DnC/Node/NodeApplicationManager_Impl.h
+++ b/TAO/CIAO/DnC/Node/NodeApplicationManager_Impl.h
@@ -6,10 +6,11 @@
  * @auther Tao Lu <lu@dre.vanderbilt.edu>
  *
  **/
-
+// @@ Tao: IMHO, its a good practice to have CIAO included to make
+// things as clear as possible.
 #ifndef NODEAPPLICATIONMANAGER_IMPL_H
 #define NODEAPPLICATIONMANAGER_IMPL_H
-
+// @@ Tao: Please include pre.h and post.h.
 #if !defined (ACE_LACKS_PRAGMA_ONCE)
 # pragma once
 #endif /* ACE_LACKS_PRAGMA_ONCE */
@@ -17,9 +18,11 @@
 #include "ace/Synch_Traits.h"
 #include "DnC_DeploymentS.h"
 
+// @@Tao: Since you are doing an implementation of the DnC, should
+// this be in Deployment namespace. Wouldn't it be better put it in
+// CIAO namespace?
 namespace Deployment
 {
-
   /**
    *
    * @class NodeApplicationManager_Impl
@@ -28,6 +31,7 @@ namespace Deployment
    *        This interface is very simillar to the old DnC CIAO_Daemon.
    *
    **/
+  // @@ Tao: Don't you want to inherit from RefcountedServantBase.?
   class NodeApplicationManager_Impl
     : public virtual POA_Deployment::NodeApplicationManager
     {
@@ -35,7 +39,7 @@ namespace Deployment
 
       // constructor.
       NodeApplicationManager (NodeApplicationManagerManager_ptr nam_ptr,
-		       ACE_ENV_ARG_DECL_WITH_DEFAULTS);
+                       ACE_ENV_ARG_DECL_WITH_DEFAULTS);
 
       // Default destructor.
       virtual ~NodeApplicationManager (void);
@@ -48,15 +52,15 @@ namespace Deployment
        */
       virtual ::Deployment::Application_ptr
       startLaunch (const Deployment::Properties & configProperty,
-		   Deployment::Connections_out providedReference,
-		   CORBA::Boolean start
+                   Deployment::Connections_out providedReference,
+                   CORBA::Boolean start
                    ACE_ENV_ARG_DECL_WITH_DEFAULTS)
-	ACE_THROW_SPEC ((
-			 CORBA::SystemException,
-			 Deployment::ResourceNotAvailable,
-			 Deployment::StartError,
-			 Deployment::InvalidProperty
-		       ));
+        ACE_THROW_SPEC ((
+                         CORBA::SystemException,
+                         Deployment::ResourceNotAvailable,
+                         Deployment::StartError,
+                         Deployment::InvalidProperty
+                       ));
 
       /**
        * @method start
@@ -65,11 +69,11 @@ namespace Deployment
        *
        */
       irtual void destroyApplication (Deployment::Application_ptr app
-				      ACE_ENV_ARG_DECL_WITH_DEFAULTS)
-	ACE_THROW_SPEC ((
-			 CORBA::SystemException,
-			 Deployment::StopError
-    		       ));
+                                      ACE_ENV_ARG_DECL_WITH_DEFAULTS)
+        ACE_THROW_SPEC ((
+                         CORBA::SystemException,
+                         Deployment::StopError
+                       ));
 
       /**
        * @name internal operations using the ACE_Map methods
@@ -82,19 +86,24 @@ namespace Deployment
 
     protected:
 
+      // @@ Tao: DO you really two locks here? Can't you use just one
+      // and synchronize access to the map. Do you need so much
+      // concurrency out here?
       typedef ACE_Hash_Map_Manager_Ex<const char *,
-	                              CORBA::Object_ptr,
+                                      CORBA::Object_ptr,
                                       ACE_Hash<const char *>,
-	                              ACE_Equal_To<const char *>,
+                                      ACE_Equal_To<const char *>,
                                       TAO_SYNCH_MUTEX> OBJ_Table;
       typedef OBJ_Table::iterator OBJ_Iterator;
 
       // CIAO's Object_Set<> could be used here as well.
       // It takes o(n) time to do find though.
+      // @@ Tao: Are you sure that Hash_Map is O(n)? :-). May be its
+      // good in algo class not for us.
       typedef ACE_Hash_Map_Manager_Ex<const char *,
-	                              ::Deployment::NodeApplication_ptr,
+                                      ::Deployment::NodeApplication_ptr,
                                       ACE_Hash<const char *>,
-	                              ACE_Equal_To<const char *>,
+                                      ACE_Equal_To<const char *>,
                                       TAO_SYNCH_MUTEX> NodeApplication_Table;
       typedef NodeApplication_Table::iterator NodeApplication_Iterator;
 
@@ -104,6 +113,8 @@ namespace Deployment
       //Launched Application(NodeApplication) but not terminated yet.
       NodeApplication_Table nodeapplication_table;
 
+      // @@ Tao: Don't you want to use a _var here for proper memory
+      // management?
       // Cached NodeManager
       NodeManager_ptr node_manager_;