Skip to content

Make Summary 128-bit aligned within the cacheline#111

Merged
oathdruid merged 1 commit intobaidu:mainfrom
chenBright:fix_parameters_alignment
Dec 22, 2025
Merged

Make Summary 128-bit aligned within the cacheline#111
oathdruid merged 1 commit intobaidu:mainfrom
chenBright:fix_parameters_alignment

Conversation

@chenBright
Copy link
Contributor

ConcurrentSummer::Summary也有apache/brpc#3091 一样的问题,形参内存可能没有对齐,不能满足128bit原子读写要求。

inline ABSL_ATTRIBUTE_ALWAYS_INLINE ConcurrentSummer&
ConcurrentSummer::operator<<(Summary summary) noexcept {
auto& local = _storage.local();
// 等效语句local += summary
// 但涉及128bit原子写操作,需要特殊处理
#ifdef __x86_64__
// 对于128bit写操作,Intel和AMD官方声明中均承诺未原子性
// 但是参考https://rigtorp.se/isatomic/
// 当前主流X86服务端CPU,单cacheline内128bit对齐的load/store其实是原子的
// 这里主动使用sse指令即可保证原子性写入
__m128i delta_value = _mm_load_si128(reinterpret_cast<__m128i*>(&summary));
__m128i local_value = _mm_load_si128(reinterpret_cast<__m128i*>(&local));
local_value = _mm_add_epi64(local_value, delta_value);
_mm_store_si128(reinterpret_cast<__m128i*>(&local), local_value);
#else
// Armv8.4-A开始,主流服务端CPU均支持对齐场景下128bit的读写原子性
// 这里主动使用neon生成128bit写操作
int64x2_t delta_value = vld1q_s64(reinterpret_cast<const int64_t*>(&summary));

@coveralls
Copy link
Collaborator

Coverage Status

coverage: 85.089% (-0.2%) from 85.269%
when pulling ded1eed on chenBright:fix_parameters_alignment
into c230b4f on baidu:main.

Copy link
Collaborator

@oathdruid oathdruid left a comment

Choose a reason for hiding this comment

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

LGTM

@oathdruid oathdruid merged commit 9e9e518 into baidu:main Dec 22, 2025
55 of 56 checks passed
@oathdruid
Copy link
Collaborator

不过其实原理上只是形参问题的话,其实换uload按说更经济,底层存储的也是这个summary,都扩展到cachline其实稍微有点费空间

@chenBright
Copy link
Contributor Author

不过其实原理上只是形参问题的话,其实换uload按说更经济,底层存储的也是这个summary,都扩展到cachline其实稍微有点费空间

https://rigtorp.se/isatomic/,似乎不能100%保证unaligned 128 bit的原子性。

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.

3 participants