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

Reduce CPU usage when test baud rate is limited #1743

Conversation

davidBar-On
Copy link
Contributor

@davidBar-On davidBar-On commented Aug 11, 2024

  • Version of iperf3 (or development branch, such as master or
    3.1-STABLE) to which this pull request applies:
    master

  • Issues fixed (if any): 100% CPU utilization in a limited rate test #1741

  • Brief description of code changes (suitable for use as a commit message):

Suggested fix to #1741 for (significantly) reduce iperf3 send CPU usage when --baud limits the sending baud rate. The suggested change uses nanosleep() or clock_nanosleep() to sleep between sends instead of using the pacing timer. This change seem to work fine because of the current usage of threads and since the select() is used only in the main thread.

clock_nanosleep() has a cleaner implementation, so its use is preferred. However, since it seem to be newer than nanosleep(), nanosleep() is used if clock_nanosleep() is not supported. Both give similar results. If both functions are not supported, then the legacy behavior is used.

The --pacing-timer option is removed. However the related variables/functions are not removed in case they are used as part of the library.

(By the way, it seems that iperf_send_mt() can be simplified. E.g. the if (sp->sender) is probably redundant. In addition, the use of burst/multisend is also probably redundant, since the sending threads do not use select().)

Copy link
Contributor

@bmah888 bmah888 left a comment

Choose a reason for hiding this comment

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

Thanks for the pull request! The problem that this change solves is one that's kind of been bothering me ever since we implemented multi-threading, but I didn't quite have the knowledge to fix it. Also our main use cases don't use --bitrate, so admittedly we didn't put much priority on it. Your solution is a lot fewer lines of code than I expected!

I've testing this on a few hosts we have, so far so good. Just have a couple minor changes to suggest (at this point).

src/iperf_api.c Outdated Show resolved Hide resolved
src/iperf_api.c Outdated Show resolved Hide resolved
@bmah888 bmah888 self-assigned this Aug 28, 2024
Copy link
Contributor

@bmah888 bmah888 left a comment

Choose a reason for hiding this comment

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

Thanks for the changes! I might want to tweak the help message later, but the functionality looks good.

@bmah888 bmah888 merged commit 4abd5ea into esnet:master Aug 30, 2024
3 checks passed
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