This commit is mostly based on the one from the Spirent fork of OSv - https://github.com/SpirentOrion/osv/commit/29e4d2bbc23d6ddbf8d4b065fc3388c9931e705a - and its original description reads:
"Multiple syscalls used std::vector to manage local iovec copies. For our uses cases, this is totally unnecessary overhead and results in 1000's of small object memory allocations. So, just use the stack instead." In nutshell this patch slightly optimizes 4 functions - 2 that are part of the networking stack and 2 others in VFS layer to - by tweaking them to use stack instead of malloc()/free() which are relatively constly. Please note that the objects in question - copies of iovec - are pretty tiny, typically 16 bytes in size so it does make sense to use stack. Obviously it is hard to tell without measuring how significant this change is in terms of performance benefits and what use cases it would benefit. I did however find two usecases where I could observe some significant decrease of malloc invocations. The first is actually the cpiod app used to upload ZFS files during upload file which seems to be hitting the VFS code path in question. I this case I could see ~25% drop of malloc/free invocations. The second use case involved a test misc-tcp.cc which sends and receives data over socket in multiple threads and seems to be hitting the networking stack paths. In this case I could see 15-6% drop. The program was called like so: ./scripts/run.py -e '/tests/misc-tcp.so --remote=192.168.122.1 -c 10 -n 10 -l 5' with netcat running like so: ncat -l -k -p 9999 -e /bin/cat This patch also updates the test makefile to make it build misc-tcp.so after kernel no longer includes program options. It also slightly updates the test itself to output some helpful information in progress. Co-authored-by: "Timmons C. Player" <timmons.pla...@spirent.com> Co-authored-by: Waldemar Kozaczuk <jwkozac...@gmail.com> Signed-off-by: Waldemar Kozaczuk <jwkozac...@gmail.com> --- bsd/sys/kern/uipc_syscalls.cc | 26 ++++++++++++++++---------- fs/vfs/vfs_syscalls.cc | 14 ++++++++++---- modules/tests/Makefile | 8 ++++++-- tests/misc-tcp.cc | 12 +++++++++--- 4 files changed, 41 insertions(+), 19 deletions(-) diff --git a/bsd/sys/kern/uipc_syscalls.cc b/bsd/sys/kern/uipc_syscalls.cc index 9f8db3f6..1ae4090e 100644 --- a/bsd/sys/kern/uipc_syscalls.cc +++ b/bsd/sys/kern/uipc_syscalls.cc @@ -494,10 +494,12 @@ kern_sendit(int s, so = (struct socket *)file_data(fp); // Create a local copy of the user's iovec - sosend() is going to change it! - std::vector<iovec> uio_iov(mp->msg_iov, mp->msg_iov + mp->msg_iovlen); + assert(mp->msg_iovlen <= UIO_MAXIOV); + struct iovec uio_iov[mp->msg_iovlen]; + memcpy(uio_iov, mp->msg_iov, sizeof(uio_iov)); - auio.uio_iov = uio_iov.data(); - auio.uio_iovcnt = uio_iov.size(); + auio.uio_iov = uio_iov; + auio.uio_iovcnt = mp->msg_iovlen;; auio.uio_rw = UIO_WRITE; auio.uio_offset = 0; /* XXX */ auio.uio_resid = 0; @@ -585,10 +587,12 @@ kern_recvit(int s, struct msghdr *mp, struct mbuf **controlp, ssize_t* bytes) so = (socket*)file_data(fp); // Create a local copy of the user's iovec - sorecieve() is going to change it! - std::vector<iovec> uio_iov(mp->msg_iov, mp->msg_iov + mp->msg_iovlen); + assert(mp->msg_iovlen <= UIO_MAXIOV); + struct iovec uio_iov[mp->msg_iovlen]; + memcpy(uio_iov, mp->msg_iov, sizeof(uio_iov)); - auio.uio_iov = uio_iov.data(); - auio.uio_iovcnt = uio_iov.size(); + auio.uio_iov = uio_iov; + auio.uio_iovcnt = mp->msg_iovlen; auio.uio_rw = UIO_READ; auio.uio_offset = 0; /* XXX */ auio.uio_resid = 0; @@ -653,7 +657,7 @@ out: if (fromsa) free(fromsa); - if (error == 0 && controlp != NULL) + if (error == 0 && controlp != NULL) *controlp = control; else if (control) m_freem(control); @@ -1038,10 +1042,12 @@ zcopy_tx(int s, struct zmsghdr *zm) if (so->so_type != SOCK_STREAM) return (EINVAL); // Create a local copy of the user's iovec - sosend() is going to change it! - std::vector<iovec> uio_iov(mp->msg_iov, mp->msg_iov + mp->msg_iovlen); + assert(mp->msg_iovlen <= UIO_MAXIOV); + struct iovec uio_iov[mp->msg_iovlen]; + memcpy(uio_iov, mp->msg_iov, sizeof(uio_iov)); - auio.uio_iov = uio_iov.data(); - auio.uio_iovcnt = uio_iov.size(); + auio.uio_iov = uio_iov; + auio.uio_iovcnt = mp->msg_iovlen; auio.uio_rw = UIO_WRITE; auio.uio_offset = 0; auio.uio_resid = 0; diff --git a/fs/vfs/vfs_syscalls.cc b/fs/vfs/vfs_syscalls.cc index cd0d1745..055a32c7 100644 --- a/fs/vfs/vfs_syscalls.cc +++ b/fs/vfs/vfs_syscalls.cc @@ -266,8 +266,11 @@ sys_read(struct file *fp, const struct iovec *iov, size_t niov, struct uio uio; // Unfortunately, the current implementation of fp->read zeros the // iov_len fields when it reads from disk, so we have to copy iov. - std::vector<iovec> copy_iov(iov, iov + niov); - uio.uio_iov = copy_iov.data(); + assert(niov <= UIO_MAXIOV); + struct iovec copy_iov[niov]; + memcpy(copy_iov, iov, sizeof(copy_iov)); + + uio.uio_iov = copy_iov; uio.uio_iovcnt = niov; uio.uio_offset = offset; uio.uio_resid = bytes; @@ -302,8 +305,11 @@ sys_write(struct file *fp, const struct iovec *iov, size_t niov, struct uio uio; // Unfortunately, the current implementation of fp->write zeros the // iov_len fields when it writes to disk, so we have to copy iov. - std::vector<iovec> copy_iov(iov, iov + niov); - uio.uio_iov = copy_iov.data(); + assert(niov <= UIO_MAXIOV); + struct iovec copy_iov[niov]; + memcpy(copy_iov, iov, sizeof(copy_iov)); + + uio.uio_iov = copy_iov; uio.uio_iovcnt = niov; uio.uio_offset = offset; uio.uio_resid = bytes; diff --git a/modules/tests/Makefile b/modules/tests/Makefile index 33af8303..7d15522c 100644 --- a/modules/tests/Makefile +++ b/modules/tests/Makefile @@ -213,6 +213,10 @@ $(boost-tests:%=$(out)/tests/%): LIBS += \ -lboost_unit_test_framework \ -lboost_filesystem +boost-program-options-tests := misc-tcp.so misc-zfs-arc.so +$(boost-program-options-tests:%=$(out)/tests/%): LIBS += \ + -lboost_program_options + common-tests := $(tests) $(common-boost-tests) tests += $(boost-tests) @@ -253,7 +257,7 @@ usr.manifest: build_all_tests $(lastword $(MAKEFILE_LIST)) usr.manifest.skel FOR @cat $@.skel > $@ @case "$(CROSS_PREFIX)" in \ "aarch64"*) ./add_aarch64_boost_libraries.sh $(OSV_BASE) >> $@ ;; \ - *) LD_LIBRARY_PATH=$(boost-lib-dir) ldd $(addprefix $(out)/tests/,$(boost-tests)) | grep libboost | sed 's/ *[^ ] *\(.*\) => \(.*\) .*/\/usr\/lib\/\1: \2/' | sort | uniq >> $@ ;; \ + *) LD_LIBRARY_PATH=$(boost-lib-dir) ldd $(addprefix $(out)/tests/,$(boost-tests)) $(addprefix $(out)/tests/,$(boost-program-options-tests)) | grep libboost | sed 's/ *[^ ] *\(.*\) => \(.*\) .*/\/usr\/lib\/\1: \2/' | sort | uniq >> $@ ;; \ esac @echo $(all_tests) | tr ' ' '\n' | grep -v "tests/rofs/tst-.*.so" | awk '{print "/" $$0 ": ./" $$0}' >> $@ @echo $(all_tests) | tr ' ' '\n' | grep "tests/rofs/tst-.*.so" | sed 's/\.so//' | awk 'BEGIN { FS = "/" } ; { print "/tests/" $$3 "-rofs.so: ./tests/" $$2 "/" $$3 ".so"}' >> $@ @@ -266,7 +270,7 @@ common.manifest: build_all_tests $(lastword $(MAKEFILE_LIST)) usr.manifest.skel @cat usr.manifest.skel > $@ @case "$(CROSS_PREFIX)" in \ "aarch64"*) ./add_aarch64_boost_libraries.sh $(OSV_BASE) >> $@ ;; \ - *) LD_LIBRARY_PATH=$(boost-lib-dir) ldd $(addprefix $(out)/tests/,$(boost-tests)) | grep libboost | sed 's/ *[^ ] *\(.*\) => \(.*\) .*/\/usr\/lib\/\1: \2/' | sort | uniq >> $@ ;; \ + *) LD_LIBRARY_PATH=$(boost-lib-dir) ldd $(addprefix $(out)/tests/,$(boost-tests)) $(addprefix $(out)/tests/,$(boost-program-options-tests)) | grep libboost | sed 's/ *[^ ] *\(.*\) => \(.*\) .*/\/usr\/lib\/\1: \2/' | sort | uniq >> $@ ;; \ esac @echo $(common-tests) | tr ' ' '\n' | awk '{print "/tests/" $$0 ": ./tests/" $$0}' >> $@ diff --git a/tests/misc-tcp.cc b/tests/misc-tcp.cc index 53b55a08..ec98fc86 100644 --- a/tests/misc-tcp.cc +++ b/tests/misc-tcp.cc @@ -19,6 +19,10 @@ #include <random> #include <iostream> +// To run this test you need netcat running in another terminal +// to echo the data back to this program like so: +// ncat -l -k -p 9999 -e /bin/cat + using namespace std; using std::mutex; @@ -50,7 +54,7 @@ private: ~test_thread(); void run(); private: - void do_connection(); + void do_connection(int c); private: tcp_test_client& _client; unique_lock<mutex> _lock; @@ -87,7 +91,7 @@ tcp_test_client::test_thread::~test_thread() _thread.join(); } -void tcp_test_client::test_thread::do_connection() +void tcp_test_client::test_thread::do_connection(int c) { uint64_t transfer = _client._dist_transfer(_client._rand); reverse_lock<decltype(_lock)> rev_lock(_lock); @@ -105,6 +109,7 @@ void tcp_test_client::test_thread::do_connection() socket.connect(endpoint); std::array<uint32_t, 1024> send_buffer, receive_buffer; uint64_t done = 0, rdone = 0; + std::cout << "Thread: " << _thread.get_id() << ", connection: " << c << " transfering " << transfer << " bytes" << std::endl; while (done < transfer) { iota(send_buffer.begin(), send_buffer.end(), done / sizeof(uint32_t)); uint64_t offset = 0; @@ -140,6 +145,7 @@ void tcp_test_client::test_thread::do_connection() if (_client.done()) { _client._condvar.notify_all(); } + std::cout << "Thread: " << _thread.get_id() << ", connection: " << c << " transfered and received all data back!" << std::endl; } void tcp_test_client::test_thread::run() @@ -149,7 +155,7 @@ void tcp_test_client::test_thread::run() auto lifetime = _client._dist_lifetime(_client._rand); for (auto i = 0; i < lifetime; ++i) { - do_connection(); + do_connection(i); } } catch (...) { // capture the first exception -- 2.35.1 -- You received this message because you are subscribed to the Google Groups "OSv Development" group. To unsubscribe from this group and stop receiving emails from it, send an email to osv-dev+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/osv-dev/20220628223749.131041-1-jwkozaczuk%40gmail.com.