Skip to content

feat: use instrument-hook markers in walltime#118

Open
GuillaumeLagrange wants to merge 1 commit intomasterfrom
use-benchmark-start-end-markers
Open

feat: use instrument-hook markers in walltime#118
GuillaumeLagrange wants to merge 1 commit intomasterfrom
use-benchmark-start-end-markers

Conversation

@GuillaumeLagrange
Copy link
Copy Markdown
Contributor

No description provided.

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented May 6, 2026

Merging this PR will degrade performance by 30.5%

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 16 improved benchmarks
❌ 40 regressed benchmarks
✅ 221 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
WallTime test_sum_squares_slow 307.4 µs 345.7 µs -11.09%
WallTime test_sum_of_squares[sum_of_squares_sum_comprehension_product] 193.4 µs 198.4 µs -2.51%
WallTime test_generate_all_combinations[5-4] 10.2 µs 10.9 µs -6.86%
WallTime test_iir_filter_process 3 µs 3.1 µs -3.79%
WallTime test_sum_of_squares[sum_of_squares_for_loop_product] 160.8 µs 190.3 µs -15.51%
WallTime test_make_lowpass 5.4 µs 5.5 µs -2.84%
WallTime test_sudoku[initial_grid0] 7.8 µs 8.3 µs -6.45%
WallTime test_process_creation[cat /dev/null] 5.5 ms 5.3 ms +4.02%
WallTime test_array_alloc[10000] 20.5 µs 21 µs -2.35%
WallTime test_pipe_communication[10000] 12.7 ms 8.9 ms +42.37%
WallTime test_make_peak 6.2 µs 6.8 µs -8.56%
WallTime test_match_word_pattern[aba-GraphTreesGraph] 103.2 µs 105.4 µs -2.04%
WallTime test_fs_write[10000] 43.7 µs 44.9 µs -2.69%
WallTime test_threadpool_map[100000] 3.8 s 3.9 s -2.64%
WallTime test_recursive_fibo_20 5.8 ms 5.6 ms +4.35%
WallTime test_sum_of_squares[sum_of_squares_sum_labmda_power] 317.4 µs 326.6 µs -2.82%
WallTime test_multiprocessing_map[1000] 57.9 ms 56.1 ms +3.3%
WallTime test_fs_write[1000] 17.7 µs 18.4 µs -3.48%
WallTime test_hostname_resolution[8.8.8.8] 7.9 µs 8.1 µs -2.89%
WallTime test_combination_sum[candidates0-8] 10.9 µs 11.8 µs -7.81%
... ... ... ... ... ...

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


Comparing use-benchmark-start-end-markers (ae47707) with master (2dc7398)

Open in CodSpeed

@GuillaumeLagrange GuillaumeLagrange force-pushed the use-benchmark-start-end-markers branch from 754fb43 to ae47707 Compare May 6, 2026 16:00
Copy link
Copy Markdown
Member

@not-matthias not-matthias left a comment

Choose a reason for hiding this comment

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

LGTM overall, but we shouldn't modify the time computation.

Comment on lines +239 to 254
if hooks:
start = hooks.current_timestamp()
else:
start = perf_counter_ns()

for _ in iter_range:
__codspeed_root_frame__()
end = perf_counter_ns()

if hooks:
end = hooks.current_timestamp()
hooks.current_timestamp()
hooks.add_benchmark_timestamps(start, end)
else:
end = perf_counter_ns()

times_per_round_ns.append(end - start)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

we shouldn't move the perf_counter_ns -> it's the measuring code

We need to measure both the time AND define our markers

Comment on lines +310 to +323
if hooks:
start = hooks.current_timestamp()
else:
start = perf_counter_ns()

for _ in iter_range:
__codspeed_root_frame__(*args, **kwargs)
end = perf_counter_ns()

if hooks:
end = hooks.current_timestamp()
hooks.add_benchmark_timestamps(start, end)
else:
end = perf_counter_ns()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

same here

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.

2 participants