-
Notifications
You must be signed in to change notification settings - Fork 49
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: Bug #443 -- better optimize network.py high-speed network #479
Conversation
providers/base/bin/network.py
Outdated
filename = "/sys/class/net/" + device + "/device/numa_node" | ||
try: | ||
file = open(filename, "r") | ||
node_num = int(file.read()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would be a bit better using with:
try:
with open(filename, "r") as file:
node_num = int(file.read())
except FileNotFoundError
so that the file is closed once it's been read.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor point about using with when opening filename so that the file is properly closed, otherwise LGTM. I think this is going to be a bit noisy (I haven't run it yet to see) so we likely will want to revisit this next cycle and clean up the output a bit in general.
…mance. Make Jeff's suggested changes
48a596a
to
e2f4e7f
Compare
I've made Jeff's suggested changes and squashed the results. Also, I've changed the description (shown near the top of this page, at least to me). I accidentally left the whole thing commented out on the original submission; I've fixed that mistake. Note that the description includes a sample output from running the revised script. The PR includes code to disable logging when doing the test runs, but adds summaries for each test run. This minimizes the added clutter, but there IS some extra output, to help in case there are problems when this type of output might be helpful. Finally, I've also created a PR for minor changes to the STG so that users know to launch more instances of iperf3 on the Target system: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool, Thanks!
Hi @pieqq and @rodwsmith Test environment:
I ran into a problem which gives the error like below
What happened?According this PR, it tries to find the number of NUMA node via
|
@baconYao , could you please file a bug report? It looks like this should be easy enough to fix, but it would be helpful to have a bug report to reference in the PR. I don't have an RPi 4, but I do have an earlier version, so I can try testing on it. |
Hi @rodwsmith, I file an issue #539. |
Fix: Bug #443 -- better optimize network.py high-speed network performance.
Description
This pull request fixes sub-optimal network performance in the network.py network speed tests on high-speed (>10 Gbps) network devices. It makes three changes:
The patch has no effect on slow (10 Mbps and slower) network devices, but on faster ones, the thread optimization tests add about four minutes to the run time, and add some output (see below).
Resolved issues
Fixes: #443
Documentation
Neither Checkbox nor the network.py script has changes to their calling conventions or configuration; however, output is expanded a bit, as in this example:
sudo ./network.py test -i ens1 -t iperf --iperf3 --scan-timeout 36 --fail-threshold 80 --cpu-load-fail-threshold 90 --runtime 30 --num_runs 1 --target 10.1.16.15
INFO:root:Testing ens1 against 10.1.16.15
INFO:root:Have successfully pinged 10.1.16.15 on ens1
INFO:root:--------------- Optimizing Number of Threads ---------------
INFO:root:Testing optimization with 5 threads
INFO:root:Found throughput of 39526 with 5 threads
INFO:root:Testing optimization with 10 threads
INFO:root:Found throughput of 85962 with 10 threads
INFO:root:Testing optimization with 15 threads
INFO:root:Found throughput of 75820 with 15 threads
INFO:root:Testing optimization with 20 threads
INFO:root:Found throughput of 98198 with 20 threads
INFO:root:Setting number of threads to 20.
INFO:root:-------------------- Test Run Number 1 --------------------
INFO:root:Using 20 threads.
INFO:root:NUMA node of ens1 is 0....
INFO:root:Connecting to port 5201 on server....
INFO:root:Connecting to port 5202 on server....
INFO:root:Connecting to port 5203 on server....
INFO:root:Connecting to port 5204 on server....
INFO:root:Connecting to port 5205 on server....
INFO:root:Connecting to port 5206 on server....
INFO:root:Connecting to port 5207 on server....
INFO:root:Connecting to port 5208 on server....
INFO:root:Connecting to port 5209 on server....
INFO:root:Connecting to port 5210 on server....
INFO:root:Connecting to port 5211 on server....
INFO:root:Connecting to port 5212 on server....
INFO:root:Connecting to port 5213 on server....
INFO:root:Connecting to port 5214 on server....
INFO:root:Connecting to port 5215 on server....
INFO:root:Connecting to port 5216 on server....
INFO:root:Connecting to port 5217 on server....
INFO:root:Connecting to port 5218 on server....
INFO:root:Connecting to port 5219 on server....
INFO:root:Connecting to port 5220 on server....
INFO:root:Avg Transfer speed: 89510.53333333335 Mb/s
INFO:root:89.51% of theoretical max 100000 Mb/s
INFO:root:Average CPU utilization: 16.0%
INFO:root:
The "Optimizing Number of Threads" section is new, and summarizes the optimization testing. This section is omitted on slow (10 Gbps and slower) tests.
To take advantage of these changes, the iperf3 server may need to run more threads. This will require changes to the Server Certification documentation (the Self-Test Guide).
Tests
Testing was done by manually running the test using several servers in Needham. Improvements in performance of about 5-10% were observed after these changes were implemented.