Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cod 711 file name missing in codspeed rust flamegraph #93

Merged

Conversation

GuillaumeLagrange
Copy link
Contributor

  • Root cause was missing debug symbols, because cargo seems to strip them by default if they are not explicitly enabled in Cargo.toml and only through rust flags (this is a theory, have not dived into cargo code)
  • Noticed that fibo(500) is actually a very huge and overflowing integer when troubleshooting benches without release profile, so fixed it

Verified

This commit was signed with the committer’s verified signature.
clintonb Clinton Blackburn
…symbols
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 addresses missing debug symbols and integer overflow issues in benchmarks.

  • Update fibonacci functions’ parameter and return types to u64 to prevent overflow.
  • Rename the benchmark function from fibo_500 to fibo_50 to reflect the adjusted test value.
  • Disable debug info stripping in the build script to preserve symbols in release builds.

Reviewed Changes

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

File Description
crates/divan_compat/benches/basic_example.rs Adjusts fibonacci function types and benchmark values to prevent overflow.
crates/cargo-codspeed/src/build.rs Adds a rust flag to disable stripping of debug symbols in release builds.
Comments suppressed due to low confidence (2)

crates/divan_compat/benches/basic_example.rs:17

  • [nitpick] The function was renamed from fibo_500 to fibo_50, which may be confusing without context. Consider adding a comment or updating the function name to clearly reflect the new benchmark parameters.
fn fibo_50() -> u64 {

crates/cargo-codspeed/src/build.rs:125

  • [nitpick] Disabling stripping of debug info via rust flags is a workaround for Cargo's release profile settings. Please verify that this flag does not conflict with other settings or introduces issues in different build environments.
rust_flags.push_str(" -C strip=none");

Copy link

codspeed-hq bot commented Mar 28, 2025

CodSpeed Instrumentation Performance Report

Merging #93 will degrade performances by 18.88%

Comparing cod-711-file-name-missing-in-codspeed-rust-flamegraph (d7a1b4b) with main (250ad31)

Summary

⚡ 3 improvements
❌ 3 regressions
✅ 156 untouched benchmarks
🆕 1 new benchmarks
⁉️ 1 dropped benchmarks

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

Benchmarks breakdown

Benchmark BASE HEAD Change
bench_array1[10] 195.8 ns 225 ns -12.96%
bench_array1[1] 119.2 ns 90 ns +32.41%
bench_array2[10] 195.8 ns 225 ns -12.96%
bench_array2[1] 119.2 ns 90 ns +32.41%
bench_array2[4] 125.3 ns 154.4 ns -18.88%
fibo_10 119.2 ns 90 ns +32.41%
🆕 fibo_50 N/A 90.3 ns N/A
⁉️ fibo_500 257.8 ns N/A N/A

Copy link

codspeed-hq bot commented Mar 28, 2025

CodSpeed Walltime Performance Report

Merging #93 will degrade performances by 53.23%

Comparing cod-711-file-name-missing-in-codspeed-rust-flamegraph (d7a1b4b) with main (250ad31)

Summary

⚡ 6 improvements
❌ 11 regressions
✅ 127 untouched benchmarks
🆕 1 new benchmarks
⁉️ 1 dropped benchmarks

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

Benchmarks breakdown

Benchmark BASE HEAD Change
build_vec 210 ns 219 ns -4.11%
from_elem[4096] 316 ns 330 ns -4.24%
bench_array1[1] 4.5 µs 3.6 µs +25.08%
bench_array2[1] 4.5 µs 3.6 µs +25.08%
init_array[1000] 7.1 µs 2 µs ×3.5
fibo_10 3.6 µs 4.5 µs -19.79%
🆕 fibo_50 N/A 4.5 µs N/A
⁉️ fibo_500 7.1 µs N/A N/A
recursive[10] 6.4 µs 6.6 µs -3.44%
generate_combinations[6] 2.7 µs 1.3 µs ×2
generate_parentheses[5] 20.8 µs 21.9 µs -5.36%
graph_coloring[3] 1 µs 2.2 µs -53.23%
graph_coloring[5] 3.9 µs 4.5 µs -13.65%
graph_coloring[6] 4.3 µs 4.8 µs -11.67%
hamiltonian_cycle[4] 4.5 µs 4.4 µs +3%
n_queens_solver[5] 9.4 µs 9.1 µs +3.04%
rat_in_maze[8] 3.7 µs 3.9 µs -3.41%
add_two_integers[(0, 0)] 5.6 µs 5.9 µs -5.47%
generate_gray_code[2] 3.8 µs 3.9 µs -3.4%

@GuillaumeLagrange GuillaumeLagrange merged commit d7a1b4b into main Mar 31, 2025
5 of 6 checks passed
@GuillaumeLagrange GuillaumeLagrange deleted the cod-711-file-name-missing-in-codspeed-rust-flamegraph branch March 31, 2025 08:10
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.

None yet

2 participants