wwbmmm commented on code in PR #3062: URL: https://github.com/apache/brpc/pull/3062#discussion_r2278179385
########## src/butil/iobuf_inl.cpp: ########## @@ -0,0 +1,116 @@ +// Licensed to the Apache Software Foundation (ASF) under one Review Comment: 1. The file name `iobuf_inl` means there are inline functions in it. It is always for header file, not cpp file. You can just move these code to `iobuf_inl.h`, and delete this `iobuf_inl.cpp` file. 2. When you seperate the function body from class/struct definition, that function is by default not inline. This may affect performance. To make sure the function is inline, you can keep the function body in class/struct definition, or add `inline` keyword for each function. ########## src/butil/iobuf.cpp: ########## @@ -341,7 +214,7 @@ uint32_t block_size(IOBuf::Block const* b) { return b->size; } -inline IOBuf::Block* create_block(const size_t block_size) { +IOBuf::Block* create_block(const size_t block_size) { Review Comment: remove the inline keyword may affect performance, you can move the function body to iobuf_inl.h, since that file is for inline functions. -- 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: dev-unsubscr...@brpc.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@brpc.apache.org For additional commands, e-mail: dev-h...@brpc.apache.org