-
Notifications
You must be signed in to change notification settings - Fork 5.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add brpc serialization support. #11430
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM basically, but still need a way to check this through CI later.
iobuf->append(reinterpret_cast<char*>(&k), 4); | ||
iobuf->append(reinterpret_cast<char*>(&vlen), 8); | ||
|
||
// FIXME(gongwb): use append_zerocopy |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why cannot use this? need brpc support?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We changed tensor memory holder, and brpc didn't support destroy userdata but only support destroy the payload data.
PADDLE_ENFORCE_NOT_NULL(payload); | ||
|
||
// FIXME(gongwb): it seems that can use zero copy. | ||
if (var_is_not_stable) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what var_is_not_stable
means?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When a var is a temp variable, it's memory can be free out, so we can't append only a pointer but copy data.
Yuyang change it tensor data holder to sharedptr and then this not useful.I'll clean up next pr.
a6be0a7
to
ddbd878
Compare
test=develop
test=develop
The macro should be defined by compiler rather than by source. test=develop
test=develop
test=develop
test=develop
test=develop
test=develop
test=develop fix visualizer test=develop add push test=develop add push test=develop clean up test=develop
test=develop
b60199a
to
8c58ede
Compare
a57508e
to
f459d4a
Compare
if (it != rpc_call_map.end()) { | ||
request_send_h_ = it->second; | ||
send_threads_.reset(new paddle::framework::ThreadPool( | ||
rpc_server_->GetThreadNum(distributed::kRequestSend))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can use FLAGS_rpc_send_thread_num
etc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The caller will use FLAGS_rpc_send_thread_num
to register this property and it use the arguments.
Fix part of #10804