Skip to content

Conversation

not-matthias
Copy link
Member

No description provided.

Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes performance measurement accuracy by moving the BenchGuard to only capture the actual benchmark execution time, excluding the overhead from input/output dropping operations.

  • Removed the global BenchGuard that was capturing the entire benchmarking loop including drop operations
  • Added localized BenchGuard instances around the core timing sections to measure only the benchmark execution
  • Added a test benchmark to demonstrate the issue with large input drops

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
crates/divan_compat/divan_fork/src/bench/mod.rs Moved BenchGuard placement from outer loop to inner timing sections to exclude drop overhead
crates/divan_compat/benches/drop_example.rs Added test benchmark with large input that has expensive drop to verify the fix
crates/divan_compat/Cargo.toml Added bench configuration for the new drop_example benchmark

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link

codspeed-hq bot commented Sep 16, 2025

CodSpeed WallTime Performance Report

Merging #118 will degrade performances by 16.49%

Comparing cod-1344-investigate-benchmark-time-in-drop_in_place-and-perf (b02554d) with main (0565866)

Summary

⚡ 14 improvements
❌ 21 regressions
✅ 119 untouched
🆕 11 new

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

Benchmarks breakdown

Benchmark BASE HEAD Change
large_drop 163.4 µs 168.9 µs -3.27%
large_setup 14 ns 15 ns -6.67%
from_elem[16384] 867 ns 948 ns -8.54%
from_elem[2048] 243 ns 258 ns -5.81%
from_elem[8192] 514 ns 534 ns -3.75%
from_elem_decimal[2048] 248 ns 264 ns -6.06%
bench_array1[10] 10 ns 9 ns +11.11%
bench_array2[10] 10 ns 9 ns +11.11%
init_array[1000] 1.8 µs 2 µs -9.76%
fibo_10 2 ns 1 ns ×2
🆕 bench_large_input N/A 50.5 ms N/A
🆕 bench_large_input_local N/A 50.4 ms N/A
🆕 fib_in_thread[30] N/A 6.8 ms N/A
🆕 fib_in_thread[31] N/A 10.9 ms N/A
🆕 fib_in_thread[32] N/A 17.5 ms N/A
🆕 fib_in_thread_bench[30] N/A 6.8 ms N/A
🆕 fib_in_thread_bench[31] N/A 10.9 ms N/A
🆕 fib_in_thread_bench[32] N/A 17.5 ms N/A
🆕 fib_in_thread_bench_local[30] N/A 6.8 ms N/A
🆕 fib_in_thread_bench_local[31] N/A 10.9 ms N/A
... ... ... ... ...

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

Copy link

codspeed-hq bot commented Sep 16, 2025

CodSpeed Instrumentation Performance Report

Merging #118 will degrade performances by 6.78%

Comparing cod-1344-investigate-benchmark-time-in-drop_in_place-and-perf (b02554d) with main (0565866)

Summary

⚡ 5 improvements
❌ 5 regressions
✅ 157 untouched
🆕 16 new

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

Benchmarks breakdown

Benchmark BASE HEAD Change
sleep_100ms 595.3 ns 624.4 ns -4.67%
sleep_10ms 566.1 ns 595.3 ns -4.9%
sleep_1ms 566.1 ns 595.3 ns -4.9%
sleep_50ms 595.3 ns 624.4 ns -4.67%
bench_array1[10] 254.2 ns 225 ns +12.96%
bench_array2[10] 254.2 ns 225 ns +12.96%
mut_borrow 924.2 ns 895 ns +3.26%
🆕 bench_large_input N/A 1.6 ms N/A
🆕 bench_large_input_local N/A 1.5 ms N/A
🆕 sleep_100ms N/A 566.1 ns N/A
🆕 sleep_100ms_with_custom_sample N/A 566.1 ns N/A
🆕 sleep_10ms N/A 566.1 ns N/A
🆕 sleep_1ms N/A 566.1 ns N/A
🆕 sleep_50ms N/A 566.1 ns N/A
🆕 fib_in_thread[30] N/A 9.7 ms N/A
🆕 fib_in_thread[31] N/A 15.8 ms N/A
🆕 fib_in_thread[32] N/A 25.5 ms N/A
🆕 fib_in_thread_bench[30] N/A 9.7 ms N/A
🆕 fib_in_thread_bench[31] N/A 15.8 ms N/A
🆕 fib_in_thread_bench[32] N/A 25.5 ms N/A
... ... ... ... ...

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

@not-matthias not-matthias force-pushed the cod-1344-investigate-benchmark-time-in-drop_in_place-and-perf branch 3 times, most recently from 8943507 to 8b522b2 Compare September 17, 2025 12:22
@not-matthias not-matthias force-pushed the cod-1344-investigate-benchmark-time-in-drop_in_place-and-perf branch from b02554d to b21bf84 Compare September 22, 2025 18:00
@not-matthias
Copy link
Member Author

Closing in favor of #121

@not-matthias not-matthias deleted the cod-1344-investigate-benchmark-time-in-drop_in_place-and-perf branch September 26, 2025 08:28
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.

1 participant