You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brpc.apache.org by "lorinlee (via GitHub)" <gi...@apache.org> on 2023/10/22 14:47:29 UTC
[PR] enable aligned new if supported (brpc)
lorinlee opened a new pull request, #2421:
URL: https://github.com/apache/brpc/pull/2421
### What problem does this PR solve?
Issue Number: #2416
Problem Summary:
### What is changed and the side effects?
Changed:
Side effects:
- Performance effects(性能影响):
- Breaking backward compatibility(向后兼容性):
---
### Check List:
- Please make sure your changes are compilable(请确保你的更改可以通过编译).
- When providing us with a new feature, it is best to add related tests(如果你向我们增加一个新的功能, 请添加相关测试).
- Please follow [Contributor Covenant Code of Conduct](../../master/CODE_OF_CONDUCT.md).(请遵循贡献者准则).
--
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-unsubscribe@brpc.apache.org
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
Re: [PR] enable aligned new if supported (brpc)
Posted by "lorinlee (via GitHub)" <gi...@apache.org>.
lorinlee commented on PR #2421:
URL: https://github.com/apache/brpc/pull/2421#issuecomment-1774130250
@ehds @wwbmmm @zyearn c++17就先不默认打开了，先默认打开aligned-new吧，辛苦review下看看
--
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-unsubscribe@brpc.apache.org
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
Re: [PR] enable aligned new if supported (brpc)
Posted by "yanglimingcn (via GitHub)" <gi...@apache.org>.
yanglimingcn commented on PR #2421:
URL: https://github.com/apache/brpc/pull/2421#issuecomment-2119047824
所以，看上去这个问题是brpc单方面解决不了的，需要用户和bRPC框架的共同配合才行，是这么理解吧。
--
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-unsubscribe@brpc.apache.org
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
Re: [PR] enable aligned new if supported (brpc)
Posted by "ehds (via GitHub)" <gi...@apache.org>.
ehds commented on code in PR #2421:
URL: https://github.com/apache/brpc/pull/2421#discussion_r1368083110
##########
CMakeLists.txt:
##########
@@ -148,8 +148,8 @@ if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
# segmentation fault in libcontext
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fno-gcse")
endif()
- if(NOT (CMAKE_CXX_COMPILER_VERSION VERSION_LESS 7.0))
- set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-aligned-new")
+ if(CMAKE_CXX_COMPILER_VERSION VERSION_GREATER 7.0)
Review Comment:
我看 CMakeLists 也有判断 Clang 的逻辑，这里是否也需要对 Clang 编译器生效？
##########
CMakeLists.txt:
##########
@@ -148,8 +148,8 @@ if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
# segmentation fault in libcontext
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fno-gcse")
endif()
- if(NOT (CMAKE_CXX_COMPILER_VERSION VERSION_LESS 7.0))
- set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-aligned-new")
+ if(CMAKE_CXX_COMPILER_VERSION VERSION_GREATER 7.0)
Review Comment:
我看 CMakeLists 也有判断 Clang 的逻辑，这里是否也需要对 Clang 编译器生效？
--
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-unsubscribe@brpc.apache.org
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
Re: [PR] enable aligned new if supported (brpc)
Posted by "uvletter (via GitHub)" <gi...@apache.org>.
uvletter commented on PR #2421:
URL: https://github.com/apache/brpc/pull/2421#issuecomment-1965752700
The solution is not sophisticated in my opinion. The brpc header file may be included in other translation units, which could be not aligned-new, so the final artifact will be a mixture of aligned-new and non-aligned-new.
--
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-unsubscribe@brpc.apache.org
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
Re: [PR] enable aligned new if supported (brpc)
Posted by "ehds (via GitHub)" <gi...@apache.org>.
ehds commented on PR #2421:
URL: https://github.com/apache/brpc/pull/2421#issuecomment-2119135643
> 是这么理解吧
是的。如果用户使用 brpc 提供的默认 CMakeLists 来编译安装，但是在项目中又开启了该参数就会引发这个问题，两者要保持。
我的理解是：既然某些类显示的标注了 CacheLine 对齐，那么如果编译器支持，那么尽量就是用对齐的分配方式，来达到 CacheLine 对齐所带来的益处。
--
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-unsubscribe@brpc.apache.org
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
Re: [PR] enable aligned new if supported (brpc)
Posted by "yanglimingcn (via GitHub)" <gi...@apache.org>.
yanglimingcn commented on PR #2421:
URL: https://github.com/apache/brpc/pull/2421#issuecomment-2119166722
brpc的默认行为和编译器的默认行为保持一致这样是不是不容易产生问题。
一个用户如果对brpc的编译和对自己程序的编译选项不一致，他本身得认识到这个问题。
--
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-unsubscribe@brpc.apache.org
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