From f3c61cbc4cc6239958ce394980a82e511e475de5 Mon Sep 17 00:00:00 2001
From: qiaolongfei <qiaolongfei@baidu.com>
Date: Tue, 3 Jan 2017 10:43:14 +0800
Subject: [PATCH 1/8] add pserver util and parameter server config

---
 demo/quick_start/cluster/cluster_train.sh |   1 +
 demo/quick_start/cluster/pserver.sh       |   2 +-
 paddle/pserver/CMakeLists.txt             |   6 +-
 paddle/pserver/PServerUtil.cpp            | 101 ++++++++++++++++++++++
 paddle/pserver/PServerUtil.h              |  39 +++++++++
 paddle/pserver/ParameterServer2Main.cpp   |  59 ++-----------
 paddle/trainer/TrainerMain.cpp            |  53 +-----------
 proto/CMakeLists.txt                      |   3 +-
 proto/ParameterServerConfig.proto         |  43 +++++++++
 9 files changed, 199 insertions(+), 108 deletions(-)
 create mode 100644 paddle/pserver/PServerUtil.cpp
 create mode 100644 paddle/pserver/PServerUtil.h
 create mode 100644 proto/ParameterServerConfig.proto

diff --git a/demo/quick_start/cluster/cluster_train.sh b/demo/quick_start/cluster/cluster_train.sh
index aac9b89b14b98..a7b1f01064b29 100755
--- a/demo/quick_start/cluster/cluster_train.sh
+++ b/demo/quick_start/cluster/cluster_train.sh
@@ -25,6 +25,7 @@ log_file="$bin_dir/train.log"
 pushd "$home_dir"
 cfg=trainer_config.lr.py
 paddle train \
+  --start_pserver=false \
   --config=$cfg \
   --save_dir=${model_dir} \
   --trainer_count=4 \
diff --git a/demo/quick_start/cluster/pserver.sh b/demo/quick_start/cluster/pserver.sh
index b187c1d9b9108..4e1ffe5139e27 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=1 \
+  --ports_num=2 \
   --ports_num_for_sparse=1 \
   --num_gradient_servers=1 \
   --comment="paddle_pserver" \
diff --git a/paddle/pserver/CMakeLists.txt b/paddle/pserver/CMakeLists.txt
index 1c1e1964b8d3f..9bc48294f06b8 100644
--- a/paddle/pserver/CMakeLists.txt
+++ b/paddle/pserver/CMakeLists.txt
@@ -24,13 +24,15 @@ set(PSERVER_SOURCES
     BaseClient.cpp
     ParameterClient2.cpp
     ParameterServer2.cpp
-    SparseParameterDistribution.cpp)
+    SparseParameterDistribution.cpp
+    PServerUtil.cpp)
 
 set(PSERVER_HEADERS
     BaseClient.h
     ParameterClient2.h
     ParameterServer2.h
-    SparseParameterDistribution.h)
+    SparseParameterDistribution.h
+    PServerUtil.h)
 
 add_library(paddle_pserver STATIC
     ${PSERVER_SOURCES})
diff --git a/paddle/pserver/PServerUtil.cpp b/paddle/pserver/PServerUtil.cpp
new file mode 100644
index 0000000000000..e64569793613c
--- /dev/null
+++ b/paddle/pserver/PServerUtil.cpp
@@ -0,0 +1,101 @@
+/* Copyright (c) 2016 PaddlePaddle Authors. All Rights Reserve.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License. */
+
+#include "PServerUtil.h"
+
+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()) {
+    pservers_.resize(numPorts);
+    for (int i = 0; i < numPorts; ++i) {
+      if (FLAGS_rdma_tcp == "rdma") {
+        pservers_[i].reset(
+            new ParameterServer2(std::string(), FLAGS_port + i, rdmaCpu++));
+        rdmaCpu = rdmaCpu % onlineCpus;
+      } else {
+        pservers_[i].reset(new ParameterServer2(std::string(), FLAGS_port + i));
+      }
+      CHECK(pservers_[i]->init()) << "Fail to initialize parameter server"
+                                  << FLAGS_port + i;
+    }
+  } else {
+    str::split(FLAGS_nics, ',', &devices);
+    pservers_.resize(devices.size() * numPorts);
+    for (int i = 0; i < numPorts; ++i) {
+      for (size_t j = 0; j < devices.size(); ++j) {
+        if (FLAGS_rdma_tcp == "rdma") {
+          pservers_[i * devices.size() + j].reset(new ParameterServer2(
+              getIpAddr(devices[j]), FLAGS_port + i, rdmaCpu++));
+          rdmaCpu = rdmaCpu % onlineCpus;
+        } else {
+          pservers_[i * devices.size() + j].reset(
+              new ParameterServer2(getIpAddr(devices[j]), FLAGS_port + i));
+        }
+        CHECK(pservers_[i * devices.size() + j]->init())
+            << "Fail to initialize parameter server" << devices[j]
+            << FLAGS_port + i;
+      }
+    }
+  }
+}
+
+PServerUtil::~PServerUtil() { this->join(); }
+
+void PServerUtil::start() {
+  LOG(INFO) << "pserver sizes : " << pservers_.size();
+  int i = 0;
+  for (const auto& pserver : pservers_) {
+    LOG(INFO) << "pserver started : " << i;
+    pserver->start();
+    i++;
+  }
+}
+
+void PServerUtil::join() {
+  LOG(INFO) << "pserver sizes : " << pservers_.size();
+  int i = 0;
+  for (const auto& pserver : pservers_) {
+    LOG(INFO) << "pserver join : " << i;
+    pserver->join();
+    i++;
+  }
+}
+
+}  // namespace paddle
diff --git a/paddle/pserver/PServerUtil.h b/paddle/pserver/PServerUtil.h
new file mode 100644
index 0000000000000..dd8d32a4e9bc6
--- /dev/null
+++ b/paddle/pserver/PServerUtil.h
@@ -0,0 +1,39 @@
+/* Copyright (c) 2016 PaddlePaddle Authors. All Rights Reserve.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License. */
+
+#pragma once
+
+#include "ParameterServer2.h"
+#include "ParameterServerConfig.pb.h"
+#include "RDMANetwork.h"
+#include "paddle/utils/StringUtil.h"
+
+namespace paddle {
+
+class PServerUtil {
+public:
+  DISABLE_COPY(PServerUtil);
+  static PServerUtil* create();
+  static PServerUtil* create(const ParameterServerConfig& config);
+  explicit PServerUtil(const ParameterServerConfig& config);
+  ~PServerUtil();
+  static ParameterServerConfig* initConfig();
+  void start();
+  void join();
+
+private:
+  std::vector<std::shared_ptr<ParameterServer2>> pservers_;
+};
+
+}  // namespace paddle
diff --git a/paddle/pserver/ParameterServer2Main.cpp b/paddle/pserver/ParameterServer2Main.cpp
index ffc521f2c143d..afba7293eb8c9 100644
--- a/paddle/pserver/ParameterServer2Main.cpp
+++ b/paddle/pserver/ParameterServer2Main.cpp
@@ -13,66 +13,17 @@ See the License for the specific language governing permissions and
 limitations under the License. */
 
 #include <fstream>
-#include "paddle/utils/StringUtil.h"
-#include "paddle/utils/Util.h"
-
-#include "ParameterServer2.h"
-#include "RDMANetwork.h"
-#include "paddle/utils/Flags.h"
+#include "PServerUtil.h"
+#include "paddle/trainer/ParamUtil.h"
 
 using namespace paddle;  // NOLINT
 
 int main(int argc, char** argv) {
   initMain(argc, argv);
 
-  std::vector<std::string> devices;
-  std::vector<std::shared_ptr<ParameterServer2>> pservers;
-
-  // round robin to loadbalance RDMA server ENGINE
-  int rdmaCpu = 0;
-  int onlineCpus = rdma::numCpus();
-  int numPorts = FLAGS_ports_num + FLAGS_ports_num_for_sparse;
-  if (FLAGS_nics.empty()) {
-    pservers.resize(numPorts);
-    for (int i = 0; i < numPorts; ++i) {
-      if (FLAGS_rdma_tcp == "rdma") {
-        pservers[i].reset(
-            new ParameterServer2(std::string(), FLAGS_port + i, rdmaCpu++));
-        rdmaCpu = rdmaCpu % onlineCpus;
-      } else {
-        pservers[i].reset(new ParameterServer2(std::string(), FLAGS_port + i));
-      }
-      CHECK(pservers[i]->init()) << "Fail to initialize parameter server"
-                                 << FLAGS_port + i;
-      LOG(INFO) << "pserver started : " << FLAGS_port + i;
-      pservers[i]->start();
-    }
-  } else {
-    str::split(FLAGS_nics, ',', &devices);
-    pservers.resize(devices.size() * numPorts);
-    for (int i = 0; i < numPorts; ++i) {
-      for (size_t j = 0; j < devices.size(); ++j) {
-        if (FLAGS_rdma_tcp == "rdma") {
-          pservers[i * devices.size() + j].reset(new ParameterServer2(
-              getIpAddr(devices[j]), FLAGS_port + i, rdmaCpu++));
-          rdmaCpu = rdmaCpu % onlineCpus;
-        } else {
-          pservers[i * devices.size() + j].reset(
-              new ParameterServer2(getIpAddr(devices[j]), FLAGS_port + i));
-        }
-        CHECK(pservers[i * devices.size() + j]->init())
-            << "Fail to initialize parameter server" << devices[j]
-            << FLAGS_port + i;
-        LOG(INFO) << "pserver started : " << devices[j] << ":"
-                  << FLAGS_port + i;
-        pservers[i * devices.size() + j]->start();
-      }
-    }
-  }
-
-  for (auto& pserver : pservers) {
-    pserver->join();
-  }
+  std::unique_ptr<PServerUtil> pServerPtr(paddle::PServerUtil::create());
+  pServerPtr->start();
+  pServerPtr->join();
 
   return 0;
 }
diff --git a/paddle/trainer/TrainerMain.cpp b/paddle/trainer/TrainerMain.cpp
index 947f9cadcc983..0d3e4514d6b00 100644
--- a/paddle/trainer/TrainerMain.cpp
+++ b/paddle/trainer/TrainerMain.cpp
@@ -13,14 +13,12 @@ See the License for the specific language governing permissions and
 limitations under the License. */
 
 #include <fenv.h>
-#include "paddle/pserver/ParameterServer2.h"
+#include "paddle/pserver/PServerUtil.h"
 #include "paddle/utils/Excepts.h"
 #include "paddle/utils/PythonUtil.h"
-#include "paddle/utils/StringUtil.h"
 
 #include "ParamUtil.h"
 #include "Trainer.h"
-#include "paddle/pserver/RDMANetwork.h"
 
 DEFINE_bool(start_pserver, false, "Whether to start pserver");
 DECLARE_int32(gpu_id);
@@ -39,54 +37,9 @@ int main(int argc, char** argv) {
   initMain(argc, argv);
   initPython(argc, argv);
 
-  std::vector<std::unique_ptr<ParameterServer2>> pservers;
-  std::vector<std::string> devices;
-
   if (FLAGS_start_pserver) {
-    // round robin to loadbalance RDMA server ENGINE
-    int rdmaCpu = 0;
-    int onlineCpus = rdma::numCpus();
-    int numPorts = FLAGS_ports_num + FLAGS_ports_num_for_sparse;
-    if (FLAGS_nics.empty()) {
-      pservers.resize(numPorts);
-      for (int i = 0; i < numPorts; ++i) {
-        if (FLAGS_rdma_tcp == "rdma") {
-          pservers[i].reset(
-              new ParameterServer2(std::string(), FLAGS_port + i, rdmaCpu++));
-          rdmaCpu = rdmaCpu % onlineCpus;
-        } else {
-          pservers[i].reset(
-              new ParameterServer2(std::string(), FLAGS_port + i));
-        }
-
-        CHECK(pservers[i]->init()) << "Fail to initialize parameter server"
-                                   << FLAGS_port + i;
-        LOG(INFO) << "pserver started : " << FLAGS_port + i;
-        pservers[i]->start();
-      }
-    } else {
-      str::split(FLAGS_nics, ',', &devices);
-      pservers.resize(devices.size() * numPorts);
-      for (int i = 0; i < numPorts; ++i) {
-        for (size_t j = 0; j < devices.size(); ++j) {
-          if (FLAGS_rdma_tcp == "rdma") {
-            pservers[i * devices.size() + j].reset(new ParameterServer2(
-                getIpAddr(devices[j]), FLAGS_port + i, rdmaCpu++));
-            rdmaCpu = rdmaCpu % onlineCpus;
-          } else {
-            pservers[i * devices.size() + j].reset(
-                new ParameterServer2(getIpAddr(devices[j]), FLAGS_port + i));
-          }
-
-          CHECK(pservers[i * devices.size() + j]->init())
-              << "Fail to initialize parameter server" << devices[j]
-              << FLAGS_port + i;
-          LOG(INFO) << "pserver started : " << devices[j] << ":"
-                    << FLAGS_port + i;
-          pservers[i * devices.size() + j]->start();
-        }
-      }
-    }
+    PServerUtil* pServerUtil = paddle::PServerUtil::create();
+    pServerUtil->start();
   }
   Trainer trainer;
   auto config = TrainerConfigHelper::createFromFlags();
diff --git a/proto/CMakeLists.txt b/proto/CMakeLists.txt
index 2c40070eca44d..e53d06e773aeb 100644
--- a/proto/CMakeLists.txt
+++ b/proto/CMakeLists.txt
@@ -4,7 +4,8 @@ set(proto_filenames
     ModelConfig.proto
     ParameterConfig.proto
     ParameterService.proto
-    TrainerConfig.proto)
+    TrainerConfig.proto
+    ParameterServerConfig.proto)
 
 set(PROTO_GEN)
 set(PROTO_GEN_PY)
diff --git a/proto/ParameterServerConfig.proto b/proto/ParameterServerConfig.proto
new file mode 100644
index 0000000000000..b4fbf901c20cc
--- /dev/null
+++ b/proto/ParameterServerConfig.proto
@@ -0,0 +1,43 @@
+/* Copyright (c) 2016 PaddlePaddle Authors. All Rights Reserve.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License. */
+syntax = "proto2";
+
+package paddle;
+
+message ParameterClientConfig {
+  required int32 trainer_id = 1;
+}
+
+message ParameterServerConfig {
+  // The ports number for parameter send,
+  // increment based on default port number
+  required int32 ports_num = 1 [default = 1];
+  // The ports number for parameter send,
+  // increment based on default (port + ports_num
+  required int32 ports_num_for_sparse = 2 [default = 0];
+  // network device name for pservers
+  required string nics = 3 [default = "xgbe0,xgbe1"];
+  required string rdma_tcp = 4 [default = "tcp"];
+  // Listening port for pserver
+  required int32 port = 5 [default = 20134];
+  // number of gradient servers
+  required int32 num_gradient_servers = 6 [default = 1];
+  // number of threads for sync op exec
+  required int32 pserver_num_threads = 7 [default = 1];
+  // control config_.async_lagged_grad_discard_ratio() min value
+  required double async_lagged_ratio_min = 8 [default = 1.0];
+  // if async_lagged_grad_discard_ratio is not set in trainer_config.conf
+  // use it as defalut value
+  required double async_lagged_ratio_default = 9 [default = 1.5];
+}
\ No newline at end of file

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 2/8] 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 4e1ffe5139e27..b187c1d9b9108 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 e64569793613c..68a9174330626 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 dd8d32a4e9bc6..117dde37e3f88 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 afba7293eb8c9..8c1baea0cef76 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 0d3e4514d6b00..a690268c2c7ab 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;

From cfbb4c481e0b2a59f335ae3f34d2aa8dba39e26d Mon Sep 17 00:00:00 2001
From: qiaolongfei <qiaolongfei@baidu.com>
Date: Tue, 3 Jan 2017 15:34:50 +0800
Subject: [PATCH 3/8] use unique_ptr

---
 paddle/trainer/TrainerMain.cpp | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/paddle/trainer/TrainerMain.cpp b/paddle/trainer/TrainerMain.cpp
index a690268c2c7ab..52983e46eb907 100644
--- a/paddle/trainer/TrainerMain.cpp
+++ b/paddle/trainer/TrainerMain.cpp
@@ -37,9 +37,10 @@ int main(int argc, char** argv) {
   initMain(argc, argv);
   initPython(argc, argv);
 
+  std::unique_ptr<PServerUtil> pServerPtr(nullptr);
   if (FLAGS_start_pserver) {
-    PServerUtil* pServerUtil = paddle::PServerUtil::createWithGflags();
-    pServerUtil->start();
+    pServerPtr.reset(paddle::PServerUtil::createWithGflags());
+    pServerPtr->start();
   }
   Trainer trainer;
   auto config = TrainerConfigHelper::createFromFlags();

From 77839826a4030efd5f692c4e6ab2cf8cc011a363 Mon Sep 17 00:00:00 2001
From: qiaolongfei <qiaolongfei@baidu.com>
Date: Tue, 3 Jan 2017 17:09:59 +0800
Subject: [PATCH 4/8] change FLAGS to proto config in PServerUtil

---
 paddle/pserver/PServerUtil.cpp | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/paddle/pserver/PServerUtil.cpp b/paddle/pserver/PServerUtil.cpp
index 68a9174330626..bf4cf0771ccd6 100644
--- a/paddle/pserver/PServerUtil.cpp
+++ b/paddle/pserver/PServerUtil.cpp
@@ -23,35 +23,36 @@ PServerUtil::PServerUtil(const ParameterServerConfig& config) {
   int onlineCpus = rdma::numCpus();
   int numPorts = config.ports_num() + config.ports_num_for_sparse();
 
-  if (FLAGS_nics.empty()) {
+  if (config.nics().empty()) {
     pservers_.resize(numPorts);
     for (int i = 0; i < numPorts; ++i) {
-      if (FLAGS_rdma_tcp == "rdma") {
+      if (config.rdma_tcp() == "rdma") {
         pservers_[i].reset(
-            new ParameterServer2(std::string(), FLAGS_port + i, rdmaCpu++));
+            new ParameterServer2(std::string(), config.port() + i, rdmaCpu++));
         rdmaCpu = rdmaCpu % onlineCpus;
       } else {
-        pservers_[i].reset(new ParameterServer2(std::string(), FLAGS_port + i));
+        pservers_[i].reset(
+            new ParameterServer2(std::string(), config.port() + i));
       }
       CHECK(pservers_[i]->init()) << "Fail to initialize parameter server"
-                                  << FLAGS_port + i;
+                                  << config.port() + i;
     }
   } else {
-    str::split(FLAGS_nics, ',', &devices);
+    str::split(config.nics(), ',', &devices);
     pservers_.resize(devices.size() * numPorts);
     for (int i = 0; i < numPorts; ++i) {
       for (size_t j = 0; j < devices.size(); ++j) {
-        if (FLAGS_rdma_tcp == "rdma") {
+        if (config.rdma_tcp() == "rdma") {
           pservers_[i * devices.size() + j].reset(new ParameterServer2(
-              getIpAddr(devices[j]), FLAGS_port + i, rdmaCpu++));
+              getIpAddr(devices[j]), config.port() + i, rdmaCpu++));
           rdmaCpu = rdmaCpu % onlineCpus;
         } else {
           pservers_[i * devices.size() + j].reset(
-              new ParameterServer2(getIpAddr(devices[j]), FLAGS_port + i));
+              new ParameterServer2(getIpAddr(devices[j]), config.port() + i));
         }
         CHECK(pservers_[i * devices.size() + j]->init())
             << "Fail to initialize parameter server" << devices[j]
-            << FLAGS_port + i;
+            << config.port() + i;
       }
     }
   }

From 93e74f89c6b91db1d57014c2b7b7ac4f59486121 Mon Sep 17 00:00:00 2001
From: qiaolongfei <qiaolongfei@baidu.com>
Date: Wed, 4 Jan 2017 10:17:07 +0800
Subject: [PATCH 5/8] rename PServerUtil to PServerController

---
 paddle/pserver/CMakeLists.txt                 |  4 ++--
 ...{PServerUtil.cpp => PServerController.cpp} | 21 ++++++++++---------
 .../{PServerUtil.h => PServerController.h}    | 14 ++++++-------
 paddle/pserver/ParameterServer2Main.cpp       |  7 +++----
 paddle/trainer/TrainerMain.cpp                |  6 +++---
 5 files changed, 26 insertions(+), 26 deletions(-)
 rename paddle/pserver/{PServerUtil.cpp => PServerController.cpp} (83%)
 rename paddle/pserver/{PServerUtil.h => PServerController.h} (83%)

diff --git a/paddle/pserver/CMakeLists.txt b/paddle/pserver/CMakeLists.txt
index 9bc48294f06b8..ac52b8dbeced7 100644
--- a/paddle/pserver/CMakeLists.txt
+++ b/paddle/pserver/CMakeLists.txt
@@ -25,14 +25,14 @@ set(PSERVER_SOURCES
     ParameterClient2.cpp
     ParameterServer2.cpp
     SparseParameterDistribution.cpp
-    PServerUtil.cpp)
+    PServerController.cpp)
 
 set(PSERVER_HEADERS
     BaseClient.h
     ParameterClient2.h
     ParameterServer2.h
     SparseParameterDistribution.h
-    PServerUtil.h)
+    PServerController.h)
 
 add_library(paddle_pserver STATIC
     ${PSERVER_SOURCES})
diff --git a/paddle/pserver/PServerUtil.cpp b/paddle/pserver/PServerController.cpp
similarity index 83%
rename from paddle/pserver/PServerUtil.cpp
rename to paddle/pserver/PServerController.cpp
index bf4cf0771ccd6..2d00019ceff6d 100644
--- a/paddle/pserver/PServerUtil.cpp
+++ b/paddle/pserver/PServerController.cpp
@@ -12,11 +12,11 @@ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 See the License for the specific language governing permissions and
 limitations under the License. */
 
-#include "PServerUtil.h"
+#include "PServerController.h"
 
 namespace paddle {
 
-PServerUtil::PServerUtil(const ParameterServerConfig& config) {
+PServerController::PServerController(const ParameterServerConfig& config) {
   // round robin to load balance RDMA server ENGINE
   std::vector<std::string> devices;
   int rdmaCpu = 0;
@@ -58,9 +58,9 @@ PServerUtil::PServerUtil(const ParameterServerConfig& config) {
   }
 }
 
-PServerUtil::~PServerUtil() { this->join(); }
+PServerController::~PServerController() { this->join(); }
 
-ParameterServerConfig* PServerUtil::initConfig() {
+ParameterServerConfig* PServerController::initConfigByGflags() {
   ParameterServerConfig* config = new ParameterServerConfig();
   config->set_nics(FLAGS_nics);
   config->set_port(FLAGS_port);
@@ -69,16 +69,17 @@ ParameterServerConfig* PServerUtil::initConfig() {
   return config;
 }
 
-PServerUtil* PServerUtil::createWithGflags() {
-  auto& pServerConfig = *paddle::PServerUtil::initConfig();
+PServerController* PServerController::createByGflags() {
+  auto& pServerConfig = *paddle::PServerController::initConfigByGflags();
   return create(pServerConfig);
 }
 
-PServerUtil* PServerUtil::create(const ParameterServerConfig& config) {
-  return new PServerUtil(config);
+PServerController* PServerController::create(
+    const ParameterServerConfig& config) {
+  return new PServerController(config);
 }
 
-void PServerUtil::start() {
+void PServerController::start() {
   LOG(INFO) << "pserver sizes : " << pservers_.size();
   int i = 0;
   for (const auto& pserver : pservers_) {
@@ -88,7 +89,7 @@ void PServerUtil::start() {
   }
 }
 
-void PServerUtil::join() {
+void PServerController::join() {
   LOG(INFO) << "pserver sizes : " << pservers_.size();
   int i = 0;
   for (const auto& pserver : pservers_) {
diff --git a/paddle/pserver/PServerUtil.h b/paddle/pserver/PServerController.h
similarity index 83%
rename from paddle/pserver/PServerUtil.h
rename to paddle/pserver/PServerController.h
index 117dde37e3f88..6fb7e0a31ab22 100644
--- a/paddle/pserver/PServerUtil.h
+++ b/paddle/pserver/PServerController.h
@@ -21,31 +21,31 @@ limitations under the License. */
 
 namespace paddle {
 
-class PServerUtil {
+class PServerController {
 public:
-  DISABLE_COPY(PServerUtil);
+  DISABLE_COPY(PServerController);
 
   /**
    * @brief Ctor, Create a PServerUtil from ParameterServerConfig.
    */
-  explicit PServerUtil(const ParameterServerConfig& config);
+  explicit PServerController(const ParameterServerConfig& config);
 
   /**
    * @brief Dtor.
    */
-  ~PServerUtil();
+  ~PServerController();
 
   /**
    * @brief create PServerUtil from gflags, this is used for
    * compatibility with the old usage of configuration by gflags.
    */
-  static PServerUtil* createWithGflags();
+  static PServerController* createByGflags();
 
   /**
    * @brief create PServerUtil with ParameterServerConfig, remove gflags
    * from ParameterServer. Init all pservers thread according to the config.
    */
-  static PServerUtil* create(const ParameterServerConfig& config);
+  static PServerController* create(const ParameterServerConfig& config);
 
   /**
    * @brief start all pserver thread in this PServerUtil.
@@ -64,7 +64,7 @@ class PServerUtil {
    * @brief create ParameterServerConfig from gflags, this is used for
    * compatibility with the old usage of configuration by gflags.
    */
-  static ParameterServerConfig* initConfig();
+  static ParameterServerConfig* initConfigByGflags();
 };
 
 }  // namespace paddle
diff --git a/paddle/pserver/ParameterServer2Main.cpp b/paddle/pserver/ParameterServer2Main.cpp
index 8c1baea0cef76..6e683cdd2c2c8 100644
--- a/paddle/pserver/ParameterServer2Main.cpp
+++ b/paddle/pserver/ParameterServer2Main.cpp
@@ -13,16 +13,15 @@ See the License for the specific language governing permissions and
 limitations under the License. */
 
 #include <fstream>
-#include "PServerUtil.h"
-#include "paddle/trainer/ParamUtil.h"
+#include "PServerController.h"
 
 using namespace paddle;  // NOLINT
 
 int main(int argc, char** argv) {
   initMain(argc, argv);
 
-  std::unique_ptr<PServerUtil> pServerPtr(
-      paddle::PServerUtil::createWithGflags());
+  std::unique_ptr<PServerController> pServerPtr(
+      paddle::PServerController::createByGflags());
   pServerPtr->start();
   pServerPtr->join();
 
diff --git a/paddle/trainer/TrainerMain.cpp b/paddle/trainer/TrainerMain.cpp
index 52983e46eb907..3ce3d67842dda 100644
--- a/paddle/trainer/TrainerMain.cpp
+++ b/paddle/trainer/TrainerMain.cpp
@@ -13,7 +13,7 @@ See the License for the specific language governing permissions and
 limitations under the License. */
 
 #include <fenv.h>
-#include "paddle/pserver/PServerUtil.h"
+#include "paddle/pserver/PServerController.h"
 #include "paddle/utils/Excepts.h"
 #include "paddle/utils/PythonUtil.h"
 
@@ -37,9 +37,9 @@ int main(int argc, char** argv) {
   initMain(argc, argv);
   initPython(argc, argv);
 
-  std::unique_ptr<PServerUtil> pServerPtr(nullptr);
+  std::unique_ptr<PServerController> pServerPtr(nullptr);
   if (FLAGS_start_pserver) {
-    pServerPtr.reset(paddle::PServerUtil::createWithGflags());
+    pServerPtr.reset(paddle::PServerController::createByGflags());
     pServerPtr->start();
   }
   Trainer trainer;

From 3f6c2b3621f4ec7fdf051f7c8e21faff31e2881d Mon Sep 17 00:00:00 2001
From: qiaolongfei <qiaolongfei@baidu.com>
Date: Wed, 4 Jan 2017 11:25:28 +0800
Subject: [PATCH 6/8] rm initConfigByGflags of PServerController, use stack
 value instead

---
 paddle/pserver/PServerController.cpp | 20 +++++++++-----------
 paddle/pserver/PServerController.h   | 10 ++--------
 2 files changed, 11 insertions(+), 19 deletions(-)

diff --git a/paddle/pserver/PServerController.cpp b/paddle/pserver/PServerController.cpp
index 2d00019ceff6d..8d2e026bca79d 100644
--- a/paddle/pserver/PServerController.cpp
+++ b/paddle/pserver/PServerController.cpp
@@ -60,18 +60,16 @@ PServerController::PServerController(const ParameterServerConfig& config) {
 
 PServerController::~PServerController() { this->join(); }
 
-ParameterServerConfig* PServerController::initConfigByGflags() {
-  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;
-}
-
 PServerController* PServerController::createByGflags() {
-  auto& pServerConfig = *paddle::PServerController::initConfigByGflags();
-  return create(pServerConfig);
+  ParameterServerConfig config;
+
+  config.set_nics(FLAGS_nics);
+  config.set_rdma_tcp(FLAGS_rdma_tcp);
+  config.set_port(FLAGS_port);
+  config.set_ports_num(FLAGS_ports_num);
+  config.set_ports_num_for_sparse(FLAGS_ports_num_for_sparse);
+
+  return create(config);
 }
 
 PServerController* PServerController::create(
diff --git a/paddle/pserver/PServerController.h b/paddle/pserver/PServerController.h
index 6fb7e0a31ab22..cecf7290094a2 100644
--- a/paddle/pserver/PServerController.h
+++ b/paddle/pserver/PServerController.h
@@ -21,7 +21,7 @@ limitations under the License. */
 
 namespace paddle {
 
-class PServerController {
+class PServerController final {
 public:
   DISABLE_COPY(PServerController);
 
@@ -58,13 +58,7 @@ class PServerController {
   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* initConfigByGflags();
+  std::vector<std::unique_ptr<ParameterServer2>> pservers_;
 };
 
 }  // namespace paddle

From 5aaaef446818e52106f652d8b070c803cbbcce20 Mon Sep 17 00:00:00 2001
From: qiaolongfei <qiaolongfei@baidu.com>
Date: Fri, 6 Jan 2017 00:57:21 +0800
Subject: [PATCH 7/8] rename PServerController to ParameterServerController

---
 paddle/pserver/CMakeLists.txt                 |  4 +--
 paddle/pserver/ParameterServer2Main.cpp       |  6 ++---
 ...ller.cpp => ParameterServerController.cpp} | 17 ++++++------
 ...ntroller.h => ParameterServerController.h} | 26 ++++++++++---------
 paddle/trainer/TrainerMain.cpp                |  6 ++---
 5 files changed, 31 insertions(+), 28 deletions(-)
 rename paddle/pserver/{PServerController.cpp => ParameterServerController.cpp} (85%)
 rename paddle/pserver/{PServerController.h => ParameterServerController.h} (56%)

diff --git a/paddle/pserver/CMakeLists.txt b/paddle/pserver/CMakeLists.txt
index ac52b8dbeced7..b7f85ea1a6dfd 100644
--- a/paddle/pserver/CMakeLists.txt
+++ b/paddle/pserver/CMakeLists.txt
@@ -25,14 +25,14 @@ set(PSERVER_SOURCES
     ParameterClient2.cpp
     ParameterServer2.cpp
     SparseParameterDistribution.cpp
-    PServerController.cpp)
+    ParameterServerController.cpp)
 
 set(PSERVER_HEADERS
     BaseClient.h
     ParameterClient2.h
     ParameterServer2.h
     SparseParameterDistribution.h
-    PServerController.h)
+    ParameterServerController.h)
 
 add_library(paddle_pserver STATIC
     ${PSERVER_SOURCES})
diff --git a/paddle/pserver/ParameterServer2Main.cpp b/paddle/pserver/ParameterServer2Main.cpp
index 6e683cdd2c2c8..114505252211d 100644
--- a/paddle/pserver/ParameterServer2Main.cpp
+++ b/paddle/pserver/ParameterServer2Main.cpp
@@ -13,15 +13,15 @@ See the License for the specific language governing permissions and
 limitations under the License. */
 
 #include <fstream>
-#include "PServerController.h"
+#include "ParameterServerController.h"
 
 using namespace paddle;  // NOLINT
 
 int main(int argc, char** argv) {
   initMain(argc, argv);
 
-  std::unique_ptr<PServerController> pServerPtr(
-      paddle::PServerController::createByGflags());
+  std::unique_ptr<ParameterServerController> pServerPtr(
+      paddle::ParameterServerController::createByGflags());
   pServerPtr->start();
   pServerPtr->join();
 
diff --git a/paddle/pserver/PServerController.cpp b/paddle/pserver/ParameterServerController.cpp
similarity index 85%
rename from paddle/pserver/PServerController.cpp
rename to paddle/pserver/ParameterServerController.cpp
index 8d2e026bca79d..ec24bc7e573dc 100644
--- a/paddle/pserver/PServerController.cpp
+++ b/paddle/pserver/ParameterServerController.cpp
@@ -12,11 +12,12 @@ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 See the License for the specific language governing permissions and
 limitations under the License. */
 
-#include "PServerController.h"
+#include "ParameterServerController.h"
 
 namespace paddle {
 
-PServerController::PServerController(const ParameterServerConfig& config) {
+ParameterServerController::ParameterServerController(
+    const ParameterServerConfig& config) {
   // round robin to load balance RDMA server ENGINE
   std::vector<std::string> devices;
   int rdmaCpu = 0;
@@ -58,9 +59,9 @@ PServerController::PServerController(const ParameterServerConfig& config) {
   }
 }
 
-PServerController::~PServerController() { this->join(); }
+ParameterServerController::~ParameterServerController() { this->join(); }
 
-PServerController* PServerController::createByGflags() {
+ParameterServerController* ParameterServerController::createByGflags() {
   ParameterServerConfig config;
 
   config.set_nics(FLAGS_nics);
@@ -72,12 +73,12 @@ PServerController* PServerController::createByGflags() {
   return create(config);
 }
 
-PServerController* PServerController::create(
+ParameterServerController* ParameterServerController::create(
     const ParameterServerConfig& config) {
-  return new PServerController(config);
+  return new ParameterServerController(config);
 }
 
-void PServerController::start() {
+void ParameterServerController::start() {
   LOG(INFO) << "pserver sizes : " << pservers_.size();
   int i = 0;
   for (const auto& pserver : pservers_) {
@@ -87,7 +88,7 @@ void PServerController::start() {
   }
 }
 
-void PServerController::join() {
+void ParameterServerController::join() {
   LOG(INFO) << "pserver sizes : " << pservers_.size();
   int i = 0;
   for (const auto& pserver : pservers_) {
diff --git a/paddle/pserver/PServerController.h b/paddle/pserver/ParameterServerController.h
similarity index 56%
rename from paddle/pserver/PServerController.h
rename to paddle/pserver/ParameterServerController.h
index cecf7290094a2..ee249de9d802d 100644
--- a/paddle/pserver/PServerController.h
+++ b/paddle/pserver/ParameterServerController.h
@@ -21,39 +21,41 @@ limitations under the License. */
 
 namespace paddle {
 
-class PServerController final {
+class ParameterServerController final {
 public:
-  DISABLE_COPY(PServerController);
+  DISABLE_COPY(ParameterServerController);
 
   /**
-   * @brief Ctor, Create a PServerUtil from ParameterServerConfig.
+   * @brief Ctor, Create a ParameterServerController from ParameterServerConfig.
    */
-  explicit PServerController(const ParameterServerConfig& config);
+  explicit ParameterServerController(const ParameterServerConfig& config);
 
   /**
    * @brief Dtor.
    */
-  ~PServerController();
+  ~ParameterServerController();
 
   /**
-   * @brief create PServerUtil from gflags, this is used for
+   * @brief create ParameterServerController from gflags, this is used for
    * compatibility with the old usage of configuration by gflags.
    */
-  static PServerController* createByGflags();
+  static ParameterServerController* createByGflags();
 
   /**
-   * @brief create PServerUtil with ParameterServerConfig, remove gflags
-   * from ParameterServer. Init all pservers thread according to the config.
+   * @brief create ParameterServerController with ParameterServerConfig, remove
+   * gflags from ParameterServer. Init all pservers thread according to the
+   * config.
    */
-  static PServerController* create(const ParameterServerConfig& config);
+  static ParameterServerController* create(const ParameterServerConfig& config);
 
   /**
-   * @brief start all pserver thread in this PServerUtil.
+   * @brief start all pserver thread in this ParameterServerController.
    */
   void start();
 
   /**
-   * @brief join and wait for all pserver thread in this PServerUtil.
+   * @brief join and wait for all pserver thread in this
+   * ParameterServerController.
    */
   void join();
 
diff --git a/paddle/trainer/TrainerMain.cpp b/paddle/trainer/TrainerMain.cpp
index 3ce3d67842dda..ccf67f96fa8f5 100644
--- a/paddle/trainer/TrainerMain.cpp
+++ b/paddle/trainer/TrainerMain.cpp
@@ -13,7 +13,7 @@ See the License for the specific language governing permissions and
 limitations under the License. */
 
 #include <fenv.h>
-#include "paddle/pserver/PServerController.h"
+#include "paddle/pserver/ParameterServerController.h"
 #include "paddle/utils/Excepts.h"
 #include "paddle/utils/PythonUtil.h"
 
@@ -37,9 +37,9 @@ int main(int argc, char** argv) {
   initMain(argc, argv);
   initPython(argc, argv);
 
-  std::unique_ptr<PServerController> pServerPtr(nullptr);
+  std::unique_ptr<ParameterServerController> pServerPtr(nullptr);
   if (FLAGS_start_pserver) {
-    pServerPtr.reset(paddle::PServerController::createByGflags());
+    pServerPtr.reset(paddle::ParameterServerController::createByGflags());
     pServerPtr->start();
   }
   Trainer trainer;

From aa9f5162605d317caa72dcc4cfa1c08fc8faee55 Mon Sep 17 00:00:00 2001
From: qiaolongfei <qiaolongfei@baidu.com>
Date: Wed, 11 Jan 2017 12:59:23 +0800
Subject: [PATCH 8/8] code refine, add comment and some naming problem

---
 paddle/pserver/ParameterServer2Main.cpp      |  8 ++--
 paddle/pserver/ParameterServerController.cpp | 43 ++++++++++----------
 paddle/pserver/ParameterServerController.h   | 22 ++++++----
 paddle/trainer/TrainerMain.cpp               |  7 ++--
 proto/ParameterServerConfig.proto            |  7 ++++
 5 files changed, 52 insertions(+), 35 deletions(-)

diff --git a/paddle/pserver/ParameterServer2Main.cpp b/paddle/pserver/ParameterServer2Main.cpp
index 114505252211d..845a2c27e242c 100644
--- a/paddle/pserver/ParameterServer2Main.cpp
+++ b/paddle/pserver/ParameterServer2Main.cpp
@@ -20,10 +20,10 @@ using namespace paddle;  // NOLINT
 int main(int argc, char** argv) {
   initMain(argc, argv);
 
-  std::unique_ptr<ParameterServerController> pServerPtr(
-      paddle::ParameterServerController::createByGflags());
-  pServerPtr->start();
-  pServerPtr->join();
+  std::unique_ptr<ParameterServerController> parameterServerPtr(
+      paddle::ParameterServerController::createFromGflags());
+  parameterServerPtr->start();
+  parameterServerPtr->wait();
 
   return 0;
 }
diff --git a/paddle/pserver/ParameterServerController.cpp b/paddle/pserver/ParameterServerController.cpp
index ec24bc7e573dc..1d11a2e1acbc0 100644
--- a/paddle/pserver/ParameterServerController.cpp
+++ b/paddle/pserver/ParameterServerController.cpp
@@ -25,43 +25,44 @@ ParameterServerController::ParameterServerController(
   int numPorts = config.ports_num() + config.ports_num_for_sparse();
 
   if (config.nics().empty()) {
-    pservers_.resize(numPorts);
+    parameterServers_.resize(numPorts);
     for (int i = 0; i < numPorts; ++i) {
       if (config.rdma_tcp() == "rdma") {
-        pservers_[i].reset(
+        parameterServers_[i].reset(
             new ParameterServer2(std::string(), config.port() + i, rdmaCpu++));
         rdmaCpu = rdmaCpu % onlineCpus;
       } else {
-        pservers_[i].reset(
+        parameterServers_[i].reset(
             new ParameterServer2(std::string(), config.port() + i));
       }
-      CHECK(pservers_[i]->init()) << "Fail to initialize parameter server"
-                                  << config.port() + i;
+      CHECK(parameterServers_[i]->init()) << "Fail to initialize parameter "
+                                             "server on port "
+                                          << config.port() + i;
     }
   } else {
     str::split(config.nics(), ',', &devices);
-    pservers_.resize(devices.size() * numPorts);
+    parameterServers_.resize(devices.size() * numPorts);
     for (int i = 0; i < numPorts; ++i) {
       for (size_t j = 0; j < devices.size(); ++j) {
         if (config.rdma_tcp() == "rdma") {
-          pservers_[i * devices.size() + j].reset(new ParameterServer2(
+          parameterServers_[i * devices.size() + j].reset(new ParameterServer2(
               getIpAddr(devices[j]), config.port() + i, rdmaCpu++));
           rdmaCpu = rdmaCpu % onlineCpus;
         } else {
-          pservers_[i * devices.size() + j].reset(
+          parameterServers_[i * devices.size() + j].reset(
               new ParameterServer2(getIpAddr(devices[j]), config.port() + i));
         }
-        CHECK(pservers_[i * devices.size() + j]->init())
-            << "Fail to initialize parameter server" << devices[j]
+        CHECK(parameterServers_[i * devices.size() + j]->init())
+            << "Fail to initialize parameter server with device " << devices[j]
             << config.port() + i;
       }
     }
   }
 }
 
-ParameterServerController::~ParameterServerController() { this->join(); }
+ParameterServerController::~ParameterServerController() { this->wait(); }
 
-ParameterServerController* ParameterServerController::createByGflags() {
+ParameterServerController* ParameterServerController::createFromGflags() {
   ParameterServerConfig config;
 
   config.set_nics(FLAGS_nics);
@@ -79,21 +80,21 @@ ParameterServerController* ParameterServerController::create(
 }
 
 void ParameterServerController::start() {
-  LOG(INFO) << "pserver sizes : " << pservers_.size();
+  LOG(INFO) << "number of parameterServer instances: "
+            << parameterServers_.size();
   int i = 0;
-  for (const auto& pserver : pservers_) {
-    LOG(INFO) << "pserver started : " << i;
-    pserver->start();
+  for (const auto& parameterServer : parameterServers_) {
+    LOG(INFO) << "Starting parameterServer[" << i << "]";
+    parameterServer->start();
     i++;
   }
 }
 
-void ParameterServerController::join() {
-  LOG(INFO) << "pserver sizes : " << pservers_.size();
+void ParameterServerController::wait() {
   int i = 0;
-  for (const auto& pserver : pservers_) {
-    LOG(INFO) << "pserver join : " << i;
-    pserver->join();
+  for (const auto& parameterServer : parameterServers_) {
+    LOG(INFO) << "Waiting parameterServer[" << i << "]";
+    parameterServer->join();
     i++;
   }
 }
diff --git a/paddle/pserver/ParameterServerController.h b/paddle/pserver/ParameterServerController.h
index ee249de9d802d..fe9bb0b4d0233 100644
--- a/paddle/pserver/ParameterServerController.h
+++ b/paddle/pserver/ParameterServerController.h
@@ -21,6 +21,12 @@ limitations under the License. */
 
 namespace paddle {
 
+/**
+ * @brief ParameterServerController is used for create, init and manage multi
+ * parameter server instances. The num of the instances is decided by port
+ * num(the ports number for parameter send) and network devices configured
+ * by gflags or proto.
+ */
 class ParameterServerController final {
 public:
   DISABLE_COPY(ParameterServerController);
@@ -39,28 +45,30 @@ class ParameterServerController final {
    * @brief create ParameterServerController from gflags, this is used for
    * compatibility with the old usage of configuration by gflags.
    */
-  static ParameterServerController* createByGflags();
+  static ParameterServerController* createFromGflags();
 
   /**
    * @brief create ParameterServerController with ParameterServerConfig, remove
-   * gflags from ParameterServer. Init all pservers thread according to the
-   * config.
+   * gflags from ParameterServer. Init all ParameterServer2 instances according
+   * to
+   * the config.
    */
   static ParameterServerController* create(const ParameterServerConfig& config);
 
   /**
-   * @brief start all pserver thread in this ParameterServerController.
+   * @brief start all ParameterServer2 instances in this
+   * ParameterServerController.
    */
   void start();
 
   /**
-   * @brief join and wait for all pserver thread in this
+   * @brief join and wait for all ParameterServer2 instances thread in this
    * ParameterServerController.
    */
-  void join();
+  void wait();
 
 private:
-  std::vector<std::unique_ptr<ParameterServer2>> pservers_;
+  std::vector<std::unique_ptr<ParameterServer2>> parameterServers_;
 };
 
 }  // namespace paddle
diff --git a/paddle/trainer/TrainerMain.cpp b/paddle/trainer/TrainerMain.cpp
index 61de728f2a2a2..c5c1d484e5f85 100644
--- a/paddle/trainer/TrainerMain.cpp
+++ b/paddle/trainer/TrainerMain.cpp
@@ -36,10 +36,11 @@ int main(int argc, char** argv) {
   initMain(argc, argv);
   initPython(argc, argv);
 
-  std::unique_ptr<ParameterServerController> pServerPtr(nullptr);
+  std::unique_ptr<ParameterServerController> parameterServerPtr(nullptr);
   if (FLAGS_start_pserver) {
-    pServerPtr.reset(paddle::ParameterServerController::createByGflags());
-    pServerPtr->start();
+    parameterServerPtr.reset(
+        paddle::ParameterServerController::createFromGflags());
+    parameterServerPtr->start();
   }
   Trainer trainer;
   auto config = TrainerConfigHelper::createFromFlags();
diff --git a/proto/ParameterServerConfig.proto b/proto/ParameterServerConfig.proto
index b4fbf901c20cc..3068bba8b10d8 100644
--- a/proto/ParameterServerConfig.proto
+++ b/proto/ParameterServerConfig.proto
@@ -15,10 +15,17 @@ syntax = "proto2";
 
 package paddle;
 
+
+/**
+ * Configuration structure for ParameterClient2.
+ */
 message ParameterClientConfig {
   required int32 trainer_id = 1;
 }
 
+/**
+ * Configuration structure for ParameterServer2.
+ */
 message ParameterServerConfig {
   // The ports number for parameter send,
   // increment based on default port number