Skip to content
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.

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

Darwin drop callps step2 #1229

Merged
merged 3 commits into from
Jan 30, 2022
Merged

Conversation

PierreF
Copy link
Contributor

@PierreF PierreF commented Jan 13, 2022

Continuation of #1226. This PR include #1226 (so we could merge this one directly and ignoring the first one).

Using libproc (CGO) we can speed-up retrieving information of most process. Some (likely all that belong to another user) fail with EPERM. For them I do fallback on using ps.

On my Mac, I got close to 5x faster run time: 3 seconds for 732 processes vs 14.5 seconds on master.

I've used https://gist.github.com/PierreF/d48d6c56bf5f06b471362d06eafe0111 to test performance and difference between this PR and master.

I didn't see significant difference. Most difference are either:

  • fixes of existings bugs (like GIDs missing, cf comment in Avoid ps command and use KProc on MacOS #1226)
  • Precision changes (like in createtime that now includes milliseconds, or user/system time that are no longer truncated to 1/100th of seconds).
  • Real change, since the two program don't run at the exact same time.

My system is a Mac M1 (arm64) using MacOS 12. I'll try it on a Mac Intel tomorow.

@Lomanic
Copy link
Collaborator

Lomanic commented Jan 13, 2022

Please squash the last two commits in one.

Could you be more explicit with your 3rd commit message? (at least mention the process package and darwin).

Some (likely all that belong to another user) fail with EPERM. For them I do fallback on using ps.

That feels really hacky and we don't do the same for linux. Users should run their binary as setuid 0 (like ps) if they want to look at foreign programs.

@PierreF
Copy link
Contributor Author

PierreF commented Jan 14, 2022

That feels really hacky and we don't do the same for linux. Users should run their binary as setuid 0 (like ps) if they want to look at foreign programs.

That feels really hacky and we don't do the same for linux. Users should run their binary as setuid 0 (like ps) if they want to look at foreign programs.

Ohhh, I didn't see that ps is a setuid binary. That make sense and I've updated the commit to drop the fallback on EPERM.

@PierreF
Copy link
Contributor Author

PierreF commented Jan 26, 2022

Is there something I can do to help this PR getting merged ?

I should have fixed your comments, or is there something missing ?

@shirou
Copy link
Owner

shirou commented Jan 26, 2022

Since I don't have a mac, I can not confirm this PR works or not, and no one reported except noble kind @Lomanic (but I think he also does not have a real mac).

I will borrow a intel mac from my friend in this weekend.

Copy link
Owner

@shirou shirou left a comment

Choose a reason for hiding this comment

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

I confirmed this PR works on my friend's mac, Big Sur Intel i7. All tests in process by go test -v have been become faster from 12sec to 3 sec. Great improvement! Thank you so much and sorry to late to review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants