Improves iperf thread performance by 13.5% in a dual-core cpu-bound testbed.
Details
- Reviewers
rscheff tuexen - Commits
- rGfafb03ab4254: siftr: flush pkt_nodes to the log file in batch
test result: test result of D41175
From lock_profile statistics, lock contention on "kern_alq.c:215 (spin mutex:ALD Queue)" has 50% reduction.
Compared with the iperf performance in base, this change brings 13.5% improvement in iperf performance.
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 52818 Build 49709: arc lint + arc unit
Event Timeline
I'd feel better if the remaining size of the buffer was length checked to avoid any possible overruns...
sys/netinet/siftr.c | ||
---|---|---|
511 | if you do the buffer length check like this (statically assuming that the buffer will always have sufficient space for one log string), make sure that the buffer can hold MAX_LOG_MSG_LEN or more before calling down to the formatter... I was thinking more of providing the remaining size of the allocated buffer down, and allow the snprintf to verify it won't run over. Someone may want to change the printf() string, and some of these are variable length. BTW is there a reason for cycling over 3 messages only and not more? Was that the sweet spot between CPU savings and timeliness? |
sys/netinet/siftr.c | ||
---|---|---|
511 | I couldn't figure out a better solution to the case if the string size is larger than MAX_LOG_MSG_LEN. The current result does not change the output. If the string is larger than MAX_LOG_MSG_LEN, the strange output will alert the user there is a string length problem. In my private version, I have a counter that captures the max string size and it is printed in the foot note when siftr is disabled. Do you think this is acceptable? The cycling over 3 messages is the sweet spot. I didn't find further performance improvement for "4" and larger messages. Logically, "3 == 2 + 1", it is already a waste of 50% messages allocation for low rate traffic, but 100% hit prediction for high rate traffic. Increase the messages in a batch allocation more than "3" does not seem to be reasonable either to me. |