Faster (almost 2x) mutexes (was slower due to ThreadFuzzer) #60823
Conversation
Recently I noticed that DB::injection() pops up in perf top, so let's optimize it slightly: - Add -fomit-frame-pointer -momit-leaf-frame-pointer explicitly -- almost 0 effect - Add ALWAYS_INLINE for ThreadFuzzer::isStarted() (just in case) - Disable ThreadFuzzer if non of env variables had been set, this is needed to avoid extra checks in DB::injection() - Add ALWAYS_INLINE for ThreadFuzzer::injection() And here are some results for ThreadFuzzer test: - before: elapsed 6.27368 / test time 654 ms - after: elapsed 3.14167 / test time 325 ms - disabled: elapsed 2.46666 / test time 263 ms *But note, it is still slower then with ThreadFuzzer disabled.* Note, that this numbers for AMD 5975WX, for server with 2x Xeon Silver 4216 2.10: - before: elapsed 12.109 / test time 1325 ms - after: elapsed 10.506 / test time 1101 ms - disabled: elapsed 8.41043 / test time 917 ms P.S. I've also tried with the same glibc version as server had - zero changes. Refs: https://gist.github.com/azat/51a5fcc3a40af9f678906a3a6e14e079 Signed-off-by: Azat Khuzhin <[email protected]>
robot-ch-test-poll4
added
the
pr-performance
|
This is an automated comment for commit d5825ec with description of existing statuses. It's updated for the latest CI running ❌ Click here to open a full report in a separate page Successful checks
|
|
Ok. Where do we have most of the mutex contention? |
|
There are few very common loggers (per-table), initially I looked into this while looking at #59531 And with this patch I got:
While before it was:
With empty
With ThreadFuzzer completely compiled out:
Without mutations:
|
|
a little silly question: it shouldn't be enabled on servers running prod workloads, so what are we optimising? |
It is - ClickHouse/src/Common/ThreadFuzzer.cpp Line 26 in af3b16a Since the same binary used for CI and for releases |
|
Yes, we should run the same binary in production and in CI. |
robot-ch-test-poll
added
the
pr-synced-to-cloud
| @@ -172,6 +176,8 @@ void ThreadFuzzer::stop() | |||
|
|
|||
| void ThreadFuzzer::start() | |||
| { | |||
| if (!instance().isEffective()) | |||
There was a problem hiding this comment.
start() checks for isEffective, and isEffective checks for isStarted(). Result: Impossible to start it. So fast 😄
There was a problem hiding this comment.
Some consequences:
SYSTEM START THREAD FUZZERis useless. Once the fuzzer is stopped it can't start.- If you don't use
THREAD_FUZZER_CPU_TIME_PERIOD_USsleep faults won't work because isEffective will return false.
There was a problem hiding this comment.
start() checks for isEffective, and isEffective checks for isStarted(). Result: Impossible to start it. So fast 😄
What a shame... But it should affect only SYSTEM START THREAD FUZZER, while the overhead was there all the time.
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Faster (almost 2x) mutexes (was slower due to ThreadFuzzer)
Recently I noticed that DB::injection() pops up in perf top, so let's optimize it slightly:
And here are some results for ThreadFuzzer test:
But note, it is still slower then with ThreadFuzzer disabled.
Note, that this numbers for AMD 5975WX, for server with 2x Xeon Silver 4216 2.10:
P.S. I've also tried with the same glibc version as server had - zero changes.
Refs: https://gist.github.com/azat/51a5fcc3a40af9f678906a3a6e14e079