Skip to content

Conversation

not-matthias
Copy link
Member

No description provided.

@not-matthias not-matthias force-pushed the simplify-instrument-hooks branch 2 times, most recently from 607879e to 2561d9f Compare October 3, 2025 16:29
Copy link

codspeed-hq bot commented Oct 3, 2025

CodSpeed Instrumentation Performance Report

Merging #132 will not alter performance

Comparing simplify-instrument-hooks (8054636) with main (3de526b)

Summary

✅ 186 untouched

Copy link

codspeed-hq bot commented Oct 3, 2025

CodSpeed WallTime Performance Report

Merging #132 will degrade performances by 50%

Comparing simplify-instrument-hooks (8054636) with main (3de526b)

Summary

⚡ 18 improvements
❌ 13 regressions
✅ 137 untouched

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

Benchmarks breakdown

Benchmark BASE HEAD Change
build_vec 223 ns 208 ns +7.21%
Recursive 52.5 µs 47.4 µs +10.74%
Recursive[20] 52.5 µs 47.5 µs +10.57%
Recursive[21] 84.9 µs 76 µs +11.7%
small_drop 60 ns 53 ns +13.21%
large_setup 18 ns 20 ns -10%
iter_batched_large_input 44 ns 40 ns +10%
iter_batched_ref_large_input 44 ns 40 ns +10%
iter_batched_ref_per_iteration 44 ns 40 ns +10%
from_elem[16384] 986 ns 952 ns +3.57%
fibo_10 1 ns 2 ns -50%
fib_10 427 ns 376 ns +13.56%
fib_20 52.3 µs 45.6 µs +14.73%
fib_30 6.5 ms 5.6 ms +14.77%
iterative[5] 7 ns 8 ns -12.5%
recursive_memoized[BTreeMap<u64, u64>, 0] 18 ns 14 ns +28.57%
recursive_memoized[BTreeMap<u64, u64>, 5] 210 ns 221 ns -4.98%
generate_combinations[5] 1,157 ns 908 ns +27.42%
generate_combinations[6] 1.8 µs 1.9 µs -3.17%
generate_combinations[7] 2.6 µs 3.3 µs -19.42%
... ... ... ... ...

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

@not-matthias not-matthias force-pushed the simplify-instrument-hooks branch from 2561d9f to 773b742 Compare October 3, 2025 16:56
@not-matthias not-matthias force-pushed the simplify-instrument-hooks branch from 773b742 to 8054636 Compare October 3, 2025 17:00
@not-matthias not-matthias requested review from art049 and Copilot October 3, 2025 17:15
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 refactors the codspeed crate to always compile the instrument-hooks with stub implementations for non-Linux platforms, instead of conditionally excluding them entirely. The main goal is to provide a unified API across all platforms while maintaining platform-specific optimizations.

Key changes:

  • Removes conditional compilation guards that excluded instrument-hooks on non-Linux platforms
  • Consolidates platform-specific implementations into a single struct with conditional behavior
  • Updates error handling to use unsigned integers instead of signed integers

Reviewed Changes

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

File Description
crates/codspeed/src/instrument_hooks/mod.rs Unified InstrumentHooks implementation with platform-specific conditional logic instead of separate modules
crates/codspeed/instrument-hooks Updated subproject commit reference
crates/codspeed/build.rs Removed Linux-only compilation guard to build on all platforms
crates/codspeed/Cargo.toml Made nix dependency Linux-specific using target-specific dependencies

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

@not-matthias not-matthias merged commit 8054636 into main Oct 6, 2025
33 of 35 checks passed
@not-matthias not-matthias deleted the simplify-instrument-hooks branch October 6, 2025 08:54
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