fix: perf instrumentation not available for cpp codspeed

[not-matthias]

Moved from #13, so that it's linked to the correct Linear issue.

Blocked by CodSpeedHQ/instrument-hooks#6

[codspeed-hq]

CodSpeed Instrumentation Performance Report

Merging #17 will improve performances by ×2.5

_{Comparing cod-1040-perf-instrumentation-not-available-for-cpp-codspeed (c943b12) with main (835fcda)}

Summary

⚡ 25 improvements
✅ 17 untouched benchmarks
🆕 20 new benchmarks

Benchmarks breakdown

	Benchmark	BASE	HEAD	Change
🆕	BM_FibonacciIterative[50]	N/A	174.4 ns	N/A
🆕	BM_FibonacciRecursive[35]	N/A	100.9 ms	N/A
⚡	BarTest[MyFixture]	122.2 ns	61.9 ns	+97.31%
⚡	DoubleTest[MyTemplatedFixture, double]	122.2 ns	61.9 ns	+97.31%
⚡	FooTest[MyFixture]	122.2 ns	61.9 ns	+97.31%
⚡	IntTest[MyTemplatedFixture, int]	122.2 ns	61.9 ns	+97.31%
⚡	TestA[MyTemplate1, int]	122.2 ns	61.9 ns	+97.31%
⚡	TestB[MyTemplate2, int, double]	122.2 ns	61.9 ns	+97.31%
⚡	BM_Capture[int_string_test]	121.4 ns	61.4 ns	+97.74%
⚡	BM_Capture[int_test]	121.4 ns	61.4 ns	+97.74%
⚡	BM_memcpy[64]	334.4 ns	303.3 ns	+10.26%
⚡	BM_memcpy[8]	305.6 ns	274.4 ns	+11.34%
⚡	BM_rand_vector	182.8 ns	122.8 ns	+48.87%
🆕	BM_sleep_100ms	N/A	436.7 ns	N/A
🆕	BM_sleep_100us	N/A	436.7 ns	N/A
🆕	BM_sleep_10ms	N/A	436.7 ns	N/A
🆕	BM_sleep_10us	N/A	436.7 ns	N/A
🆕	BM_sleep_1ms	N/A	436.7 ns	N/A
🆕	BM_sleep_1us	N/A	436.7 ns	N/A
🆕	BM_sleep_50ms	N/A	436.7 ns	N/A
...	...	...	...	...

ℹ️ Only the first 20 benchmarks are displayed. Go to the app to view all benchmarks.

[codspeed-hq]

CodSpeed WallTime Performance Report

Merging #17 will not alter performance

_{Comparing cod-1040-perf-instrumentation-not-available-for-cpp-codspeed (c943b12) with main (835fcda)}

Summary

✅ 42 untouched benchmarks
🆕 20 new benchmarks

Benchmarks breakdown

	Benchmark	BASE	HEAD	Change
🆕	BM_FibonacciIterative[50]	N/A	62.2 ns	N/A
🆕	BM_FibonacciRecursive[35]	N/A	66.6 ms	N/A
🆕	BM_sleep_100ms	N/A	100.1 ms	N/A
🆕	BM_sleep_100us	N/A	158.5 µs	N/A
🆕	BM_sleep_10ms	N/A	10.1 ms	N/A
🆕	BM_sleep_10us	N/A	68.4 µs	N/A
🆕	BM_sleep_1ms	N/A	1.1 ms	N/A
🆕	BM_sleep_1us	N/A	59.2 µs	N/A
🆕	BM_sleep_50ms	N/A	50.1 ms	N/A
🆕	BM_sleep_50us	N/A	108.5 µs	N/A
🆕	BM_FibonacciIterative[50]	N/A	62.2 ns	N/A
🆕	BM_FibonacciRecursive[35]	N/A	66.6 ms	N/A
🆕	BM_sleep_100ms	N/A	100.1 ms	N/A
🆕	BM_sleep_100us	N/A	158.5 µs	N/A
🆕	BM_sleep_10ms	N/A	10.1 ms	N/A
🆕	BM_sleep_10us	N/A	68.5 µs	N/A
🆕	BM_sleep_1ms	N/A	1.1 ms	N/A
🆕	BM_sleep_1us	N/A	59.5 µs	N/A
🆕	BM_sleep_50ms	N/A	50.1 ms	N/A
🆕	BM_sleep_50us	N/A	108.6 µs	N/A

[GuillaumeLagrange]

We also have performance regression introduced on instrumented test, I'm guessing this is due to the instrument-hooks library introduction, could we hunt changes to make them either non existant or as minimal as possible ?

Callgraphs show that we pickup instrumentation in callgrind in a way that we did not before, if we don't change thsi it will be a breaking change: https://codspeed.io/CodSpeedHQ/codspeed-cpp/branches/cod-1040-perf-instrumentation-not-available-for-cpp-codspeed?runnerMode=Instrumentation

[art049]

The regression is quite problematic since it appears to introduce a significant amount of overhead here. For interpreted languages, it doesn't make a big difference, but here, it seems to be substantial for micro benchmarks (even if the ones with regressions are very small). And we'll probably encounter the same issue when we port this to codspeed-rust.

[art049]

Still a few changes left

[not-matthias]

The benchmarks show the correct timings now. We're not optimizing away the memcpy and some benchmarks even have improved performance, since we're placing the measurement_stop closer to the benchmark (so we're not measuring extra instructions)

[GuillaumeLagrange]

olgtm if becnhmark results don't vary to much, but this is still a breaking change

[GuillaumeLagrange]

this will have to be a major release because we are essentially changing benchmark result values for end users.

I agree that this is better here, but we'll have to keep this in mind

[art049]

We have significant deltas on the following walltime benches:

🆕 BM_sleep_100ns N/A 58.8 µs N/A
🆕 BM_sleep_100us N/A 158.7 µs N/A
🆕 BM_sleep_10ms N/A 10.1 ms N/A
🆕 BM_sleep_1ms N/A 1.1 ms N/A
🆕 BM_sleep_1ns N/A 58.6 µs N/A
🆕 BM_sleep_1us N/A 59.6 µs N/A
🆕 BM_sleep_100ns N/A 58.6 µs N/A
🆕 BM_sleep_100us N/A 158.5 µs N/A
🆕 BM_sleep_1ns N/A 58.5 µs N/A
🆕 BM_sleep_1us N/A 59.5 µs N/A

Is this expected? Do we have a resolution issue? @not-matthias

[art049]

Also it seems we have significant issues with instrumentation. This time the benchmarks are getting twice as fast, this seems to be an issue.

We need to be sure of what's happening both for the walltime issue and the instrumentation

[not-matthias]

We have significant deltas on the following walltime benches:

🆕 BM_sleep_100ns N/A 58.8 µs N/A
🆕 BM_sleep_100us N/A 158.7 µs N/A
🆕 BM_sleep_10ms N/A 10.1 ms N/A
🆕 BM_sleep_1ms N/A 1.1 ms N/A
🆕 BM_sleep_1ns N/A 58.6 µs N/A
🆕 BM_sleep_1us N/A 59.6 µs N/A
🆕 BM_sleep_100ns N/A 58.6 µs N/A
🆕 BM_sleep_100us N/A 158.5 µs N/A
🆕 BM_sleep_1ns N/A 58.5 µs N/A
🆕 BM_sleep_1us N/A 59.5 µs N/A

Is this expected? Do we have a resolution issue? @not-matthias

We are not calculating this ourselves, but rather take the measurements from Google Benchmark (@GuillaumeLagrange pls correct me if i'm wrong). So my assumption is that it's also measuring parts of the benchmark framework which we don't have that much control over.

EDIT: I removed the nanosecond benchmarks since they are just too short to be measured correctly (we also don't have them in Rust). Even the microsecond benchmarks are quite unstable in Rust:

• 100us: Rust
• 1us: Rust
• 100ns: Rust
• For C++ it's roughly 50-100%

I tried to time it manually using this code and still got 150us for a 100us sleep:

auto start = std::chrono::high_resolution_clock::now();
std::this_thread::sleep_for(std::chrono::microseconds(100));
auto end = std::chrono::high_resolution_clock::now();

Also it seems we have significant issues with instrumentation. This time the benchmarks are getting twice as fast, this seems to be an issue.

We need to be sure of what's happening both for the walltime issue and the instrumentation

The instrumentation speedup is expected, since we moved the measurement_stop closer to the benchmark end which means we're not measuring additional unrelated instructions.