8000 pgrep: Add command `pgrep` by Krysztal112233 · Pull Request #95 · uutils/procps · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

pgrep: Add command pgrep #95

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 37 commits into from
Jul 1, 2024
Merged

pgrep: Add command pgrep #95

merged 37 commits into from
Jul 1, 2024

Conversation

Krysztal112233
Copy link
Collaborator
@Krysztal112233 Krysztal112233 commented May 29, 2024 •
edited
Loading

fix #26

@codecov Codecov
Copy link
codecov bot commented May 29, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 66.46341% with 110 lines in your changes missing coverage. Please review.

Project coverage is 51.99%. Comparing base (589acc8) to head (11036e7).
Report is 21 commits behind head on main.

Files Patch % Lines
src/uu/pgrep/src/process.rs 53.16% 62 Missing and 12 partials ⚠️
src/uu/pgrep/src/pgrep.rs 72.09% 14 Missing and 22 partials ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main      #95      +/-   ##
==========================================
+ Coverage   45.95%   51.99%   +6.04%     
==========================================
  Files          18       21       +3     
  Lines        2298     2652     +354     
  Branches      332      403      +71     
==========================================
+ Hits         1056     1379     +323     
- Misses       1066     1145      +79     
+ Partials      176      128      -48
Flag Coverage Δ
macos_latest 45.80% <22.44%> (?)
ubuntu_latest 48.97% <66.46%> (+2.39%) ⬆️
windows_latest 0.09% <0.00%> (-0.02%) ⬇️

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.

cakebaker
cakebaker reviewed May 29, 2024
View reviewed changes
cakebaker
cakebaker reviewed May 29, 2024
View reviewed changes
cakebaker
cakebaker reviewed May 29, 2024
View reviewed changes
cakebaker
cakebaker reviewed May 29, 2024
View reviewed changes
@Krysztal112233
Copy link
Collaborator Author
Krysztal112233 commented May 30, 2024 •
edited
Loading

regex is too slow, need to write algorithm for parsing /proc/self/stat

# Uutils implementation
./pgrep -o  0.55s user 0.01s system 99% cpu 0.561 total
./pgrep -n  0.50s user 0.02s system 99% cpu 0.522 total

# GNU implementation
pgrep -o  0.02s user 0.00s system 95% cpu 0.022 total
pgrep -n  0.00s user 0.02s system 94% cpu 0.017 total

@Krysztal112233
Copy link
Collaborator Author

after rewrite algorithm

./pgrep -o  0.04s user 0.05s system 98% cpu 0.092 total
./pgrep -n  0.04s user 0.05s system 98% cpu 0.089 total

cakebaker
cakebaker reviewed May 30, 2024
View reviewed changes
cakebaker
cakebaker reviewed May 30, 2024
View reviewed changes
cakebaker
cakebaker reviewed May 30, 2024
View reviewed changes
@sylvestre
Copy link
Contributor

for benchmarking, you should use hyperfine :)

sylvestre
sylvestre reviewed May 30, 2024
View reviewed changes
cakebaker
cakebaker reviewed Jun 1, 2024
View reviewed changes
cakebaker
cakebaker reviewed Jun 1, 2024
View reviewed changes
cakebaker
cakebaker reviewed Jun 1, 2024
View reviewed changes
@Krysztal112233
Copy link
Collaborator Author

I came up with a better idea, I think make the handle_* method just collect the required pid and let the uumain method decide the output format based on the flag, the code will be more understandable and concise than before.

cakebaker
cakebaker reviewed Jun 11, 2024
View reviewed changes
cakebaker
cakebaker reviewed Jun 11, 2024
View reviewed changes
cakebaker
cakebaker reviewed Jun 11, 2024
View reviewed changes
cakebaker
cakebaker reviewed Jun 11, 2024
View reviewed changes
cakebaker
cakebaker reviewed Jun 11, 2024
View reviewed changes
cakebaker
cakebaker reviewed Jun 11, 2024
View reviewed changes
cakebaker
cakebaker reviewed Jun 12, 2024
View reviewed changes
cakebaker
cakebaker reviewed Jun 12, 2024
View reviewed changes
Copy link
Contributor
@cakebaker cakebaker left a comment

Choose a reason for hiding this comment

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

A general feedback: please add more tests :)

Krysztal112233
Krysztal112233 commented Jun 12, 2024
View reviewed changes
cakebaker
cakebaker reviewed Jun 13, 2024
View reviewed changes
sylvestre
sylvestre reviewed Jun 17, 2024
View reviewed changes
sylvestre
sylvestre reviewed Jun 17, 2024
View reviewed changes
cakebaker
cakebaker reviewed Jun 20, 2024
View reviewed changes
cakebaker
cakebaker reviewed Jun 20, 2024
View reviewed changes
cakebaker
cakebaker reviewed Jun 20, 2024
View reviewed changes
cakebaker
cakebaker reviewed Jun 20, 2024
View reviewed changes
cakebaker
cakebaker reviewed Jun 20, 2024
View reviewed changes
cakebaker
cakebaker reviewed Jun 20, 2024
View reviewed changes
cakebaker
cakebaker reviewed Jun 20, 2024
View reviewed changes
cakebaker
cakebaker reviewed Jun 20, 2024
View reviewed changes
cakebaker
cakebaker reviewed Jun 20, 2024
View reviewed changes
cakebaker
cakebaker reviewed Jun 20, 2024
View reviewed changes
cakebaker
cakebaker reviewed Jun 20, 2024
View reviewed changes
Copy link
Contributor
@cakebaker cakebaker left a comment

Choose a reason for hiding this comment

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

A review of pgrep.rs hopefully follows tomorrow.

cakebaker
cakebaker reviewed Jun 21, 2024
View reviewed changes
cakebaker
cakebaker reviewed Jun 21, 2024
View reviewed changes
cakebaker
cakebaker reviewed Jun 21, 2024
View reviewed changes
cakebaker
cakebaker reviewed Jun 21, 2024
View reviewed changes
cakebaker
cakebaker reviewed Jun 21, 2024
View reviewed changes
tertsdiepraam
tertsdiepraam reviewed Jun 29, 2024
View reviewed changes
Copy link
Member
@tertsdiepraam tertsdiepraam left a comment

Choose a reason for hiding this comment

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

Small suggestion. I think @cakebaker is better equipped to do a review. Happy to help out more if you need it!

cakebaker
cakebaker reviewed Jun 29, 2024
View reviewed changes
cakebaker
cakebaker reviewed Jun 30, 2024
View reviewed changes
cakebaker
cakebaker reviewed Jun 30, 2024
View reviewed changes
Copy link
Contributor
@cakebaker cakebaker left a comment

Choose a reason for hiding this comment

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

With the exception of the two function names it looks good.

@cakebaker cakebaker merged commit 5339454 into uutils:main Jul 1, 2024
16 checks passed
@cakebaker
Copy link
Contributor

Good work :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cakebaker cakebaker cakebaker left review comments

@sylvestre sylvestre Awaiting requested review from sylvestre

@tertsdiepraam tertsdiepraam Awaiting requested review from tertsdiepraam

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add command pgrep
4 participants
@Krysztal112233 @sylvestre @cakebaker @tertsdiepraam
0