wwbmmm commented on PR #3140: URL: https://github.com/apache/brpc/pull/3140#issuecomment-3496491514
@lh2debug Thank you for your contribution! This solve a very important issue. You have done a good job! Currently I have two concerns: 1. This implementation add some locks and shard_ptr，it may affect performance. Have you compare its performance with the original version? 2. This implementation change some internal APIs (such as `ControllerPrivateAccessor::span()`. Although these will not be used by most brpc user, they may be used by some advance users, who have extended brpc functions (for example, add new protocols). So this PR will make a incompatible upgrade to these users. Maybe you can try to add some macros to controll this feature, for example, you can define alias types like SpanSharedPtr and SpanWeakPtr, when BRPC_ENABLE_SHARED_SPAN is on, they are `std::shared_ptr<Span>` and `std::weak_ptr<Span>`, when the flag is off, they are `Span*`. -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
