Merge pull request #8716 from sylvestre/refactor

bench: call the function directly to improve the profiling + move the common functions in uucore

Author: sylvestre

.github/workflows/CICD.yml

@@ -387,7 +387,6 @@ jobs:
         test -h /tmp/usr/local/bin/sha512sum
         test -h /tmp/usr/local/bin/shake128sum
         test -h /tmp/usr/local/bin/shake256sum
-  
   build_rust_stable:
     name: Build/stable
     needs: [ min_version, deps ]
@@ -1294,20 +1293,40 @@ jobs:
         shell: bash
         run: cargo install cargo-codspeed --locked
 
+      - name: Get benchmark list
+        id: benchmark_list
+        shell: bash
+        run: |
+          echo "Finding all utilities with benchmarks..."
+          benchmark_packages=""
+          for bench_dir in $(ls -d src/uu/*/benches 2>/dev/null); do
+            prog_dir=$(dirname "$bench_dir")
+            prog_name=$(basename "$prog_dir")
+            echo "Found benchmarks for uu_$prog_name"
+            benchmark_packages="$benchmark_packages uu_$prog_name"
+          done
+          echo "benchmark_packages=${benchmark_packages}" >> $GITHUB_OUTPUT
+          echo "Found benchmark packages:${benchmark_packages}"
+
+      - name: Build benchmarks
+        shell: bash
+        run: |
+          echo "Building benchmarks for packages: ${{ steps.benchmark_list.outputs.benchmark_packages }}"
+          for package in ${{ steps.benchmark_list.outputs.benchmark_packages }}; do
+            echo "Building benchmarks for $package"
+            cargo codspeed build -p $package
+          done
+
       - name: Run benchmarks
         uses: CodSpeedHQ/action@v4
         env:
           CODSPEED_LOG: debug
         with:
           mode: instrumentation
           run: |
-            # Find all utilities with benchmarks and run them individually
-            echo "Starting CodSpeed benchmark collection..."
-            for bench_dir in $(ls -d src/uu/*/benches 2>/dev/null); do
-              prog_dir=$(dirname "$bench_dir")
-              prog_name=$(basename "$prog_dir")
-              echo "Processing benchmarks for uu_$prog_name"
-              cargo codspeed build -p uu_$prog_name
-              cargo codspeed run -p uu_$prog_name
+            echo "Running benchmarks for packages: ${{ steps.benchmark_list.outputs.benchmark_packages }}"
+            for package in ${{ steps.benchmark_list.outputs.benchmark_packages }}; do
+              echo "Running benchmarks for $package"
+              cargo codspeed run -p $package
             done
           token: ${{ secrets.CODSPEED_TOKEN }}

Cargo.lock

@@ -27,6 +27,7 @@ allow = [
   "CC0-1.0",
   "Unicode-3.0",
   "Zlib",
+  "MPL-2.0",
 ]
 confidence-threshold = 0.8
 
@@ -106,6 +107,8 @@ skip = [
   { name = "rand_core", version = "0.6.4" },
   # utmp-classic
   { name = "zerocopy", version = "0.7.35" },
+  # divans/codspeed tooling
+  { name = "nix", version = "0.29.0" },
 ]
 # spell-checker: enable
 

deny.toml

@@ -30,6 +30,7 @@ path = "src/main.rs"
 [dev-dependencies]
 divan = { workspace = true }
 tempfile = { workspace = true }
+uucore = { workspace = true, features = ["benchmark"] }
 
 [[bench]]
 name = "tsort_bench"

src/uu/tsort/Cargo.toml

@@ -4,9 +4,8 @@
 // file that was distributed with this source code.
 
 use divan::{Bencher, black_box};
-use std::fs::File;
-use std::io::{BufWriter, Write};
-use tempfile::TempDir;
+use uu_tsort::uumain;
+use uucore::benchmark::{create_test_file, run_util_function};
 
 /// Generate topological sort test data with different characteristics
 fn generate_linear_chain(num_nodes: usize) -> Vec<u8> {
@@ -117,42 +116,17 @@ fn generate_wide_dag(num_nodes: usize) -> Vec<u8> {
     data
 }
 
-/// Create a temporary file with test data
-fn create_test_file(data: &[u8], temp_dir: &TempDir) -> std::path::PathBuf {
-    let file_path = temp_dir.path().join("test_data.txt");
-    let file = File::create(&file_path).unwrap();
-    let mut writer = BufWriter::new(file);
-    writer.write_all(data).unwrap();
-    writer.flush().unwrap();
-    file_path
-}
-
-/// Run uutils tsort with given arguments
-fn run_uutils_tsort(args: &[&str]) -> i32 {
-    use std::process::{Command, Stdio};
-
-    // Use the binary instead of calling uumain directly to avoid stdout issues
-    let output = Command::new("../../../target/release/coreutils")
-        .args(["tsort"].iter().chain(args.iter()))
-        .stdout(Stdio::null())
-        .stderr(Stdio::null())
-        .status()
-        .expect("Failed to execute tsort command");
-
-    i32::from(!output.success())
-}
-
 /// Benchmark linear chain graphs of different sizes
 /// This tests the performance improvements mentioned in PR #8694
 #[divan::bench(args = [1_000, 10_000, 100_000, 1_000_000])]
 fn tsort_linear_chain(bencher: Bencher, num_nodes: usize) {
     let temp_dir = tempfile::tempdir().unwrap();
     let data = generate_linear_chain(num_nodes);
-    let file_path = create_test_file(&data, &temp_dir);
+    let file_path = create_test_file(&data, temp_dir.path());
     let file_path_str = file_path.to_str().unwrap();
 
     bencher.bench(|| {
-        black_box(run_uutils_tsort(&[file_path_str]));
+        black_box(run_util_function(uumain, &[file_path_str]));
     });
 }
 
@@ -161,11 +135,11 @@ fn tsort_linear_chain(bencher: Bencher, num_nodes: usize) {
 fn tsort_tree_dag(bencher: Bencher, (depth, branching): (usize, usize)) {
     let temp_dir = tempfile::tempdir().unwrap();
     let data = generate_tree_dag(depth, branching);
-    let file_path = create_test_file(&data, &temp_dir);
+    let file_path = create_test_file(&data, temp_dir.path());
     let file_path_str = file_path.to_str().unwrap();
 
     bencher.bench(|| {
-        black_box(run_uutils_tsort(&[file_path_str]));
+        black_box(run_util_function(uumain, &[file_path_str]));
     });
 }
 
@@ -174,11 +148,11 @@ fn tsort_tree_dag(bencher: Bencher, (depth, branching): (usize, usize)) {
 fn tsort_complex_dag(bencher: Bencher, num_nodes: usize) {
     let temp_dir = tempfile::tempdir().unwrap();
     let data = generate_complex_dag(num_nodes);
-    let file_path = create_test_file(&data, &temp_dir);
+    let file_path = create_test_file(&data, temp_dir.path());
     let file_path_str = file_path.to_str().unwrap();
 
     bencher.bench(|| {
-        black_box(run_uutils_tsort(&[file_path_str]));
+        black_box(run_util_function(uumain, &[file_path_str]));
     });
 }
 
@@ -188,11 +162,11 @@ fn tsort_complex_dag(bencher: Bencher, num_nodes: usize) {
 fn tsort_wide_dag(bencher: Bencher, num_nodes: usize) {
     let temp_dir = tempfile::tempdir().unwrap();
     let data = generate_wide_dag(num_nodes);
-    let file_path = create_test_file(&data, &temp_dir);
+    let file_path = create_test_file(&data, temp_dir.path());
     let file_path_str = file_path.to_str().unwrap();
 
     bencher.bench(|| {
-        black_box(run_uutils_tsort(&[file_path_str]));
+        black_box(run_util_function(uumain, &[file_path_str]));
     });
 }
 
@@ -213,11 +187,11 @@ fn tsort_input_parsing_heavy(bencher: Bencher, num_edges: usize) {
         }
     }
 
-    let file_path = create_test_file(&data, &temp_dir);
+    let file_path = create_test_file(&data, temp_dir.path());
     let file_path_str = file_path.to_str().unwrap();
 
     bencher.bench(|| {
-        black_box(run_uutils_tsort(&[file_path_str]));
+        black_box(run_util_function(uumain, &[file_path_str]));
     });
 }
 

src/uu/tsort/benches/tsort_bench.rs

@@ -32,6 +32,7 @@ libc = { workspace = true }
 [dev-dependencies]
 divan = { workspace = true }
 tempfile = { workspace = true }
+uucore = { workspace = true, features = ["benchmark"] }
 
 [[bin]]
 name = "wc"