From 95f20b9472b9e754207cc489246d3f51f6fb2793 Mon Sep 17 00:00:00 2001
From: qiaolongfei <qiaolongfei@baidu.com>
Date: Tue, 3 Jan 2017 14:39:44 +0800
Subject: [PATCH] add comment and refine code

---
 demo/quick_start/cluster/pserver.sh     |  2 +-
 paddle/pserver/PServerUtil.cpp          | 37 ++++++++++++-------------
 paddle/pserver/PServerUtil.h            | 37 +++++++++++++++++++++++--
 paddle/pserver/ParameterServer2Main.cpp |  3 +-
 paddle/trainer/TrainerMain.cpp          |  2 +-
 5 files changed, 56 insertions(+), 25 deletions(-)

diff --git a/demo/quick_start/cluster/pserver.sh b/demo/quick_start/cluster/pserver.sh
index 4e1ffe513..b187c1d9b 100755
--- a/demo/quick_start/cluster/pserver.sh
+++ b/demo/quick_start/cluster/pserver.sh
@@ -19,7 +19,7 @@ source "$bin_dir/env.sh"
 paddle pserver \
   --nics=`get_nics` \
   --port=7164 \
-  --ports_num=2 \
+  --ports_num=1 \
   --ports_num_for_sparse=1 \
   --num_gradient_servers=1 \
   --comment="paddle_pserver" \
diff --git a/paddle/pserver/PServerUtil.cpp b/paddle/pserver/PServerUtil.cpp
index e64569793..68a917433 100644
--- a/paddle/pserver/PServerUtil.cpp
+++ b/paddle/pserver/PServerUtil.cpp
@@ -16,30 +16,11 @@ limitations under the License. */
 
 namespace paddle {
 
-ParameterServerConfig* PServerUtil::initConfig() {
-  ParameterServerConfig* config = new ParameterServerConfig();
-  config->set_nics(FLAGS_nics);
-  config->set_port(FLAGS_port);
-  config->set_ports_num(FLAGS_ports_num);
-  config->set_rdma_tcp(FLAGS_rdma_tcp);
-  return config;
-}
-
-PServerUtil* PServerUtil::create() {
-  auto& pServerConfig = *paddle::PServerUtil::initConfig();
-  return PServerUtil::create(pServerConfig);
-}
-
-PServerUtil* PServerUtil::create(const ParameterServerConfig& config) {
-  return new PServerUtil(config);
-}
-
 PServerUtil::PServerUtil(const ParameterServerConfig& config) {
   // round robin to load balance RDMA server ENGINE
   std::vector<std::string> devices;
   int rdmaCpu = 0;
   int onlineCpus = rdma::numCpus();
-  ;
   int numPorts = config.ports_num() + config.ports_num_for_sparse();
 
   if (FLAGS_nics.empty()) {
@@ -78,6 +59,24 @@ PServerUtil::PServerUtil(const ParameterServerConfig& config) {
 
 PServerUtil::~PServerUtil() { this->join(); }
 
+ParameterServerConfig* PServerUtil::initConfig() {
+  ParameterServerConfig* config = new ParameterServerConfig();
+  config->set_nics(FLAGS_nics);
+  config->set_port(FLAGS_port);
+  config->set_ports_num(FLAGS_ports_num);
+  config->set_rdma_tcp(FLAGS_rdma_tcp);
+  return config;
+}
+
+PServerUtil* PServerUtil::createWithGflags() {
+  auto& pServerConfig = *paddle::PServerUtil::initConfig();
+  return create(pServerConfig);
+}
+
+PServerUtil* PServerUtil::create(const ParameterServerConfig& config) {
+  return new PServerUtil(config);
+}
+
 void PServerUtil::start() {
   LOG(INFO) << "pserver sizes : " << pservers_.size();
   int i = 0;
diff --git a/paddle/pserver/PServerUtil.h b/paddle/pserver/PServerUtil.h
index dd8d32a4e..117dde37e 100644
--- a/paddle/pserver/PServerUtil.h
+++ b/paddle/pserver/PServerUtil.h
@@ -24,16 +24,47 @@ namespace paddle {
 class PServerUtil {
 public:
   DISABLE_COPY(PServerUtil);
-  static PServerUtil* create();
-  static PServerUtil* create(const ParameterServerConfig& config);
+
+  /**
+   * @brief Ctor, Create a PServerUtil from ParameterServerConfig.
+   */
   explicit PServerUtil(const ParameterServerConfig& config);
+
+  /**
+   * @brief Dtor.
+   */
   ~PServerUtil();
-  static ParameterServerConfig* initConfig();
+
+  /**
+   * @brief create PServerUtil from gflags, this is used for
+   * compatibility with the old usage of configuration by gflags.
+   */
+  static PServerUtil* createWithGflags();
+
+  /**
+   * @brief create PServerUtil with ParameterServerConfig, remove gflags
+   * from ParameterServer. Init all pservers thread according to the config.
+   */
+  static PServerUtil* create(const ParameterServerConfig& config);
+
+  /**
+   * @brief start all pserver thread in this PServerUtil.
+   */
   void start();
+
+  /**
+   * @brief join and wait for all pserver thread in this PServerUtil.
+   */
   void join();
 
 private:
   std::vector<std::shared_ptr<ParameterServer2>> pservers_;
+
+  /**
+   * @brief create ParameterServerConfig from gflags, this is used for
+   * compatibility with the old usage of configuration by gflags.
+   */
+  static ParameterServerConfig* initConfig();
 };
 
 }  // namespace paddle
diff --git a/paddle/pserver/ParameterServer2Main.cpp b/paddle/pserver/ParameterServer2Main.cpp
index afba7293e..8c1baea0c 100644
--- a/paddle/pserver/ParameterServer2Main.cpp
+++ b/paddle/pserver/ParameterServer2Main.cpp
@@ -21,7 +21,8 @@ using namespace paddle;  // NOLINT
 int main(int argc, char** argv) {
   initMain(argc, argv);
 
-  std::unique_ptr<PServerUtil> pServerPtr(paddle::PServerUtil::create());
+  std::unique_ptr<PServerUtil> pServerPtr(
+      paddle::PServerUtil::createWithGflags());
   pServerPtr->start();
   pServerPtr->join();
 
diff --git a/paddle/trainer/TrainerMain.cpp b/paddle/trainer/TrainerMain.cpp
index 0d3e4514d..a690268c2 100644
--- a/paddle/trainer/TrainerMain.cpp
+++ b/paddle/trainer/TrainerMain.cpp
@@ -38,7 +38,7 @@ int main(int argc, char** argv) {
   initPython(argc, argv);
 
   if (FLAGS_start_pserver) {
-    PServerUtil* pServerUtil = paddle::PServerUtil::create();
+    PServerUtil* pServerUtil = paddle::PServerUtil::createWithGflags();
     pServerUtil->start();
   }
   Trainer trainer;
-- 
GitLab