Async GRPC sendrecv #7133

gongweibao · 2018-01-02T06:59:56Z

helinwang · 2018-01-03T20:14:52Z

paddle/operators/detail/grpc_client.cc

+using grpc::ClientWriter;
+using grpc::Status;
+using sendrecv::SendRecvService;
+using sendrecv::VariableMessage;


I don't use C++ often, so I am likely to be wrong. I personally feel that:

void Run(const framework::Scope* scope, const std::string name, sendrecv::VariableMessage* msg) {

is easier to understand than:

void Run(const framework::Scope* scope, const std::string name, VariableMessage* msg) {

Since sendrecv::VariableMessage tells us that VariableMessage is from a different module, and that module does things like send and recv, so I can infer that VariableMessage is the message used for send and recv.

Done.Thanks.

helinwang · 2018-01-03T21:21:54Z

paddle/operators/detail/grpc_client.cc

+    framework::DeserializeFromStream(iss, &ret_tensor);
+    auto* outvar = scope->FindVar(name);
+    framework::LoDTensor* out_tensor =
+        outvar->GetMutable<framework::LoDTensor>();


Is it possible that the variable type for send/recv is not framework::LoDTensor?

Thanks.Add FixME comments.
Done.

helinwang · 2018-01-03T21:47:07Z

paddle/operators/detail/grpc_client.cc

+
+    // Block until the next result is available
+    // in the completion queue "cq".
+    cq.Next(&which, &ok);


Do we need to check the value of ok?

Done.Thanks.

helinwang · 2018-01-03T21:58:48Z

paddle/operators/detail/grpc_client.h

+namespace operators {
+namespace detail {
+
+struct Var {


We already have Variable, to avoid confusion, maybe change name to something like VarHandle.

helinwang · 2018-01-03T21:59:58Z

paddle/operators/detail/grpc_server.cc

+      new RequestSend(service_, cq_);
+
+      // The actual processing.
+      // std::string prefix("Hello ");


No commented out code please :)

Thanks.Done.

helinwang · 2018-01-03T22:07:51Z

paddle/operators/detail/grpc_server.cc

+      service_->RequestSendVariable(&ctx_, &request_, &responder_, cq_, cq_,
+                                    this);
+    } else if (status_ == PROCESS) {
+      new RequestSend(service_, cq_);


Could not understand what does this code block do, why does it not use request_?

A detail comments is here.

I think the status is not useful also. I'll test it later.

我的理解：

CREATE的时候注册消息处理的类。

PROCESS的时候是完成队列通知request已经准备好，可以处理了。

FINISH的时候是完成队列通知reply已经发送完了。

所以这个PROCEED从完成队列中重入两次。

helinwang · 2018-01-03T22:10:58Z

paddle/operators/detail/grpc_server.cc

+namespace operators {
+namespace detail {
+
+enum CallStatus { CREATE, PROCESS, FINISH };


Trying to understand what CREATE, PROCESS, FINISH does, but where are they used by the client?

A detail comments is here.

typhoonzero · 2018-01-04T02:11:11Z

paddle/operators/detail/grpc_client.cc

+#include "grpc_client.h"
+#include <future>
+
+using grpc::Server;


Remove unnecessary using lines.

typhoonzero · 2018-01-04T02:15:58Z

paddle/operators/detail/grpc_client.h

+  }
+};
+
+class AsyncGRPCClient {


Just call this class RPCClient the same as before since we only use just one client and remove the previous implementation.

typhoonzero · 2018-01-04T02:19:38Z

paddle/operators/detail/grpc_client.h

+  // is sent completely and don't wait all.
+ protected:
+  template <typename send_t, typename recv_t, typename Msg_t>
+  bool Call(const framework::Scope* scope, const std::vector<Var>& in,


Call has no meaning, change to some function names that mean something, please.

typhoonzero · 2018-01-04T02:26:06Z

paddle/operators/detail/grpc_client.h

+  AsyncGRPCClient() {}
+
+  void AddEndPoint(std::string ep);
+  void AddEndPoint(const std::vector<std::string>& ep);


Delete AddEndPoint, and put the initialization into two overloaded ctors.

Will refactor client interface.

typhoonzero · 2018-01-04T02:27:32Z

paddle/operators/detail/grpc_client.cc

+                  const framework::Scope* scope, std::string name) {}
+
+  template <typename send_t, typename recv_t>
+  void Call(grpc::CompletionQueue* cq,


SendMsg have Run and Call which is confusing.

typhoonzero · 2018-01-04T03:12:59Z

paddle/operators/detail/grpc_server.cc

+      responder_.Finish(reply_, Status::OK, this);
+    } else {
+      GPR_ASSERT(status_ == FINISH);
+      delete this;


delete this should be used very carefully or never use.

Will test later.

delete this cause core down.

typhoonzero · 2018-01-04T03:14:02Z

paddle/operators/detail/grpc_client.cc

+    const time_point deadline = s.start + std::chrono::milliseconds(5000);
+
+    // context
+    ClientContext* context = new ClientContext();


Use unique_ptr to avoid memory leak.

already in.

typhoonzero · 2018-01-04T03:15:39Z

paddle/operators/detail/grpc_client.cc

+};
+
+template <typename send_t, typename recv_t, typename Msg_t>
+bool AsyncGRPCClient::Call(const framework::Scope* scope,


Templates should be placed at .h

typhoonzero · 2018-01-04T03:16:04Z

paddle/operators/detail/grpc_client.cc

+  }
+};
+
+template <typename send_t, typename recv_t, typename Msg_t>


Typenames should be capitalized.

typhoonzero · 2018-01-04T03:17:32Z

paddle/operators/detail/grpc_client.cc

+  typedef std::chrono::system_clock::time_point time_point;
+
+  for (int64_t i = 0; i < (int64_t)in.size(); i++) {
+    PADDLE_ENFORCE(channels_.find(in[i].endpoint) != channels_.end());


Do not manage channels in a single client, let the caller manage this.

Or you can write a multi-client class to do multi send.

gongweibao · 2018-01-04T09:44:51Z

Reconstruction grpc_client.

typhoonzero · 2018-01-05T03:21:42Z

paddle/operators/detail/CMakeLists.txt

@@ -1 +1 @@
-grpc_library(sendrecvop_grpc SRCS recv_impl.cc send_impl.cc PROTO send_recv.proto DEPS lod_tensor selected_rows)
+grpc_library(sendrecvop_grpc SRCS recv_impl.cc send_impl.cc grpc_client.cc grpc_server.cc PROTO send_recv.proto DEPS lod_tensor selected_rows)


Remove recv_impl.cc send_impl.cc if they are nolonger used.

Done.Thanks.

typhoonzero · 2018-01-05T04:32:30Z

paddle/operators/send_op.cc

-// one tensor
+bool Send(const std::vector<std::string>& ep, const framework::Scope* scope,
+          const std::vector<std::string>& ins, int64_t time_out) {
+  detail::RPCClients c;


Creating new client object in each send is inefficient.

Done.Thanks.

typhoonzero · 2018-01-05T04:47:06Z

paddle/operators/detail/grpc_client.h

+  SendStatus send_status_;
+};
+
+class RPCClients {


I still believe that send_op should be responsible to determine which endpoint should each variable send to. Client is only responsible for how to do communication, send_op is responsible for how to manage the send, if we are going to implement something like "ring-based all reduce", will only need to write a new send_op and do not need to change this client implementation.

Done.Thanks.

typhoonzero · 2018-01-05T04:49:39Z

paddle/operators/detail/grpc_client.h

+  SendStatus GetSendStatus() { return send_status_; }
+
+ private:
+  void _Request(const framework::Scope* scope, const std::string name,


See https://google.github.io/styleguide/cppguide.html#Function_Names

Done.Thanks.

typhoonzero · 2018-01-05T04:53:20Z

paddle/operators/detail/grpc_client.h

+    ch_ = ch;
+    ep_ = ep;
+    stub_ = sendrecv::SendRecvService::NewStub(ch);
+    context_.reset(new grpc::ClientContext());


context should be created before every rpc send call.

Done.Thanks.

typhoonzero · 2018-01-05T04:55:44Z

paddle/operators/detail/grpc_client.h

+  }
+};
+
+class ClientBase {


Would an RPCClient implementing SendVariable and GetVariable be sufficient?

members are the same and send, get should be called on the same RPC client stub

+ std::shared_ptr<grpc::CompletionQueue> cq_; + std::shared_ptr<grpc::Channel> ch_; + std::string ep_; + std::unique_ptr<sendrecv::SendRecvService::Stub> stub_; + std::unique_ptr<grpc::ClientContext> context_; + std::unique_ptr<grpc::Status> status_; + std::unique_ptr<sendrecv::VoidMessage> reply_; + std::unique_ptr<grpc::ClientAsyncResponseReader<sendrecv::VoidMessage>> rpc_; + SendStatus send_status_;

Done.thanks.

Yancey1989 · 2018-01-05T07:53:35Z

#7161 add some features about send/recv op, so please fix the conflict first.

typhoonzero · 2018-01-09T03:22:02Z

paddle/operators/detail/grpc_client.h

+  bool wait();
+
+ private:
+  bool Proceed();


By Proceed do you mean Process, proceed means go on with something. The sample code use proceed maybe because it's a stream request.

typhoonzero · 2018-01-09T03:24:09Z

paddle/operators/detail/grpc_client.h

+ private:
+  grpc::CompletionQueue cq_;
+  std::map<std::string, std::shared_ptr<grpc::Channel>> channels_;
+  int64_t count_ = 0;


count_ is a name with less information, use xxx_count_ instead.

typhoonzero · 2018-01-09T03:28:34Z

paddle/operators/detail/grpc_client.h

+ public:
+  bool AsyncSendVariable(const std::string& ep,
+                         const platform::DeviceContext& ctx,
+                         const framework::Scope* scope,


Can be const framework::Scope&

typhoonzero · 2018-01-09T04:33:45Z

paddle/operators/detail/grpc_client.cc

+  auto ch = GetChannel(ep);
+  SendProcessor* s = new SendProcessor(ch);
+  s->Prepare(var_h, time_out);
+  s->response_call_back = ProcSendResponse;


Since ProcSendResponse is doing nothing, pass NULL here should be fine.

typhoonzero · 2018-01-09T05:03:52Z

paddle/operators/detail/grpc_client.h

+typedef std::function<void(const VarHandle&, const sendrecv::VoidMessage&)>
+    RequestSendCallBack;
+
+struct SendProcessor : public ClientBase {


From my point of view, thought below definition would be simpler to implement:

typedef void (*rpc_response_callback)(const framework::Scope&, void* message); class RPCClient { .... private: struct RPCResponse { rpc_response_callback callback_; const framework::Scope& scope_; std::string varname_; int ret_msg_type_; grpc::Status status_; }; };

Then AsyncSendVariable and AsyncGetVariable can use ret_msg_type_ to get whether to cast message to VoidMessage or VariableMessage. VarHandle can also be ommited.

Then AsyncSendVariable and AsyncGetVariable can use ret_msg_type_ to get whether to cast message to VoidMessage or VariableMessage.

这个我试过了。需要暴露类型的变量有两个，这是不能用一个struct解决的。

sendrecv::XXXMessageType reply; RequestXXXCallBack response_call_back

根据ret_msg_type_是不够的，不方便以后的扩展，而是应该根据action_type来判断哪种操作类型。有两个问题

代码写起来很丑陋，需要各处switch。
2.扩展起来不方便。

异步的情况下：VarHandle里边的信息还是有用的：

struct VarHandle { std::string ep; const platform::DeviceContext* ctx; const framework::Scope& scope; std::string name; };

需要暴露类型的变量有两个，这是不能用一个struct解决的。

嗯。感觉这个也不是主要原因，从扩展来讲，用继承会比较好。

异步的情况下：VarHandle里边的信息还是有用的：

我理解应该是为了减少callback中的参数个数。这样的话，可以直接把callback定义成void(void* ret_msg, void* user_data)，在不同的继承的Processor中再cast到实际的类型，扩展性会更好。

typhoonzero · 2018-01-09T05:09:38Z

paddle/operators/detail/grpc_server.cc

+enum CallStatus { PROCESS = 0, FINISH };
+
+// reference:
+// https://stackoverflow.com/questions/41732884/grpc-multiple-services-in-cpp-async-server


In fact we only have one service: service SendRecvService

For more than two types of calls, you could achieve the same behavior with an enum instead of a bool.
A more flexible approach would be to have a different CallData-like class for each RPC. However when you get a tag from cq_->Next()

Thanks for the explanation, now I figure it out!

typhoonzero · 2018-01-09T05:25:21Z

paddle/operators/detail/grpc_server.cc

+ * https://stackoverflow.com/questions/35708348/grpc-what-is-the-recommended-way-to-shut-down-an-asynchronous-server-in-c
+ */
+void AsyncGRPCServer::ShutDown() {
+  ShutdownGetQueue();


See https://github.com/grpc/grpc/blob/master/examples/cpp/helloworld/greeter_async_server.cc#L43

typhoonzero · 2018-01-09T06:01:13Z

paddle/operators/detail/utils.h

+namespace operators {
+namespace detail {
+
+void SerializeToMessage(const std::string& name, const framework::Variable* var,


Can you please put these two function into grpc_client.h, file name util.h have less information.

这样的话grpc_server.cc需要依赖grpc_client.h了。。。。

typhoonzero · 2018-01-09T06:07:34Z

paddle/operators/detail/grpc_server.h

+
+  std::mutex cq_get_mutex_;
+  mutable bool is_get_shut_down_ = false;
+  std::unique_ptr<grpc::ServerCompletionQueue> cq_get_;


Can server-side use one single ServerCompletionQueue? And two threads to deal with different message make this implement complicated. Since we have var_recv_queue_ so the backend can use multiple threads to deal with incoming requests.

Resolved in another way.Done.

… asyncsendrecv

Yancey1989 · 2018-01-09T07:58:25Z

paddle/operators/detail/CMakeLists.txt

@@ -1 +1 @@
-grpc_library(sendrecvop_grpc SRCS recv_impl.cc send_impl.cc PROTO send_recv.proto DEPS lod_tensor selected_rows)
+grpc_library(sendrecvop_grpc SRCS utils.cc grpc_client.cc grpc_server.cc PROTO send_recv.proto DEPS lod_tensor selected_rows)


utils.cc => sendrecvop_utils, I think it's not a common utils.

Yancey1989 · 2018-01-09T08:12:43Z

paddle/operators/detail/grpc_client.cc

+
+  // stub context
+  auto ch = GetChannel(ep);
+  SendProcessor* s = new SendProcessor(ch);


I think std::unique_ptr<SendProcessor> s(new SendProcessor(ch)); is a better way.

The memory should not be released and should transformed to Completion queue.

rpc->Finish(&s->reply, &s->status, (void*)s);

Yancey1989 · 2018-01-09T08:15:39Z

paddle/operators/detail/grpc_client.cc

+  PADDLE_ENFORCE(tag);
+
+  // TODO(gongwb): add more retries.
+  ClientBase* c = (ClientBase*)tag;


ClientBase* c = (ClientBase*)tag
=>
ClientBase* c = static_cast<ClientBase *>(tag)

Yancey1989 · 2018-01-09T08:21:08Z

paddle/operators/detail/grpc_client.h

+
+  virtual void Proceed() = 0;
+
+  std::unique_ptr<sendrecv::SendRecvService::Stub> stub;


Following https://google.github.io/styleguide/cppguide.html#Variable_Names, please add the underscore at end of the variable name.

Please see Struct Data Members section.

Yancey1989 · 2018-01-09T08:23:49Z

paddle/operators/detail/grpc_client.h

+
+  bool AsyncGetVariable(const std::string& ep,
+                        const platform::DeviceContext& ctx,
+                        const framework::Scope* scope,


The save as above
const framework::Scope* scope
=>
const framework::Scope& scope

Yancey1989 · 2018-01-09T08:27:14Z

paddle/operators/detail/grpc_server.cc

+  explicit RequestBase(sendrecv::SendRecvService::AsyncService* service,
+                       grpc::ServerCompletionQueue* cq)
+      : service_(service), cq_(cq) {
+    status_ = PROCESS;


Move status_ = PROCESS to the constructor initializer list.

Yancey1989 · 2018-01-09T08:28:16Z

paddle/operators/detail/grpc_server.cc

+    // proc request.
+    std::string var_name = request_.varname();
+    auto* var = scope_->FindVar(var_name);
+    // FIXME(gongwb): device context?


Can not understand what's the meaning of FIXME comment.

typhoonzero

Some tiny comments, can be merged soon!

typhoonzero · 2018-01-09T11:56:24Z

paddle/operators/detail/grpc_client.h

+void ProcGetResponse(const VarHandle& var_h,
+                     const sendrecv::VariableMessage& msg);
+
+struct ClientBase {


Change to class according to https://google.github.io/styleguide/cppguide.html#Structs_vs._Classes

typhoonzero · 2018-01-09T12:16:49Z

paddle/operators/detail/grpc_client.h

+typedef std::function<void(const VarHandle&, const sendrecv::VoidMessage&)>
+    RequestSendCallBack;
+
+struct SendProcessor : public ClientBase {


需要暴露类型的变量有两个，这是不能用一个struct解决的。

嗯。感觉这个也不是主要原因，从扩展来讲，用继承会比较好。

异步的情况下：VarHandle里边的信息还是有用的：

我理解应该是为了减少callback中的参数个数。这样的话，可以直接把callback定义成void(void* ret_msg, void* user_data)，在不同的继承的Processor中再cast到实际的类型，扩展性会更好。

typhoonzero · 2018-01-09T12:18:29Z

paddle/operators/detail/grpc_server.cc

+enum CallStatus { PROCESS = 0, FINISH };
+
+// reference:
+// https://stackoverflow.com/questions/41732884/grpc-multiple-services-in-cpp-async-server


Thanks for the explanation, now I figure it out!

typhoonzero · 2018-01-09T12:19:02Z

paddle/operators/detail/grpc_server.cc

+  virtual ~RequestSend() {}
+
+  virtual void Process() {
+    // proc request.


You can remove the comment line.

typhoonzero · 2018-01-09T12:22:14Z

paddle/operators/detail/grpc_server.cc

+  if (is_shut_down_) {
+    return;
+  }
+  // base->RegisterNewOne();


Remove commented out code.

typhoonzero · 2018-01-09T12:22:23Z

paddle/operators/detail/grpc_server.cc

+/*
+ * This URL explains why shutdown is complicate:
+ * https://stackoverflow.com/questions/35708348/grpc-what-is-the-recommended-way-to-shut-down-an-asynchronous-server-in-c
+ */


See https://google.github.io/styleguide/cppguide.html#Comment_Style

typhoonzero · 2018-01-09T12:26:10Z

paddle/operators/detail/grpc_server.h

+
+ private:
+  std::mutex cq_mutex_;
+  mutable bool is_shut_down_ = false;


mutable is for const functions, and there is no const functions?

typhoonzero · 2018-01-09T12:27:41Z

paddle/operators/detail/grpc_server.cc

+        break;
+      }
+      case FINISH: {
+        VLOG(4) << cq_name << " status:" << base->Status();


Use the same VLOG level in RPC implements.

typhoonzero · 2018-01-10T06:41:38Z

paddle/operators/send_recv_op_test.cc

@@ -136,11 +136,21 @@ void StartServerNet(bool is_sparse) {
  attrs.insert({"OptimizeProgram", program_proto});
  recv_op = f::OpRegistry::CreateOp("recv", {{"RX", {"x1"}}}, {}, attrs);
  recv_op->Run(scope, place);
+
+  std::cout << "servernet end" << std::endl;


Remove unit test printing code.

typhoonzero · 2018-01-10T08:58:53Z

paddle/operators/send_recv_op_test.cc

@@ -138,9 +138,17 @@ void StartServerNet(bool is_sparse) {
  recv_op->Run(scope, place);
 }

+void PrintTop(std::string name, float *data) {


Remove printing in the unit test.

这些print在出现错误的时候用来查找错误还是非常有用的。

ctest可以在错误的时候输出对比的数据。不需要print。

只能看到一个，其实很不利于找到错误。
Done。

typhoonzero

LGTM++

gongweibao added 9 commits December 27, 2017 13:14

init

0a22ddb

modify send

7aed2d0

before clean

1078965

add grpc async client

b833c24

add grpc client cc

4944e0e

add grpc_server

2effdf6

async update

cfabd23

check

ebd5a58

compile ok

f92005f

gongweibao requested review from Yancey1989, typhoonzero and helinwang January 3, 2018 11:54

helinwang reviewed Jan 3, 2018

View reviewed changes

typhoonzero requested changes Jan 4, 2018

View reviewed changes

gongweibao added 2 commits January 4, 2018 03:24

fix by helin comments

83232c7

refactor client

46f23cf

gongweibao added 3 commits January 4, 2018 11:48

test

4d6a55b

reconstruct grpc_client

4982c7d

clean code

a969b93

typhoonzero reviewed Jan 5, 2018

View reviewed changes

gongweibao added 2 commits January 5, 2018 07:19

fix by wuyi comments

a5308bf

clean some code

44debaf

gongweibao added 6 commits January 6, 2018 14:00

modify rpclient

15900a2

fix some bugs

c4705c9

rm wait of proto

126c118

fix confict

ed3bf64

fix client bugs

e33b936

clean some code

47bff2f

gongweibao added 5 commits January 7, 2018 13:36

modify

5bb69a8

clean some code

9543e2a

clean some code

d39efbc

server core

b86a2b5

test ok

ac1deb2

gongweibao changed the title ~~[WIP]Async GRPC sendrecv~~ Async GRPC sendrecv Jan 8, 2018

gongweibao added 2 commits January 8, 2018 12:18

fix some bugs

cd9dd1a

fix logical bug

0a401ad

typhoonzero reviewed Jan 9, 2018

View reviewed changes

gongweibao added 2 commits January 9, 2018 06:34

Merge branch 'develop' of https://github.com/PaddlePaddle/Paddle into…

9061334

… asyncsendrecv

follow by wuyi comments

b686112

Yancey1989 reviewed Jan 9, 2018

View reviewed changes

gongweibao added 3 commits January 9, 2018 09:26

server core

43a278b

test ok

44f9571

fix by comments

6e8b210

typhoonzero reviewed Jan 9, 2018

View reviewed changes

gongweibao added 2 commits January 10, 2018 01:53

fix by comments

165cddd

clean code

09e7c82

typhoonzero reviewed Jan 10, 2018

View reviewed changes

gongweibao added 2 commits January 10, 2018 06:45

follow by comments

a831898

resolve conflict

2669ad7

typhoonzero reviewed Jan 10, 2018

View reviewed changes

rm print

8256a30

typhoonzero approved these changes Jan 10, 2018

View reviewed changes

gongweibao merged commit da3087a into PaddlePaddle:develop Jan 11, 2018

gongweibao deleted the asyncsendrecv branch January 11, 2018 01:28

		@@ -1 +1 @@
		grpc_library(sendrecvop_grpc SRCS recv_impl.cc send_impl.cc PROTO send_recv.proto DEPS lod_tensor selected_rows)
		grpc_library(sendrecvop_grpc SRCS recv_impl.cc send_impl.cc grpc_client.cc grpc_server.cc PROTO send_recv.proto DEPS lod_tensor selected_rows)

		@@ -1 +1 @@
		grpc_library(sendrecvop_grpc SRCS recv_impl.cc send_impl.cc PROTO send_recv.proto DEPS lod_tensor selected_rows)
		grpc_library(sendrecvop_grpc SRCS utils.cc grpc_client.cc grpc_server.cc PROTO send_recv.proto DEPS lod_tensor selected_rows)


		virtual void Proceed() = 0;

		std::unique_ptr<sendrecv::SendRecvService::Stub> stub;

Async GRPC sendrecv #7133

Async GRPC sendrecv #7133

Conversation

gongweibao commented Jan 2, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gongweibao Jan 4, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gongweibao commented Jan 4, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Yancey1989 commented Jan 5, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gongweibao Jan 9, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gongweibao Jan 4, 2018 •

edited

Loading

gongweibao Jan 9, 2018 •

edited

Loading