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

Fix busy polling in startup phase #1524

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

BenjaminBeichler
Copy link

During the startup phase, the socket file descriptors (socket-fds) of the streams are added to the select fd-sets. However, these file descriptors are not read or written, causing the select operation to be non-blocking and leading to busy polling.

To address this issue, a separate fd-set is introduced specifically for the startup phase, which includes only the control socket.

In most scenarios, this results in a short period of increased CPU usage during startup. However, in more uncommon situations, such as simulations that utilize target code, this creates unnecessary load on the system.

PLEASE NOTE the following text from the iperf3 license. Submitting a
pull request to the iperf3 repository constitutes "[making]
Enhancements available...publicly":

You are under no obligation whatsoever to provide any bug fixes, patches, or
upgrades to the features, functionality or performance of the source code
("Enhancements") to anyone; however, if you choose to make your Enhancements
available either publicly, or directly to Lawrence Berkeley National
Laboratory, without imposing a separate written license agreement for such
Enhancements, then you hereby grant the following license: a non-exclusive,
royalty-free perpetual license to install, use, modify, prepare derivative
works, incorporate into other computer software, distribute, and sublicense
such enhancements or derivative works thereof, in binary and source code form.

The complete iperf3 license is available in the LICENSE file in the
top directory of the iperf3 source tree.

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

  • Issues fixed (if any):

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

@BenjaminBeichler
Copy link
Author

@bmah888 Are there any problems with that PR?

During the startup phase, the socket file descriptors (socket-fds) of
the streams are added to the select fd-sets. However, these file
descriptors are not read or written, causing the select operation to be
non-blocking and leading to busy polling.

To address this issue, a separate fd-set is introduced specifically for
the startup phase, which includes only the control socket.

In most scenarios, this results in a short period of increased CPU usage
during startup. However, in more uncommon situations, such as
simulations that utilize target code, this creates unnecessary load on
the system.

Signed-off-by: Benjamin Beichler <[email protected]>
@bmah888
Copy link
Contributor

bmah888 commented Sep 26, 2023

@bmah888 Are there any problems with that PR?

Thanks for the PR! I've only given it a quick glimpse, just too many things to do and not enough time. Our main development focus right now is getting the multi-threading functionality tested and ready to merge from the mt branch into the mainline. That will change the behavior of the socket handling code somewhat as each of the sockets used for test data will be serviced by its own thread (the control connection will continue to be serviced by the process's main thread). I think that's going to affect this PR, although I'm not sure in what way exactly.

It's possible there might be some similar busy-waiting issues in how iperf3's application-level pacing (--bitrate option) works in the mt branch.

@BenjaminBeichler
Copy link
Author

From my glimpse into the mt branch, it may solve the problematic behavior of the single threaded version. From my understanding, you use blocking reads in the separate threads for each stream and you do not put the stream FDs into the select of the main thread loop.

Nonetheless, the patch would not harm even the mt version, since the logic of the read and write set kept the same. I could easily adapt it for the mt branch. But since the main loop is only working on the control channel, the actual variables could be removed or replaced by only the control channel socket fd.

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