Skip to content

Conversation

not-matthias
Copy link
Member

@not-matthias not-matthias commented Oct 9, 2025

Things I changed:

  • rust-bindgen: Invoke manually with the right args to only generate for our functions -> otherwise assertions for unrelated things will fail
  • Fallback to noop implementation when build fails (similar to pytest-codspeed)
  • Build with optimizations (removes a warning)
  • Replaced gnu17 with c11, otherwise it causes compilation errors when not supported.
  • Added cargo-cross tests

Copy link

codspeed-hq bot commented Oct 9, 2025

CodSpeed Performance Report

Merging #137 will degrade performances by 70%

Comparing cod-1483-error-building-codspeed-rust-with-clang (898484d) with main (27c0199)

Summary

⚡ 26 improvements
❌ 24 (👁 1) regressions
✅ 313 untouched

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

Benchmarks breakdown

Mode Benchmark BASE HEAD Change
Instrumentation Iterative 132.5 ns 161.7 ns -18.04%
WallTime Recursive 50.6 µs 45.6 µs +10.91%
Instrumentation Iterative[20] 132.5 ns 161.7 ns -18.04%
Instrumentation Iterative[21] 134.2 ns 163.3 ns -17.86%
WallTime Recursive[20] 50.5 µs 45.7 µs +10.45%
WallTime Recursive[21] 82 µs 74.5 µs +10.07%
WallTime small_drop 50 ns 60 ns -16.67%
WallTime large_setup 18 ns 14 ns +28.57%
👁 WallTime iter_with_setup 36 ns 44 ns -18.18%
WallTime iter_batched_large_input 36 ns 44 ns -18.18%
WallTime iter_batched_per_iteration 36 ns 48 ns -25%
WallTime iter_batched_ref_large_input 36 ns 44 ns -18.18%
WallTime iter_batched_ref_per_iteration 36 ns 40 ns -10%
WallTime iter_with_setup 36 ns 44 ns -18.18%
WallTime from_elem[16384] 891 ns 928 ns -3.99%
WallTime from_elem[4096] 371 ns 351 ns +5.7%
WallTime from_elem[8192] 550 ns 520 ns +5.77%
WallTime from_elem_decimal[2048] 261 ns 241 ns +8.3%
WallTime init_array[1000] 2.1 µs 1.9 µs +12.51%
Instrumentation fib_10 830.6 ns 859.7 ns -3.39%
... ... ... ... ... ...

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

@not-matthias not-matthias force-pushed the cod-1483-error-building-codspeed-rust-with-clang branch 3 times, most recently from e9e1f47 to 6c7b453 Compare October 13, 2025 08:57
@not-matthias not-matthias requested a review from Copilot October 13, 2025 12:32
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 compilation issues when building the codspeed crate with clang by addressing compiler-specific warnings and improving CI testing for different compilers.

  • Adds compiler warning suppressions for C code generated from Zig
  • Replaces specific tests with build-only validation for macOS and Windows
  • Expands CI matrix to test both GCC and Clang compilers on Linux

Reviewed Changes

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

File Description
crates/codspeed/build.rs Adds warning suppression flags and uses clang-compatible flag handling
.github/workflows/ci.yml Updates CI matrix to test GCC/Clang separately and changes to build-only for unsupported platforms

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

@not-matthias not-matthias force-pushed the cod-1483-error-building-codspeed-rust-with-clang branch 4 times, most recently from 98dfc53 to d76e6a1 Compare October 14, 2025 12:18
@not-matthias not-matthias requested a review from Copilot October 14, 2025 12:22
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

Copilot reviewed 6 out of 7 changed files in this pull request and generated 1 comment.


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

@not-matthias not-matthias force-pushed the cod-1483-error-building-codspeed-rust-with-clang branch from d76e6a1 to 817b416 Compare October 14, 2025 12:25
@not-matthias not-matthias requested a review from art049 October 14, 2025 12:42
@not-matthias not-matthias force-pushed the cod-1483-error-building-codspeed-rust-with-clang branch 3 times, most recently from 97933b1 to d08b3c3 Compare October 15, 2025 15:23
Copy link
Member

@art049 art049 left a comment

Choose a reason for hiding this comment

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

Looks good but can you add a check in the CI making sure the bindings generated are up to date?

As well maybe a small section explaining how to generate the bindings in the README/contributing?

@not-matthias not-matthias force-pushed the cod-1483-error-building-codspeed-rust-with-clang branch 2 times, most recently from e40c3c9 to 26140ba Compare October 17, 2025 09:21
@not-matthias
Copy link
Member Author

Looks good but can you add a check in the CI making sure the bindings generated are up to date?

As well maybe a small section explaining how to generate the bindings in the README/contributing?

Added a check in the lint step to regenerate the bindings and fail if they are different. I added the script in the same dir as bindings.rs and added a comment in the Rust code. I feel like it doesn't make that much sense to add it to the README/CONTRIBUTING since it's much harder to discover and not really linked to the code.

@not-matthias not-matthias requested a review from art049 October 17, 2025 09:29
Copy link
Member

@art049 art049 left a comment

Choose a reason for hiding this comment

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

lgtm!

@art049
Copy link
Member

art049 commented Oct 18, 2025

If I'm correct @not-matthias this also fixes #139 right?

@not-matthias
Copy link
Member Author

If I'm correct @not-matthias this also fixes #139 right?

Yes, I've added tests for cargo-cross to ensure it works

@not-matthias not-matthias force-pushed the cod-1483-error-building-codspeed-rust-with-clang branch from 26140ba to 898484d Compare October 20, 2025 09:24
@not-matthias not-matthias merged commit 898484d into main Oct 20, 2025
25 of 26 checks passed
@not-matthias not-matthias deleted the cod-1483-error-building-codspeed-rust-with-clang branch October 20, 2025 09:44
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