You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brpc.apache.org by GitBox <gi...@apache.org> on 2020/07/18 13:57:17 UTC
[GitHub] [incubator-brpc] heyuyi0906 opened a new pull request #1175: about support protobuf arena
heyuyi0906 opened a new pull request #1175:
URL: https://github.com/apache/incubator-brpc/pull/1175
#535
大佬们麻烦看下这个实现是否可以被接受，当前先只改了baidu rpc protocol。
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
[GitHub] [incubator-brpc] yjhjstz commented on pull request #1175: about support protobuf arena
Posted by GitBox <gi...@apache.org>.
yjhjstz commented on pull request #1175:
URL: https://github.com/apache/incubator-brpc/pull/1175#issuecomment-671147041
赞，好奇有多少的性能提升？
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
[GitHub] [incubator-brpc] hyswtj removed a comment on pull request #1175: about support protobuf arena
Posted by GitBox <gi...@apache.org>.
hyswtj removed a comment on pull request #1175:
URL: https://github.com/apache/incubator-brpc/pull/1175#issuecomment-671353784
赞！测试时，需要开启pb的arena么？您发的图片加载不了呢！
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
[GitHub] [incubator-brpc] mapleFU commented on a change in pull request #1175: about support protobuf arena
Posted by GitBox <gi...@apache.org>.
mapleFU commented on a change in pull request #1175:
URL: https://github.com/apache/incubator-brpc/pull/1175#discussion_r474183991
##########
File path: src/butil/memory_arena.h
##########
@@ -0,0 +1,159 @@
+
+#ifndef BUTIL_MEMORY_ARENA_H
+#define BUTIL_MEMORY_ARENA_H
+
+#include <memory>
+#include "google/protobuf/message.h"
+#include "butil/logging.h"
+
+namespace butil {
+
+template<class T>
+struct ArenaObjDeleter {
+ constexpr ArenaObjDeleter() noexcept = default;
Review comment:
为什么不把这个 mark 成 delete 呢？
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
[GitHub] [incubator-brpc] heyuyi0906 commented on pull request #1175: about support protobuf arena
Posted by GitBox <gi...@apache.org>.
heyuyi0906 commented on pull request #1175:
URL: https://github.com/apache/incubator-brpc/pull/1175#issuecomment-671292722
在大包情况下，自测发现对于性能提升还是比较明显的。
support_pb_arena分支测试结果：
master分支测试结果：
测试条件：
机器：8核 Intel(R) Xeon(R) Platinum 8255C CPU @ 2.50GHz
protobuf版本：v3.6.1
压测工具：rpc_press thread_num=50
协议：
```
syntax="proto2";
package example;
option cc_generic_services = true;
message Obj {
optional uint64 val = 1;
};
message EchoRequest {
required string message = 1;
repeated double array = 2;
repeated Obj objs = 3;
};
message EchoResponse {
required string message = 1;
repeated double array = 2;
repeated Obj objs = 3;
};
service EchoService {
rpc Echo(EchoRequest) returns (EchoResponse);
};
```
server处理逻辑：
```
response->set_message(request->message());
for (int i = 0; i < request->array_size(); ++i) {
response->add_array(request->array(i));
}
for (int i = 0; i < request->objs_size(); ++i) {
auto o = response->add_objs();
o->set_val(request->objs(i).val());
}
```
压测input：
```
{
"message":"xxx......",
"array":[2.9,2.9,......],
"objs":[{"val":10},......]
}
```
message字符串长度16384
array包含8192个double
objs包含8192个对象
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
[GitHub] [incubator-brpc] gydong commented on a change in pull request #1175: about support protobuf arena
Posted by GitBox <gi...@apache.org>.
gydong commented on a change in pull request #1175:
URL: https://github.com/apache/incubator-brpc/pull/1175#discussion_r474359936
##########
File path: src/brpc/policy/baidu_rpc_protocol.cpp
##########
@@ -148,8 +148,14 @@ void SendRpcResponse(int64_t correlation_id,
Socket* sock = accessor.get_sending_socket();
std::unique_ptr<Controller, LogErrorTextAndDelete> recycle_cntl(cntl);
ConcurrencyRemover concurrency_remover(method_status, cntl, received_us);
- std::unique_ptr<const google::protobuf::Message> recycle_req(req);
- std::unique_ptr<const google::protobuf::Message> recycle_res(res);
+ butil::ArenaObjDeleter<const google::protobuf::Message>
+ del(!cntl->get_memory_arena()->OwnObject());
+ std::unique_ptr<const google::protobuf::Message,
+ butil::ArenaObjDeleter<const google::protobuf::Message>
+ > recycle_req(req, del);
Review comment:
这种indent方式有些怪，可以统一成下面这样吗
```
std::unique_ptr<google::protobuf::Message,
butil::ArenaObjDeleter<google::protobuf::Message>> req;
std::unique_ptr<google::protobuf::Message,
butil::ArenaObjDeleter<google::protobuf::Message>> res;
```
##########
File path: src/brpc/controller.h
##########
@@ -510,6 +511,17 @@ friend void policy::ProcessThriftRequest(InputMessageBase*);
// -1 means no deadline.
int64_t deadline_us() const { return _deadline_us; }
+ std::shared_ptr<butil::MemoryArenaDefault> get_memory_arena() {
+ if (!_arena) {
+ _arena.reset(new butil::MemoryArenaDefault);
+ }
+ return _arena;
+ }
+ void share_memory_arena(Controller *cntl) {
Review comment:
为保持和现有代码风格一致，引用和指针符号统一对齐到左边与类型挨着吧
##########
File path: src/butil/memory_arena.h
##########
@@ -0,0 +1,159 @@
+
+#ifndef BUTIL_MEMORY_ARENA_H
+#define BUTIL_MEMORY_ARENA_H
+
+#include <memory>
+#include "google/protobuf/message.h"
+#include "butil/logging.h"
+
+namespace butil {
+
+template<class T>
+struct ArenaObjDeleter {
+ constexpr ArenaObjDeleter() noexcept = default;
+
+ ArenaObjDeleter(bool own) noexcept
+ : _own_obj(own) {
+ }
+
+ void operator()(T *ptr) const {
+ if (_own_obj) {
+ VLOG(199) << "delete!";
Review comment:
VLOG(RPC_VLOG_LEVEL)
##########
File path: src/butil/memory_arena.h
##########
@@ -0,0 +1,159 @@
+
+#ifndef BUTIL_MEMORY_ARENA_H
+#define BUTIL_MEMORY_ARENA_H
+
+#include <memory>
+#include "google/protobuf/message.h"
+#include "butil/logging.h"
+
+namespace butil {
+
+template<class T>
+struct ArenaObjDeleter {
+ constexpr ArenaObjDeleter() noexcept = default;
+
+ ArenaObjDeleter(bool own) noexcept
+ : _own_obj(own) {
+ }
+
+ void operator()(T *ptr) const {
+ if (_own_obj) {
+ VLOG(199) << "delete!";
+ delete ptr;
+ }
+ }
+
+private:
+ bool _own_obj;
+};
+
+namespace internal {
+
+// Template meta to check protobuf version.
+// protobuf 3.x: ArenaCheck::test<T>(0) => int8_t
+// protobuf 2.x: ArenaCheck::test<T>(0) => int64_t
+struct ArenaCheck {
+ template<class T> static int8_t test(decltype(&T::GetArena));
+ template<class T> static int64_t test(...);
+};
+
+// Don't specialize by yourselves, MemoryArenaDefault and MemoryArenaSimple
+// will be enough.
+// A simple implementation.
+template<class Def, class Enable = void>
+class MemoryArena {
+public:
+ std::unique_ptr<
+ google::protobuf::Message,
+ ArenaObjDeleter<google::protobuf::Message>
+ >
+ CreateMessage(const google::protobuf::Message &proto_type) {
+ google::protobuf::Message *msg = proto_type.New();
+ VLOG(199) << __FUNCTION__;
Review comment:
__FUNCTION__ 是哪里定义的宏，用__func__变量会更标准些吧
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
[GitHub] [incubator-brpc] hyswtj commented on pull request #1175: about support protobuf arena
Posted by GitBox <gi...@apache.org>.
hyswtj commented on pull request #1175:
URL: https://github.com/apache/incubator-brpc/pull/1175#issuecomment-671353784
赞！测试时，需要开启pb的arena么？您发的图片加载不了呢！
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
[GitHub] [incubator-brpc] heyuyi0906 commented on pull request #1175: about support protobuf arena
Posted by GitBox <gi...@apache.org>.
heyuyi0906 commented on pull request #1175:
URL: https://github.com/apache/incubator-brpc/pull/1175#issuecomment-671915132
好，我补充下这块的数据
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
[GitHub] [incubator-brpc] zyearn commented on pull request #1175: about support protobuf arena
Posted by GitBox <gi...@apache.org>.
zyearn commented on pull request #1175:
URL: https://github.com/apache/incubator-brpc/pull/1175#issuecomment-671364127
大包看起来效果不错，方便看下不同包大小的场景下（比如16B，32B，64B，128B，256B，512B...）的性能差异么，这些包更典型些
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org