Benchmarks, static assertions, system.warnings for per CPU by azat · Pull Request #109283 · ClickHouse/ClickHouse

azat · 2026-07-03T11:37:29Z

Changelog category (leave one):

Not for changelog (changelog entry is not required)

Also according to the results, sched_getcpu() overhead is negligible even w/ plain syscall (though this is only the case when there is real contention), but anyway we have rseq and glibc 2.35+ (released in the beginning 2022) for amd64/aarch64

rseq_vs_atomic_benchmark

threads=64 ops=50000000 slot_count=64 rseq_supported=yes

shared: 955.76 ns/op (wall 50.80 s)
atomic: 4.00 ns/op (wall 0.28 s)
rseq:   2.86 ns/op (wall 0.15 s)
speedup (atomic / rseq):   1.40x
speedup (shared / atomic): 238.85x

src/Common/benchmarks/benchmark_sched_getcpu

BM_sched_getcpu_current        1.68 ns         1.68 ns    415019976
BM_sched_getcpu_rseq          0.279 ns        0.278 ns   1000000000
BM_sched_getcpu_vsyscall       2.51 ns         2.51 ns    279347470
BM_sched_getcpu_syscall        69.7 ns         69.5 ns     10004624

Follow-up for: #105056

Version info

Merged into: 26.8.1.124 (included in 26.8 and later)

clickhouse-gh · 2026-07-03T11:38:05Z

Workflow [PR], commit [58b575b]

Summary: ✅

Performance Comparison: Performance dashboard

AI Review

Summary

This PR adds a startup system.warnings entry for missing rseq, a lock-free assertion for signal-safe ProfileEvents, a sched_getcpu benchmark, and a new rseq_vs_atomic_benchmark example. The current head addresses the earlier review threads, but the benchmark evidence is still not isolated enough to support the quoted atomic vs rseq speedup because the worker threads are free to migrate between CPUs.

Findings

⚠️ Majors

[src/Common/examples/rseq_vs_atomic_benchmark.cpp:165] The benchmark claims its per-CPU modes avoid cache-line bouncing and therefore isolate xadd vs rseq, but the workers are never pinned. In the atomic path, a migration between sched_getcpu and __atomic_add_fetch makes one core update another core's slot, which reintroduces the cross-core contention the comment says is absent. The rseq path retries on migration, so the measured atomic / rseq gap can include scheduler-migration artifacts rather than just the instruction-level difference.
Suggested fix: pin each worker to a dedicated logical CPU, or add an equivalent same-CPU validation around the atomic increment before relying on this benchmark as performance evidence.

Final Verdict

Status: ⚠️ Request changes

Minimum required action: make rseq_vs_atomic_benchmark keep each worker on a fixed logical CPU (or otherwise guarantee that the slot-selection CPU matches the CPU that performs the increment), so the benchmark actually measures the contract it documents and the speedup cited in the PR body.

Without libc rseq registration (glibc < 2.35 or the glibc.pthread.rseq tunable disabled) sched_getcpu falls back to a slower path - a real syscall on AArch64 - paid on every per-CPU counter increment. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Tight micro-benchmark of per-CPU counter increment: - atomic path: `__atomic_add_fetch(&slots[sched_getcpu()].value, 1, RELAXED)`. - rseq path: `rseq_cpu_start()` + `rseq_load_cbne_store__ptr`. Same cache-aligned slot layout in both. Cache lines never bounce (each CPU's counter is touched by at most one CPU at a time), so the comparison isolates the bus-locked `xadd` vs. the rseq compare-and-store on the hot path. Usage: `clickhouse-examples rseq_vs_atomic_benchmark [--threads N] [--ops N]`. Defaults: threads=hardware_concurrency, ops=50M per thread. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

alexey-milovidov

Everything else LGTM.

The `rseq_vs_atomic_benchmark` example is Linux-only: it calls `sched_getcpu` and uses `rseq`. It was added to `clickhouse-examples` unconditionally, so a non-Linux `clickhouse-examples` build would pick up a source file it cannot compile. Gate both the source-list entry in `src/Examples/CMakeLists.txt` and the registration in `src/Examples/main.cpp` behind `OS_LINUX`, matching the nearby `thread_creation_latency` and `memory_statistics_os_perf` examples. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

The `system.warnings` message shown when `rseq` is not registered was phrased in terms internal to ClickHouse (`sched_getcpu`, "per-CPU profile counters"). Rewrite it to explain, in plain language, what `rseq` is, what ClickHouse uses it for, the performance impact of its absence, and how to enable it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

The startup warning emitted by `sanityChecks` when `PerCPU::haveRSeq` returns `false` previously advised only checking the `glibc.pthread.rseq` tunable. But `haveRSeq` also returns `false` when the runtime `glibc` is older than 2.35 (the weak `__rseq_size` symbol is absent), which is a normal, supported configuration for official builds via `GLIBC_COMPATIBILITY` (e.g. Ubuntu 20.04). On such hosts that tunable does not exist, so the advice was non-actionable and misleading. Reword the message to call out both cases: an old runtime `glibc` (the feature is unavailable, upgrade to benefit from it) versus rseq disabled via the `glibc.pthread.rseq` tunable. Addresses the AI review finding on `programs/server/Server.cpp`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

The startup `system.warnings` message for an unavailable `rseq` framed the only causes as "glibc older than 2.35" or the `glibc.pthread.rseq` tunable being disabled. But `PerCPU::haveRSeq` only probes `__rseq_size >= 8`, and `__rseq_size` is 0 whenever the runtime C library or the kernel did not register a usable rseq area - which also happens on non-glibc runtimes (e.g. the `amd_musl` build, whose `base/glibc-compatibility/musl/sched_getcpu.c` uses the same weak-symbol check), on kernels without rseq support, and when registration failed at startup. In those cases the old message pointed users at the wrong remediation (a `glibc` tunable that does not exist there). Reword the remediation to describe the condition generically - the runtime C library or the kernel did not register a usable rseq area - and list old `glibc`, non-glibc libraries such as musl, an old kernel, and the `glibc.pthread.rseq` tunable as possible causes rather than the only ones. Addresses the AI review "Request changes" verdict on this PR.

…INUX The `rseq_vs_atomic_benchmark` example includes `<rseq/rseq.h>` and uses the `librseq` API, both of which come from the `contrib/librseq` submodule. It was added to `clickhouse-examples` behind an `OS_LINUX` guard, but `contrib/CMakeLists.txt` lets `add_contrib(librseq-cmake librseq)` no-op when the `contrib/librseq` submodule is empty — which is how minimal-submodule Linux checkouts (for example `FastTest`) build. In that configuration `OS_LINUX` is still true, so the source was still added, yet `ch_contrib::librseq` (and its `<rseq/rseq.h>` include directory) did not exist, so the translation unit failed to compile. The later `if (TARGET ch_contrib::librseq)` link guard was too late to help. Gate the source on `TARGET ch_contrib::librseq` — the actual capability check — and expose the same condition to the code via a new `USE_LIBRSEQ` compile definition (`config.h`), used in both `src/Examples/main.cpp` (registration) and the benchmark itself in place of `OS_LINUX`. When `librseq` is present the generated code is identical to before; when it is absent the example is not compiled or registered. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

`_SC_NPROCESSORS_CONF` is a CPU count, not a dense upper bound for logical CPU ids, so on hosts with sparse CPU numbering `sched_getcpu` / `rseq_cpu_start` can return an id outside the slot array. Previously `atomicBump` and `rseqBump` dropped the increment in that case, making the reported ns/op artificially low and diverging from the production `ProfileEvents::Counters::fetchAdd` behavior, which falls back to row 0. Now both modes fall back to slot 0, so every loop iteration performs exactly one increment. In the rseq mode the fallback is a plain atomic add, since the rseq critical section only commits while running on the CPU it targets. Addresses AI review finding on ClickHouse#109283

The kernel keeps negative sentinels in the rseq area cpu_id field: -1 (UNINITIALIZED) and -2 (REGISTRATION_FAILED). The production sched_getcpu (base/glibc-compatibility/musl/sched_getcpu.c) rejects them before taking the rseq fast path, while BM_sched_getcpu_rseq only checked __rseq_size, so on a thread with a failed registration it would report a fast "rseq" number that does not match the mechanism sched_getcpu actually uses. Mirror the sentinel check in the availability test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…mic example `PerCPU::haveRSeq` and the `rseq_supported` probe in `rseq_vs_atomic_benchmark` treated a sufficiently large `__rseq_size` / `rseq_size` as proof of a usable rseq area, but the kernel uses negative sentinels in `cpu_id` (-1 UNINITIALIZED, -2 REGISTRATION_FAILED) and the production `sched_getcpu` (base/glibc-compatibility/musl/sched_getcpu.c) rejects them before taking the rseq fast path. Mirror that check in both places, like `benchmark_sched_getcpu` already does, so the `system.warnings` entry fires whenever the slow fallback is in use and the example never reports an rseq timing that does not measure a usable fast path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

# Conflicts: # src/Common/PerCPU.cpp

Address review: `_SC_NPROCESSORS_CONF` is a CPU count, not an upper bound for logical CPU ids, so on hosts with sparse CPU numbering part of the supposedly per-CPU workload collapsed into the shared `slots[0]` fallback, mixing cross-core contention into the reported `atomic`/`rseq` numbers. Use `rseq_get_max_nr_cpus` (librseq parses "/sys/devices/system/cpu/possible", falling back to sysconf) so every `sched_getcpu`/`rseq_cpu_start` value maps to its own slot; the slot-0 fallbacks remain only as guards for error sentinels.

clickhouse-gh · 2026-07-25T11:13:39Z

+
+    for (int t = 0; t < opts.threads; ++t)
+    {
+        workers.emplace_back([&, t]()


rseq_vs_atomic_benchmark promises that the per-CPU modes keep each slot private to one CPU, but the workers are never pinned. In the atomic path we do sched_getcpu and then __atomic_add_fetch; if the scheduler migrates the thread in that window, one core updates another core's slot and reintroduces the cache-line bouncing this benchmark says it removed. The rseq path rejects that migration and retries, so the reported atomic / rseq delta can partly measure scheduler-migration artifacts rather than just xadd vs rseq_load_cbne_store__ptr.

Please pin each worker to a dedicated logical CPU (or otherwise validate that the increment ran on the same CPU that selected the slot) before using this benchmark as evidence for the speedup quoted in the PR description.

clickhouse-gh · 2026-07-25T15:07:31Z

LLVM Coverage Report

Metric	Baseline	Current	Δ
Lines	86.20%	86.20%	+0.00%
Functions	92.00%	92.00%	+0.00%
Branches	78.40%	78.40%	+0.00%

Changed lines: Changed C/C++ lines covered: 10/24 (41.67%) · Uncovered code

Full report · Diff report

azat mentioned this pull request Jul 3, 2026

Make Profile Events ±30x faster by using Per CPU atomics for server-wide and users #105056

Merged

clickhouse-gh Bot added the pr-not-for-changelog This PR should not be mentioned in the changelog label Jul 3, 2026

azat and others added 4 commits July 3, 2026 15:23

Make sure that atomic is lock-free for incrementSignalSafe()

fd74460

Add benchmark for sched_getcpu mechanisms (rseq, vDSO, syscall)

455bb00

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

azat force-pushed the per-cpu-improvements branch from b27d5bc to f351072 Compare July 3, 2026 13:24

clickhouse-gh Bot reviewed Jul 3, 2026

View reviewed changes

Comment thread src/Examples/CMakeLists.txt Outdated

alexey-milovidov requested changes Jul 5, 2026

View reviewed changes

Comment thread programs/server/Server.cpp

alexey-milovidov self-assigned this Jul 5, 2026

alexey-milovidov reviewed Jul 5, 2026

View reviewed changes

alexey-milovidov and others added 2 commits July 5, 2026 05:40

clickhouse-gh Bot reviewed Jul 5, 2026

View reviewed changes

Comment thread programs/server/Server.cpp Outdated

clickhouse-gh Bot reviewed Jul 5, 2026

View reviewed changes

Comment thread programs/server/Server.cpp Outdated

alexey-milovidov added 3 commits July 5, 2026 08:17

Merge remote-tracking branch 'origin/master' into per-cpu-improvements

3426809

Merge remote-tracking branch 'origin/master' into per-cpu-improvements

ecf1127

clickhouse-gh Bot reviewed Jul 19, 2026

View reviewed changes

Comment thread src/Examples/CMakeLists.txt

clickhouse-gh Bot reviewed Jul 20, 2026

View reviewed changes

Comment thread src/Common/examples/rseq_vs_atomic_benchmark.cpp Outdated

clickhouse-gh Bot reviewed Jul 20, 2026

View reviewed changes

Comment thread src/Common/benchmarks/sched_getcpu.cpp Outdated

clickhouse-gh Bot reviewed Jul 20, 2026

View reviewed changes

Comment thread src/Common/PerCPU.cpp Outdated

Comment thread src/Common/examples/rseq_vs_atomic_benchmark.cpp Outdated

alexey-milovidov and others added 2 commits July 20, 2026 15:36

Merge remote-tracking branch 'origin/master' into per-cpu-improvements

de41a66

# Conflicts: # src/Common/PerCPU.cpp

clickhouse-gh Bot reviewed Jul 24, 2026

View reviewed changes

Comment thread src/Common/examples/rseq_vs_atomic_benchmark.cpp Outdated

alexey-milovidov added 3 commits July 24, 2026 14:34

Merge remote-tracking branch 'origin/master' into per-cpu-improvements

f1060da

Merge remote-tracking branch 'origin/master' into per-cpu-improvements

58b575b

clickhouse-gh Bot reviewed Jul 25, 2026

View reviewed changes

alexey-milovidov approved these changes Jul 25, 2026

View reviewed changes

alexey-milovidov added this pull request to the merge queue Jul 25, 2026

Merged via the queue into ClickHouse:master with commit 9c29cdc Jul 25, 2026
178 checks passed

robot-ch-test-poll4 added the pr-synced-to-cloud The PR is synced to the cloud repo label Jul 25, 2026

tiandiwonder mentioned this pull request Aug 1, 2026

ProfileEvents per-CPU sharding calls getCurrentCPU unconditionally — a SYS_getcpu syscall per increment on AArch64 without rseq #112894

Open

groeneai mentioned this pull request Aug 25, 2026

Fix compilation of benchmark_statistics with ENABLE_BENCHMARKS=ON #116292

Merged

Navigation Menu

Uh oh!

Benchmarks, static assertions, system.warnings for per CPU - #109283

Benchmarks, static assertions, system.warnings for per CPU#109283
alexey-milovidov merged 18 commits into
ClickHouse:masterfrom
azat:per-cpu-improvements

azat commented Jul 3, 2026 •

edited by robot-ch-test-poll3

Loading

Uh oh!

clickhouse-gh Bot commented Jul 3, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

alexey-milovidov left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

clickhouse-gh Bot Jul 25, 2026

Uh oh!

clickhouse-gh Bot commented Jul 25, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

azat commented Jul 3, 2026 • edited by robot-ch-test-poll3 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changelog category (leave one):

Version info

Uh oh!

clickhouse-gh Bot commented Jul 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

AI Review

Summary

Findings

Final Verdict

Uh oh!

Uh oh!

Uh oh!

alexey-milovidov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

clickhouse-gh Bot Jul 25, 2026

Choose a reason for hiding this comment

Uh oh!

clickhouse-gh Bot commented Jul 25, 2026

LLVM Coverage Report

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

azat commented Jul 3, 2026 •

edited by robot-ch-test-poll3

Loading

clickhouse-gh Bot commented Jul 3, 2026 •

edited

Loading