Skip to content
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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

lorinlee
Copy link
Contributor

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.(请遵循贡献者准则).

@lorinlee
Copy link
Contributor Author

@ehds @wwbmmm @zyearn c++17就先不默认打开了,先默认打开aligned-new吧,辛苦review下看看

@@ -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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

我看 CMakeLists 也有判断 Clang 的逻辑,这里是否也需要对 Clang 编译器生效?

@uvletter
Copy link

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.

@yanglimingcn
Copy link
Contributor

所以,看上去这个问题是brpc单方面解决不了的,需要用户和bRPC框架的共同配合才行,是这么理解吧。

@ehds
Copy link
Contributor

ehds commented May 19, 2024

是这么理解吧

是的。如果用户使用 brpc 提供的默认 CMakeLists 来编译安装,但是在项目中又开启了该参数就会引发这个问题,两者要保持。

我的理解是:既然某些类显示的标注了 CacheLine 对齐,如果编译器支持,那么尽量就是用对齐的分配方式,来达到 CacheLine 对齐所带来的益处。

@yanglimingcn
Copy link
Contributor

brpc的默认行为和编译器的默认行为保持一致这样是不是不容易产生问题。
一个用户如果对brpc的编译和对自己程序的编译选项不一致,他本身得认识到这个问题。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants