-
Notifications
You must be signed in to change notification settings - Fork 4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
enable aligned new if supported #2421
base: master
Are you sure you want to change the base?
Conversation
@@ -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) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
我看 CMakeLists 也有判断 Clang 的逻辑,这里是否也需要对 Clang 编译器生效?
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. |
所以,看上去这个问题是brpc单方面解决不了的,需要用户和bRPC框架的共同配合才行,是这么理解吧。 |
是的。如果用户使用 brpc 提供的默认 CMakeLists 来编译安装,但是在项目中又开启了该参数就会引发这个问题,两者要保持。 我的理解是:既然某些类显示的标注了 CacheLine 对齐,如果编译器支持,那么尽量就是用对齐的分配方式,来达到 CacheLine 对齐所带来的益处。 |
brpc的默认行为和编译器的默认行为保持一致这样是不是不容易产生问题。 |
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: