Skip to content

pgrep: use regex to replace stat_split function and add some tests#663

Open
Franklin-Qi wants to merge 1 commit intouutils:mainfrom
Franklin-Qi:use-regex-to-replace-stat_split-function
Open

pgrep: use regex to replace stat_split function and add some tests#663
Franklin-Qi wants to merge 1 commit intouutils:mainfrom
Franklin-Qi:use-regex-to-replace-stat_split-function

Conversation

@Franklin-Qi
Copy link
Copy Markdown
Contributor

Use regex to replace stat_split function and add some tests in test_stat_split function.

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 7, 2026

Codecov Report

❌ Patch coverage is 0% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 19.19%. Comparing base (ee98c0a) to head (d7dbb2c).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
src/uu/pgrep/src/process.rs 0.00% 5 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main     #663   +/-   ##
=======================================
  Coverage   19.19%   19.19%           
=======================================
  Files          68       68           
  Lines       10135    10135           
  Branches      545      545           
=======================================
  Hits         1945     1945           
  Misses       8190     8190           
Flag Coverage Δ
macos_latest 6.74% <0.00%> (ø)
ubuntu_latest 19.54% <0.00%> (ø)
windows_latest 0.02% <0.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Franklin-Qi
Copy link
Copy Markdown
Contributor Author

Franklin-Qi commented Apr 7, 2026

The codecov/patch is currently throwing an error, but I have already added different test cases to the unit tests. Do I need to add additional unit test cases in the test_stat_split function? @sylvestre @Krysztal112233

@Franklin-Qi Franklin-Qi changed the title pgrep: use regex to replace stat_split function and add some tests. pgrep: use regex to replace stat_split function and add some tests Apr 7, 2026
@Krysztal112233
Copy link
Copy Markdown
Collaborator

could you please use hyperfine to create a small benchmark to demonstrate the performance improvement? I'd like to see how much this change improves performance.

@Krysztal112233
Copy link
Copy Markdown
Collaborator

Krysztal112233 commented Apr 7, 2026

The codecov/patch is currently throwing an error, but I have already added different test cases to the unit tests. Do I need to add additional unit test cases in the test_stat_split function? @sylvestre @Krysztal112233

I don't think it's required, this pr doesn't involve any more changes to the implementation :)

@Franklin-Qi
Copy link
Copy Markdown
Contributor Author

could you please use hyperfine to create a small benchmark to demonstrate the performance improvement? I'd like to see how much this change improves performance.

@Krysztal112233 Thanks for your review. Below are the performance tests I performed using hyperfine on the unmodified main branch and the modified use-regex-to-replace-stat_split-function for cargo test --package uu_pgrep.

$ LC_ALL=C hyperfine   --warmup 3   --runs 10 --show-output --command-name "main (old version)"   --prepare "git switch main"   "cargo test --package uu_pgrep"   --command-name "use-regex-to-replace-stat_split-function (new version)"   --prepare "git switch use-regex-to-replace-stat_split-function"   "cargo test --package uu_pgrep"
Benchmark 1: main (old version)
Switched to branch 'main'
Your branch is up to date with 'origin/main'.
   Compiling uu_pgrep v0.0.1 (/home/ysq/code/procps/src/uu/pgrep)
    Finished `test` profile [unoptimized + debuginfo] target(s) in 0.81s
     Running unittests src/pgrep.rs (target/debug/deps/uu_pgrep-58e1985eaa5ab062)

running 13 tests
test process::tests::test_run_state_conversion ... ok
test process::tests::test_ids ... ok
test process::tests::test_cgroups ... ok
test process::tests::test_stat_split ... ok
test process::tests::test_environ ... ok
test process::tests::test_namespaces ... ok
test process::tests::test_root ... ok
test process::tests::test_thread_ids ... ok
test process::tests::test_uid_gid ... ok
test process::tests::test_pid_entry ... ok
test process::tests::test_tty_resolution ... ok
test process_matcher::test_parse_pidfile_content_valid ... ok
test process::tests::test_walk_pid ... ok

test result: ok. 13 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.04s

     Running unittests src/main.rs (target/debug/deps/pgrep-78957d45a885038f)

...

running 13 tests
test process::tests::test_cgroups ... ok
test process::tests::test_namespaces ... ok
test process::tests::test_root ... ok
test process::tests::test_run_state_conversion ... ok
test process::tests::test_thread_ids ... ok
test process::tests::test_uid_gid ... ok
test process::tests::test_environ ... ok
test process::tests::test_ids ... ok
test process::tests::test_stat_split ... ok
test process::tests::test_pid_entry ... ok
test process::tests::test_tty_resolution ... ok
test process_matcher::test_parse_pidfile_content_valid ... ok
test process::tests::test_walk_pid ... ok

test result: ok. 13 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.02s

     Running unittests src/main.rs (target/debug/deps/pgrep-78957d45a885038f)

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

   Doc-tests uu_pgrep

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

  Time (mean ± σ):     240.5 ms ±  10.1 ms    [User: 201.5 ms, System: 79.1 ms]
  Range (min … max):   230.7 ms … 263.5 ms    10 runs
 
Summary
  use-regex-to-replace-stat_split-function (new version) ran
    1.06 ± 0.11 times faster than main (old version)

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