Skip to content

fix: optimize performance of process.OpenFiles() #1866

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

Merged
merged 3 commits into from
Jun 12, 2025

Conversation

NitroCao
Copy link
Contributor

@NitroCao NitroCao commented Jun 6, 2025

fixes #1781

@NitroCao
Copy link
Contributor Author

NitroCao commented Jun 6, 2025

@Dainerx You can use the draft PR to test for now. I just find another point to optmize, and will let you know after submitting code.

@NitroCao NitroCao force-pushed the fix/optimize-openfiles branch 2 times, most recently from 86c6e99 to 9fa1f08 Compare June 6, 2025 13:05
@NitroCao
Copy link
Contributor Author

NitroCao commented Jun 7, 2025

Is it good and necessary to provide an iterator method for (*Process).OpenFiles() called OpenFilesSeq() to return a iter.Seq[OpenFilesStat]?

@shirou
Copy link
Owner

shirou commented Jun 7, 2025

The Iterator is not necessary. However, since Go 1.23 —where Iterator was introduced— is the oldest Go version we support, it’s fine to use it, I think.

@NitroCao NitroCao marked this pull request as ready for review June 7, 2025 13:34
@NitroCao NitroCao force-pushed the fix/optimize-openfiles branch from 2fcecb5 to 9dcddfe Compare June 7, 2025 13:50
@NitroCao
Copy link
Contributor Author

@shirou Please merge the PR if you think everything is ok.

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.

A slight change can now be made to speed up the process considerably. It is an amazing achievement. Thank you very much.

I have one comment to more optimization. It is not necessary but just an idea.

@NitroCao
Copy link
Contributor Author

A slight change can now be made to speed up the process considerably. It is an amazing achievement. Thank you very much.

I have one comment to more optimization. It is not necessary but just an idea.

I just implemented the idea. BTW, which tool do you use to geneerate the detailed benchmark?

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.

Awesome! This should improve performance. I really appreciate your contribution.

The tool I used is benchstat. It simply makes benchmark results easier to read.

@shirou shirou merged commit ad10d4d into shirou:master Jun 12, 2025
52 checks passed
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.

process.OpenFiles() does not scale well and consumes ton of CPU
2 participants