apache / brpc

brpc is an Industrial-grade RPC framework using C++ Language, which is often used in high performance system such as Search, Storage, Machine learning, Advertisement, Recommendation etc. "brpc" means "better RPC".
https://brpc.apache.org
Apache License 2.0
16.57k stars 3.98k forks source link

brpc lack of rwlock implementation #1025

Open htn4179 opened 4 years ago

htn4179 commented 4 years ago

we want to use bthread_rwlock_t in our program, but it seems that brpc did not implemented.

hairet commented 4 years ago

https://github.com/hairet/brpc/commit/b2f23243ba3127db82d7ee17e6caba40bead0ef3

hairet commented 4 years ago

you can use this commit,we implement rwlock,and I commit in my fork recently

zyearn commented 4 years ago

so which one is the latest? https://github.com/apache/incubator-brpc/pull/1031 or https://github.com/apache/incubator-brpc/pull/1026?

hairet commented 4 years ago

so which one is the latest? #1031 or #1026?

the implementation in 1031 will be more clear,it should be the latest

but in 1031 I got some error when travis-ci do linking like bvar_percentile_unittest.cpp:(.text+0x35): undefined reference to `bvar::detail::Percentile::Percentile()' this seems unreasonable as bavr test and bavr cc_library are not modified in my commit

htn4179 commented 4 years ago

so which one is the latest? #1031 or #1026?

1031 is classic and less flag and status,

1026 has more flag and status, We maximize the read performance,

So, I think it is your choice

hairet commented 2 years ago

@zyearn I recently repick some issue and find this rwlock is still not in brpc master. And I updated commit today for #1031 to fix some infrequent bugs which commited 2 years ago in our inner brpc. As #1031 had already stable used in our company in our inner brpc for more than 2 years, I think maybe you can merge this PR now.If there's any worry, maybe you can contact me,Thanks. FYI, #1026 is also mantained by us,but it's an experimental version, we don't use this recent 2 years.

zyearn commented 2 years ago

@hairet Thanks for uploading the latest version.