Skip to content

Refine rpc client wait sync#11132

Merged
gongweibao merged 13 commits intoPaddlePaddle:developfrom
typhoonzero:fix_grpc_terminate_core
Jun 5, 2018
Merged

Refine rpc client wait sync#11132
gongweibao merged 13 commits intoPaddlePaddle:developfrom
typhoonzero:fix_grpc_terminate_core

Conversation

@typhoonzero
Copy link
Contributor

@typhoonzero typhoonzero commented Jun 3, 2018

Refine rpc client wait request sync

@typhoonzero typhoonzero changed the title [WIP] Fix dist train terminate core Jun 4, 2018
@typhoonzero typhoonzero mentioned this pull request Jun 4, 2018
@typhoonzero typhoonzero changed the title Fix dist train terminate core Jun 5, 2018
Copy link
Contributor

@gongweibao gongweibao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two small questions.

}

virtual void Process() = 0;
virtual void Process() {}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why change from pure virtual method to a virtual method?

void RPCClient::Wait() {
std::unique_lock<std::mutex> lk(sync_mutex_);
sync_cond_.wait(lk, [this] { return req_count_ == 0; });
lk.unlock();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line is not needed!

Copy link
Contributor

@gongweibao gongweibao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@gongweibao gongweibao merged commit 036a90f into PaddlePaddle:develop Jun 5, 2018
@typhoonzero typhoonzero deleted the fix_grpc_terminate_core branch June 5, 2018 09:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants