From e6d57ccd5783b7be30bb3c1dab2a7676da0382ed Mon Sep 17 00:00:00 2001 From: Olaf Buddenhagen Date: Sun, 10 Apr 2016 18:39:45 +0200 Subject: [PATCH 01/33] Linux: Rename `get_maximum_send_size()` -> `get_system_sendbuf_size()` This should better communicate the actual meaning of this value. Also updated some comments to reflect the true meaning. --- platform/linux/mod.rs | 19 +++++++++++-------- platform/test.rs | 5 ++--- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/platform/linux/mod.rs b/platform/linux/mod.rs index eda23e258..a126f4bec 100644 --- a/platform/linux/mod.rs +++ b/platform/linux/mod.rs @@ -114,19 +114,22 @@ impl UnixSender { } } - /// Maximum total data size that can be transferred over this channel in a single packet. - pub fn get_maximum_send_size(&self) -> Result { + /// Maximum size of the kernel buffer used for transfers over this channel. + /// + /// Note: This is *not* the actual maximal packet size we are allowed to use... + /// Some of it is reserved by the kernel for bookkeeping. + pub fn get_system_sendbuf_size(&self) -> Result { unsafe { - let mut maximum_send_size: usize = 0; - let mut maximum_send_size_len = mem::size_of::() as socklen_t; + let mut socket_sendbuf_size: usize = 0; + let mut socket_sendbuf_size_len = mem::size_of::() as socklen_t; if getsockopt(self.fd, libc::SOL_SOCKET, libc::SO_SNDBUF, - &mut maximum_send_size as *mut usize as *mut c_void, - &mut maximum_send_size_len as *mut socklen_t) < 0 { + &mut socket_sendbuf_size as *mut usize as *mut c_void, + &mut socket_sendbuf_size_len as *mut socklen_t) < 0 { return Err(UnixError::last()) } - Ok(maximum_send_size) + Ok(socket_sendbuf_size) } } @@ -224,7 +227,7 @@ impl UnixSender { channels.push(UnixChannel::Receiver(dedicated_rx)); let (msghdr, mut iovec) = construct_header(&channels, &shared_memory_regions, &data_buffer); - let mut bytes_per_fragment = try!(self.get_maximum_send_size()) + let mut bytes_per_fragment = try!(self.get_system_sendbuf_size()) - (mem::size_of::() * 2 + msghdr.msg_controllen as usize + 256); diff --git a/platform/test.rs b/platform/test.rs index d344ac27e..9c90ca2da 100644 --- a/platform/test.rs +++ b/platform/test.rs @@ -182,13 +182,12 @@ fn full_packet() { // Should be the biggest size that just fits in a single packet. // - // 32 is the empirical minimal size of the "control message" header, - // which we presently always send along with the data; + // 32 is the empirical size reseved by the kernel; // the rest is for the fragment header. // // Note that this calculation might become imprecise // when certain implementation details of the send() method change... - let size = tx.get_maximum_send_size().unwrap() - 32 - mem::size_of::() * 2; + let size = tx.get_system_sendbuf_size().unwrap() - 32 - mem::size_of::() * 2; let data: Vec = (0..size).map(|i| (i % 251) as u8).collect(); let data: &[u8] = &data[..]; From e4f695e9cef7d5c123d84f056742219d58a0e51b Mon Sep 17 00:00:00 2001 From: Olaf Buddenhagen Date: Sun, 10 Apr 2016 03:54:54 +0200 Subject: [PATCH 02/33] Add some more tests for sizes around the maximum fragment size Lots of corner cases here that can break when changing implementation details... As these tests are all platform-specific, put them in a separate module, to avoid redundant platform conditionals. --- platform/test.rs | 114 ++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 93 insertions(+), 21 deletions(-) diff --git a/platform/test.rs b/platform/test.rs index 9c90ca2da..8d0bb7ead 100644 --- a/platform/test.rs +++ b/platform/test.rs @@ -10,7 +10,6 @@ use libc; use platform::{self, OsIpcChannel, OsIpcReceiverSet, OsIpcSender, OsIpcOneShotServer}; use platform::{OsIpcSharedMemory}; -use std::mem; use std::sync::Arc; use std::time::{Duration, Instant}; use std::thread; @@ -174,30 +173,103 @@ fn big_data_with_sender_transfer() { thread.join().unwrap(); } -#[test] -// This test only applies to platforms that need fragmentation. -#[cfg(target_os="linux")] -fn full_packet() { - let (tx, rx) = platform::channel().unwrap(); - - // Should be the biggest size that just fits in a single packet. - // - // 32 is the empirical size reseved by the kernel; - // the rest is for the fragment header. - // - // Note that this calculation might become imprecise - // when certain implementation details of the send() method change... - let size = tx.get_system_sendbuf_size().unwrap() - 32 - mem::size_of::() * 2; +fn with_n_fds(n: usize, size: usize) { + let (sender_fds, receivers): (Vec<_>, Vec<_>) = (0..n).map(|_| platform::channel().unwrap()) + .map(|(tx, rx)| (OsIpcChannel::Sender(tx), rx)) + .unzip(); + let (super_tx, super_rx) = platform::channel().unwrap(); let data: Vec = (0..size).map(|i| (i % 251) as u8).collect(); - let data: &[u8] = &data[..]; - - tx.send(data, vec![], vec![]).unwrap(); + super_tx.send(&data[..], sender_fds, vec![]).unwrap(); let (mut received_data, received_channels, received_shared_memory_regions) = - rx.recv().unwrap(); + super_rx.recv().unwrap(); + received_data.truncate(size); - assert_eq!((&received_data[..], received_channels, received_shared_memory_regions), - (&data[..], vec![], vec![])); + assert_eq!(received_data.len(), data.len()); + assert_eq!(&received_data[..], &data[..]); + assert_eq!(received_channels.len(), receivers.len()); + assert_eq!(received_shared_memory_regions.len(), 0); + + let data: Vec = (0..65536).map(|i| (i % 251) as u8).collect(); + for (mut sender_fd, sub_rx) in received_channels.into_iter().zip(receivers.into_iter()) { + let sub_tx = sender_fd.to_sender(); + sub_tx.send(&data[..], vec![], vec![]).unwrap(); + let (mut received_data, received_channels, received_shared_memory_regions) = + sub_rx.recv().unwrap(); + received_data.truncate(65536); + assert_eq!(received_data.len(), data.len()); + assert_eq!((&received_data[..], received_channels, received_shared_memory_regions), + (&data[..], vec![], vec![])); + } +} + +// These tests only apply to platforms that need fragmentation. +#[cfg(target_os="linux")] +mod fragment_tests { + use platform; + use std::mem; + use super::with_n_fds; + + lazy_static! { + static ref FRAGMENT_SIZE: usize = { + // Should be the biggest size that just fits in a single packet. + // + // 32 is the empirical size reseved by the kernel; + // the rest is for the fragment header. + // + // Note that this calculation might become imprecise + // when certain implementation details of the send() method change... + platform::channel().and_then(|(tx, _)| tx.get_system_sendbuf_size()).unwrap() + - 32 - mem::size_of::() * 2 + }; + } + + #[test] + fn full_packet() { + with_n_fds(0, *FRAGMENT_SIZE); + } + + #[test] + fn full_packet_with_1_fds() { + with_n_fds(1, *FRAGMENT_SIZE); + } + #[test] + fn full_packet_with_2_fds() { + with_n_fds(2, *FRAGMENT_SIZE); + } + #[test] + fn full_packet_with_3_fds() { + with_n_fds(3, *FRAGMENT_SIZE); + } + #[test] + fn full_packet_with_4_fds() { + with_n_fds(4, *FRAGMENT_SIZE); + } + #[test] + fn full_packet_with_5_fds() { + with_n_fds(5, *FRAGMENT_SIZE); + } + #[test] + fn full_packet_with_6_fds() { + with_n_fds(6, *FRAGMENT_SIZE); + } + + // MAX_FDS_IN_CMSG is currently 64. + #[test] + fn full_packet_with_64_fds() { + with_n_fds(64, *FRAGMENT_SIZE); + } + + #[test] + fn overfull_packet() { + with_n_fds(0, *FRAGMENT_SIZE + 1); + } + + // In fragmented transfers, one FD is used up for the dedicated channel. + #[test] + fn overfull_packet_with_63_fds() { + with_n_fds(63, *FRAGMENT_SIZE + 1); + } } macro_rules! create_big_data_with_n_fds { From 14386282df347a66527dd0e24d8e485bc42f8289 Mon Sep 17 00:00:00 2001 From: Olaf Buddenhagen Date: Tue, 19 Apr 2016 01:10:29 +0200 Subject: [PATCH 03/33] Linux: Cleanup: Remove casts between `usize` and `size_t` `libc::size_t` is an alias for `usize` -- so the casts are unnecessary, and only bloat the code, thus reducing readability (especially when they necessitate extra parentheses); and (as @mbrubeck pointed out) they actually become a liability when the involved types change, as they can silently turn into real casts, thus obscuring a potential need for code adaptations. --- platform/linux/mod.rs | 43 +++++++++++++++++++++---------------------- 1 file changed, 21 insertions(+), 22 deletions(-) diff --git a/platform/linux/mod.rs b/platform/linux/mod.rs index a126f4bec..a7d776cbd 100644 --- a/platform/linux/mod.rs +++ b/platform/linux/mod.rs @@ -153,8 +153,8 @@ impl UnixSender { -> (msghdr, Box) { let cmsg_length = (channels.len() + shared_memory_regions.len()) * mem::size_of::(); - let cmsg_buffer = libc::malloc(CMSG_SPACE(cmsg_length as size_t)) as *mut cmsghdr; - (*cmsg_buffer).cmsg_len = CMSG_LEN(cmsg_length as size_t); + let cmsg_buffer = libc::malloc(CMSG_SPACE(cmsg_length)) as *mut cmsghdr; + (*cmsg_buffer).cmsg_len = CMSG_LEN(cmsg_length); (*cmsg_buffer).cmsg_level = libc::SOL_SOCKET; (*cmsg_buffer).cmsg_type = SCM_RIGHTS; @@ -172,7 +172,7 @@ impl UnixSender { // Put this on the heap so address remains stable across function return. let mut iovec = Box::new(iovec { iov_base: data_buffer.as_ptr() as *const c_char as *mut c_char, - iov_len: data_buffer.len() as size_t, + iov_len: data_buffer.len(), }); let msghdr = msghdr { @@ -181,7 +181,7 @@ impl UnixSender { msg_iov: &mut *iovec, msg_iovlen: 1, msg_control: cmsg_buffer as *mut c_void, - msg_controllen: CMSG_SPACE(cmsg_length as size_t), + msg_controllen: CMSG_SPACE(cmsg_length), msg_flags: 0, }; @@ -229,7 +229,7 @@ impl UnixSender { let mut bytes_per_fragment = try!(self.get_system_sendbuf_size()) - (mem::size_of::() * 2 - + msghdr.msg_controllen as usize + 256); + + msghdr.msg_controllen + 256); // Split up the packet into fragments. let mut byte_position = 0; @@ -262,14 +262,14 @@ impl UnixSender { // Better reset this in case `data_buffer` moved around -- iterator // invalidation! iovec.iov_base = data_buffer.as_ptr() as *const c_char as *mut c_char; - iovec.iov_len = bytes_to_send as size_t; + iovec.iov_len = bytes_to_send; sendmsg(self.fd, &msghdr, 0) } else { // Trailing fragment. libc::send(dedicated_tx.fd, data_buffer.as_ptr() as *const c_void, - bytes_to_send as size_t, + bytes_to_send, 0) }; @@ -311,10 +311,9 @@ impl UnixSender { }; libc::strncpy(sockaddr.sun_path.as_mut_ptr(), name.as_ptr(), - sockaddr.sun_path.len() as size_t - 1); + sockaddr.sun_path.len() - 1); - let len = mem::size_of::() + - (libc::strlen(sockaddr.sun_path.as_ptr()) as usize); + let len = mem::size_of::() + libc::strlen(sockaddr.sun_path.as_ptr()); if libc::connect(fd, &sockaddr as *const _ as *const sockaddr, len as socklen_t) < 0 { return Err(UnixError::last()) } @@ -492,7 +491,7 @@ impl UnixOneShotServer { }; libc::strncpy(sockaddr.sun_path.as_mut_ptr(), path.as_ptr() as *const c_char, - sockaddr.sun_path.len() as size_t - 1); + sockaddr.sun_path.len() - 1); let len = mem::size_of::() + (libc::strlen(sockaddr.sun_path.as_ptr()) as usize); @@ -574,7 +573,7 @@ impl Drop for UnixSharedMemory { fn drop(&mut self) { unsafe { if !self.ptr.is_null() { - let result = libc::munmap(self.ptr as *mut c_void, self.length as size_t); + let result = libc::munmap(self.ptr as *mut c_void, self.length); assert!(thread::panicking() || result == 0); } let result = libc::close(self.fd); @@ -587,7 +586,7 @@ impl Clone for UnixSharedMemory { fn clone(&self) -> UnixSharedMemory { unsafe { let new_fd = libc::dup(self.fd); - let (address, _) = map_file(new_fd, Some(self.length as size_t)); + let (address, _) = map_file(new_fd, Some(self.length)); UnixSharedMemory::from_raw_parts(address, self.length, new_fd) } } @@ -627,13 +626,13 @@ impl UnixSharedMemory { unsafe fn from_fd(fd: c_int) -> UnixSharedMemory { let (ptr, length) = map_file(fd, None); - UnixSharedMemory::from_raw_parts(ptr, length as usize, fd) + UnixSharedMemory::from_raw_parts(ptr, length, fd) } pub fn from_byte(byte: u8, length: usize) -> UnixSharedMemory { unsafe { let fd = create_memory_backing_store(length); - let (address, _) = map_file(fd, Some(length as size_t)); + let (address, _) = map_file(fd, Some(length)); for element in slice::from_raw_parts_mut(address, length) { *element = byte; } @@ -644,7 +643,7 @@ impl UnixSharedMemory { pub fn from_bytes(bytes: &[u8]) -> UnixSharedMemory { unsafe { let fd = create_memory_backing_store(bytes.len()); - let (address, _) = map_file(fd, Some(bytes.len() as size_t)); + let (address, _) = map_file(fd, Some(bytes.len())); ptr::copy_nonoverlapping(bytes.as_ptr(), address, bytes.len()); UnixSharedMemory::from_raw_parts(address, bytes.len(), fd) } @@ -704,7 +703,7 @@ fn recv(fd: c_int, blocking_mode: BlockingMode) let channel_length = if cmsg_length == 0 { 0 } else { - ((cmsg.cmsg_len() as usize) - mem::size_of::()) / mem::size_of::() + (cmsg.cmsg_len() - mem::size_of::()) / mem::size_of::() }; let (mut channels, mut shared_memory_regions) = (Vec::new(), Vec::new()); for index in 0..channel_length { @@ -836,7 +835,7 @@ impl UnixCmsg { mem::size_of::(); assert!(maximum_recv_size > cmsg_length); let mut data_buffer: Vec = vec![0; maximum_recv_size]; - let cmsg_buffer = libc::malloc(cmsg_length as size_t) as *mut cmsghdr; + let cmsg_buffer = libc::malloc(cmsg_length) as *mut cmsghdr; let mut iovec = Box::new(iovec { iov_base: &mut data_buffer[0] as *mut _ as *mut c_char, iov_len: data_buffer.len(), @@ -852,7 +851,7 @@ impl UnixCmsg { msg_iov: iovec_ptr, msg_iovlen: 1, msg_control: cmsg_buffer as *mut c_void, - msg_controllen: cmsg_length as size_t, + msg_controllen: cmsg_length, msg_flags: 0, }, } @@ -913,17 +912,17 @@ type nfds_t = c_ulong; #[allow(non_snake_case)] fn CMSG_LEN(length: size_t) -> size_t { - CMSG_ALIGN(mem::size_of::() as size_t) + length + CMSG_ALIGN(mem::size_of::()) + length } #[allow(non_snake_case)] fn CMSG_ALIGN(length: size_t) -> size_t { - (length + (mem::size_of::() as size_t) - 1) & ((!(mem::size_of::() - 1)) as size_t) + (length + mem::size_of::() - 1) & !(mem::size_of::() - 1) } #[allow(non_snake_case)] fn CMSG_SPACE(length: size_t) -> size_t { - CMSG_ALIGN(length) + CMSG_ALIGN(mem::size_of::() as size_t) + CMSG_ALIGN(length) + CMSG_ALIGN(mem::size_of::()) } #[allow(non_snake_case)] From 46e9f8f733424a3b22f4f498b290eb17219d1301 Mon Sep 17 00:00:00 2001 From: Olaf Buddenhagen Date: Tue, 19 Apr 2016 01:21:43 +0200 Subject: [PATCH 04/33] Linux: Cleanup: Return `usize` instead of `ssize_t` from `UnixCmsg.recv()` The function actually processes the result of the system call before returning it, checking for negative values among other things -- so there is no need to return a sized type, nor a libc-specific one. The internal type should be abstracted by this kind of wrapper function, so callers don't need ugly casts on each invocation. --- platform/linux/mod.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/platform/linux/mod.rs b/platform/linux/mod.rs index a7d776cbd..144095a86 100644 --- a/platform/linux/mod.rs +++ b/platform/linux/mod.rs @@ -696,7 +696,7 @@ fn recv(fd: c_int, blocking_mode: BlockingMode) } let mut cmsg = UnixCmsg::new(maximum_recv_size); - let bytes_read = try!(cmsg.recv(fd, blocking_mode)) as usize; + let bytes_read = try!(cmsg.recv(fd, blocking_mode)); let cmsg_fds = cmsg.cmsg_buffer.offset(1) as *const u8 as *const c_int; let cmsg_length = cmsg.msghdr.msg_controllen; @@ -741,7 +741,7 @@ fn recv(fd: c_int, blocking_mode: BlockingMode) // Always use blocking mode for followup fragments, // to make sure that once we start receiving a multi-fragment message, // we don't abort in the middle of it... - let bytes_read = try!(cmsg.recv(dedicated_rx.fd, BlockingMode::Blocking)) as usize; + let bytes_read = try!(cmsg.recv(dedicated_rx.fd, BlockingMode::Blocking)); let this_fragment_id = (&cmsg.data_buffer[0..mem::size_of::()]).read_u32::().unwrap(); @@ -858,7 +858,7 @@ impl UnixCmsg { } unsafe fn recv(&mut self, fd: c_int, blocking_mode: BlockingMode) - -> Result { + -> Result { if let BlockingMode::Nonblocking = blocking_mode { if libc::fcntl(fd, libc::F_SETFL, libc::O_NONBLOCK) < 0 { return Err(UnixError::last()) @@ -867,7 +867,7 @@ impl UnixCmsg { let result = recvmsg(fd, &mut self.msghdr, 0); let result = if result > 0 { - Ok(result) + Ok(result as usize) } else if result == 0 { Err(UnixError(libc::ECONNRESET)) } else { From d0c3938ca6215cb5fda72e49168c4ed2996342f9 Mon Sep 17 00:00:00 2001 From: Olaf Buddenhagen Date: Sat, 9 Apr 2016 20:00:02 +0200 Subject: [PATCH 05/33] Linux: Refactor: Move FD array assembly out of `construct_header()` This avoids redundant processing; and will also enable further cleanups. --- platform/linux/mod.rs | 33 ++++++++++++++++----------------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/platform/linux/mod.rs b/platform/linux/mod.rs index 144095a86..0b19a8a3f 100644 --- a/platform/linux/mod.rs +++ b/platform/linux/mod.rs @@ -135,9 +135,18 @@ impl UnixSender { pub fn send(&self, data: &[u8], - mut channels: Vec, + channels: Vec, shared_memory_regions: Vec) -> Result<(),UnixError> { + + let mut fds = Vec::new(); + for channel in channels.iter() { + fds.push(channel.fd()); + } + for shared_memory_region in shared_memory_regions.iter() { + fds.push(shared_memory_region.fd); + } + let mut data_buffer = vec![0; data.len() + mem::size_of::() * 2]; { let mut data_buffer = &mut data_buffer[..]; @@ -147,24 +156,13 @@ impl UnixSender { } unsafe { - unsafe fn construct_header(channels: &[UnixChannel], - shared_memory_regions: &[UnixSharedMemory], - data_buffer: &[u8]) - -> (msghdr, Box) { - let cmsg_length = - (channels.len() + shared_memory_regions.len()) * mem::size_of::(); + unsafe fn construct_header(fds: &[c_int], data_buffer: &[u8]) -> (msghdr, Box) { + let cmsg_length = fds.len() * mem::size_of::(); let cmsg_buffer = libc::malloc(CMSG_SPACE(cmsg_length)) as *mut cmsghdr; (*cmsg_buffer).cmsg_len = CMSG_LEN(cmsg_length); (*cmsg_buffer).cmsg_level = libc::SOL_SOCKET; (*cmsg_buffer).cmsg_type = SCM_RIGHTS; - let mut fds = Vec::new(); - for channel in channels.iter() { - fds.push(channel.fd()); - } - for shared_memory_region in shared_memory_regions.iter() { - fds.push(shared_memory_region.fd); - } ptr::copy_nonoverlapping(fds.as_ptr(), cmsg_buffer.offset(1) as *mut _ as *mut c_int, fds.len()); @@ -190,7 +188,7 @@ impl UnixSender { (msghdr, iovec) }; - let (msghdr, _iovec) = construct_header(&channels, &shared_memory_regions, &data_buffer); + let (msghdr, _iovec) = construct_header(&fds[..], &data_buffer[..]); let result = sendmsg(self.fd, &msghdr, 0); libc::free(msghdr.msg_control); @@ -224,8 +222,9 @@ impl UnixSender { // The receiver end of the channel is sent with the first fragment // along any other file descriptors that are to be transferred in the message. let (dedicated_tx, dedicated_rx) = try!(channel()); - channels.push(UnixChannel::Receiver(dedicated_rx)); - let (msghdr, mut iovec) = construct_header(&channels, &shared_memory_regions, &data_buffer); + // Extract FD handle without consuming the Receiver, so the FD doesn't get closed. + fds.push(dedicated_rx.fd); + let (msghdr, mut iovec) = construct_header(&fds[..], &data_buffer[..]); let mut bytes_per_fragment = try!(self.get_system_sendbuf_size()) - (mem::size_of::() * 2 From ca14b46444934ba2f5d71bd5f61e3d87ff88bc94 Mon Sep 17 00:00:00 2001 From: Olaf Buddenhagen Date: Sat, 9 Apr 2016 22:09:09 +0200 Subject: [PATCH 06/33] Linux: Cleanup: Use mem::size_of_val() instead of manual calculation This variant is not only more compact and elegant, but also type-safe. I don't think it has any downsides. --- platform/linux/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/platform/linux/mod.rs b/platform/linux/mod.rs index 0b19a8a3f..3365185cb 100644 --- a/platform/linux/mod.rs +++ b/platform/linux/mod.rs @@ -157,7 +157,7 @@ impl UnixSender { unsafe { unsafe fn construct_header(fds: &[c_int], data_buffer: &[u8]) -> (msghdr, Box) { - let cmsg_length = fds.len() * mem::size_of::(); + let cmsg_length = mem::size_of_val(fds); let cmsg_buffer = libc::malloc(CMSG_SPACE(cmsg_length)) as *mut cmsghdr; (*cmsg_buffer).cmsg_len = CMSG_LEN(cmsg_length); (*cmsg_buffer).cmsg_level = libc::SOL_SOCKET; From 3a81025e14a27059b15f42ccdc63b30ed3aed6c9 Mon Sep 17 00:00:00 2001 From: Olaf Buddenhagen Date: Tue, 12 Apr 2016 01:15:51 +0200 Subject: [PATCH 07/33] Linux: Don't deduct auxiliary data size from main buffer size When calculating the maximum size of data we can send in a single fragment, no longer deduct any amount "dynamically" based on the size of the auxiliary data (FDs) transferred in the control message. I'm not sure what originally prompted the idea that deducting this from the main buffer would be necessary -- my testing at least doesn't show any need for that. The auxiliary data is transferred in a separate buffer with its own size limitation. (Defaulting to 10 KiB on my system.) --- platform/linux/mod.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/platform/linux/mod.rs b/platform/linux/mod.rs index 3365185cb..bd3b52d00 100644 --- a/platform/linux/mod.rs +++ b/platform/linux/mod.rs @@ -227,8 +227,7 @@ impl UnixSender { let (msghdr, mut iovec) = construct_header(&fds[..], &data_buffer[..]); let mut bytes_per_fragment = try!(self.get_system_sendbuf_size()) - - (mem::size_of::() * 2 - + msghdr.msg_controllen + 256); + - (mem::size_of::() * 2 + 256); // Split up the packet into fragments. let mut byte_position = 0; From 65d964eb7d343b1a53bdd60218728818fc22ddcc Mon Sep 17 00:00:00 2001 From: Olaf Buddenhagen Date: Sun, 17 Apr 2016 15:13:07 +0200 Subject: [PATCH 08/33] Linux: Cleanup: Turn magic value into `RESERVED_SIZE` symbolic constant --- platform/linux/mod.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/platform/linux/mod.rs b/platform/linux/mod.rs index bd3b52d00..79f2947cf 100644 --- a/platform/linux/mod.rs +++ b/platform/linux/mod.rs @@ -28,6 +28,10 @@ const MAX_FDS_IN_CMSG: u32 = 64; // Yes, really! const MAP_FAILED: *mut u8 = (!0usize) as *mut u8; +// The value Linux returns for SO_SNDBUF +// is not the size we are actually allowed to use... +const RESERVED_SIZE: usize = 256; + static LAST_FRAGMENT_ID: AtomicUsize = ATOMIC_USIZE_INIT; pub fn channel() -> Result<(UnixSender, UnixReceiver),UnixError> { @@ -227,7 +231,7 @@ impl UnixSender { let (msghdr, mut iovec) = construct_header(&fds[..], &data_buffer[..]); let mut bytes_per_fragment = try!(self.get_system_sendbuf_size()) - - (mem::size_of::() * 2 + 256); + - (mem::size_of::() * 2 + RESERVED_SIZE); // Split up the packet into fragments. let mut byte_position = 0; From 6d94eed1d65b794ace5c6010813c0274e3c45a99 Mon Sep 17 00:00:00 2001 From: Olaf Buddenhagen Date: Sat, 30 Apr 2016 00:46:58 +0200 Subject: [PATCH 09/33] Linux: Don't over-allocate followup fragment receive buffers As we explicitly size all fragments in a fragmented send, the maximum size of followup packets is always well known -- so there is no need to allocate any extra just in case. (Unlike for the first fragment, which is implicitly sized if it comes from an unfragmented send; and thus might potentially have a larger size than what we expect, in case our reserved size doesn't match reality...) --- platform/linux/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/platform/linux/mod.rs b/platform/linux/mod.rs index 79f2947cf..5b124b022 100644 --- a/platform/linux/mod.rs +++ b/platform/linux/mod.rs @@ -739,7 +739,7 @@ fn recv(fd: c_int, blocking_mode: BlockingMode) // through which all the remaining fragments will be coming in. let dedicated_rx = channels.pop().unwrap().to_receiver(); while next_fragment_id != 0 { - let mut cmsg = UnixCmsg::new(maximum_recv_size); + let mut cmsg = UnixCmsg::new(maximum_recv_size - RESERVED_SIZE); // Always use blocking mode for followup fragments, // to make sure that once we start receiving a multi-fragment message, // we don't abort in the middle of it... From ef7424739bc5502ddb52566f5720333e233060db Mon Sep 17 00:00:00 2001 From: Olaf Buddenhagen Date: Tue, 12 Apr 2016 01:15:51 +0200 Subject: [PATCH 10/33] Linux: Only deduct 32 bytes from maximum send size reported by kernel I wonder whether there is some obscure documentation I'm not aware of that suggests we have to deduct 256 bytes? According to my testing with Linux 4.4 on i386 as well as Linux 3.16 on x86_64, only 32 bytes actually need to be deducted. --- platform/linux/mod.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/platform/linux/mod.rs b/platform/linux/mod.rs index 5b124b022..4c7285388 100644 --- a/platform/linux/mod.rs +++ b/platform/linux/mod.rs @@ -30,7 +30,8 @@ const MAP_FAILED: *mut u8 = (!0usize) as *mut u8; // The value Linux returns for SO_SNDBUF // is not the size we are actually allowed to use... -const RESERVED_SIZE: usize = 256; +// Empirically, we have to deduct 32 bytes from that. +const RESERVED_SIZE: usize = 32; static LAST_FRAGMENT_ID: AtomicUsize = ATOMIC_USIZE_INIT; From 04ac943a97bcf2a35e6fdc9a0c6965a6a65dfc96 Mon Sep 17 00:00:00 2001 From: Olaf Buddenhagen Date: Tue, 12 Apr 2016 23:13:07 +0200 Subject: [PATCH 11/33] Linux: Cleanup: Improve size recalculation on ENOBUFS Rather than halving the payload size, halve the total buffer size, and recalculate the payload size from that. This way, the buffer size remains a (more or less) round number, rather than becoming something just slightly above a round number -- which should improve resource utilisation, and might even reduce the number of downsizes necessary in some cases. This will also faciliate further cleanups. --- platform/linux/mod.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/platform/linux/mod.rs b/platform/linux/mod.rs index 4c7285388..f24f7a803 100644 --- a/platform/linux/mod.rs +++ b/platform/linux/mod.rs @@ -231,18 +231,18 @@ impl UnixSender { fds.push(dedicated_rx.fd); let (msghdr, mut iovec) = construct_header(&fds[..], &data_buffer[..]); - let mut bytes_per_fragment = try!(self.get_system_sendbuf_size()) - - (mem::size_of::() * 2 + RESERVED_SIZE); - // Split up the packet into fragments. + let mut sendbuf_size = try!(self.get_system_sendbuf_size()); let mut byte_position = 0; let mut this_fragment_id = 0; while byte_position < data.len() { if downsize { // We got ENOBUFS. Retry send with half the packet size. - bytes_per_fragment /= 2; + sendbuf_size /= 2; downsize = false; } + let bytes_per_fragment = sendbuf_size + - (mem::size_of::() * 2 + RESERVED_SIZE); let end_byte_position = cmp::min(data.len(), byte_position + bytes_per_fragment); let next_fragment_id = if end_byte_position == data.len() { From f2b1a02bc780538c0ffba7c5580a879884e5f5a5 Mon Sep 17 00:00:00 2001 From: Olaf Buddenhagen Date: Tue, 26 Apr 2016 03:16:33 +0200 Subject: [PATCH 12/33] Linux: Refactor: Introduce `downsize()` function Instead of setting a flag in two places and handling it in one, just move the handling to a (very simple) function that can be directly used in both places. This should make the code more robust and easier to follow. As a side effect, the safeguard against unexpected ENOBUFS is now executed in both places, which is actually more correct I guess. (Though the guard only protects against a hypothetical case anyway, which I don't actually expect ever to come up...) Note: I'm not entirely happy with how the safeguard is handled syntactically -- but I can't think of any more elegant approach... --- platform/linux/mod.rs | 46 ++++++++++++++++++++++--------------------- 1 file changed, 24 insertions(+), 22 deletions(-) diff --git a/platform/linux/mod.rs b/platform/linux/mod.rs index f24f7a803..dea9d27cc 100644 --- a/platform/linux/mod.rs +++ b/platform/linux/mod.rs @@ -193,27 +193,41 @@ impl UnixSender { (msghdr, iovec) }; + let mut sendbuf_size = try!(self.get_system_sendbuf_size()); + + /// Reduce send buffer size after getting ENOBUFS, + /// i.e. when the kernel failed to allocate a large enough buffer. + /// + /// (If the buffer already was significantly smaller + /// than the memory page size though, + /// if means something else must have gone wrong; + /// so there is no point in further downsizing, + /// and we error out instead.) + fn downsize(sendbuf_size: &mut usize, sent_size: usize) -> Result<(),()> { + if sent_size > 2000 { + *sendbuf_size /= 2; + Ok(()) + } else { + Err(()) + } + } + let (msghdr, _iovec) = construct_header(&fds[..], &data_buffer[..]); let result = sendmsg(self.fd, &msghdr, 0); libc::free(msghdr.msg_control); - let mut downsize = false; - if result > 0 { return Ok(()) } else { let error = UnixError::last(); - if error.0 == libc::ENOBUFS { + if error.0 == libc::ENOBUFS + && downsize(&mut sendbuf_size, data_buffer.len()).is_ok() { // If we get this error, // it means the message was small enough to fit the maximum send size, // but the kernel failed to allocate a buffer large enough // to actually transfer the message -- // so we have to proceed with a fragmented send nevertheless. - // - // The flag indicates that packets need to be smaller - // than the ordinary maximum send size. - downsize = true; } else if error.0 != libc::EMSGSIZE { return Err(error) } @@ -232,15 +246,9 @@ impl UnixSender { let (msghdr, mut iovec) = construct_header(&fds[..], &data_buffer[..]); // Split up the packet into fragments. - let mut sendbuf_size = try!(self.get_system_sendbuf_size()); let mut byte_position = 0; let mut this_fragment_id = 0; while byte_position < data.len() { - if downsize { - // We got ENOBUFS. Retry send with half the packet size. - sendbuf_size /= 2; - downsize = false; - } let bytes_per_fragment = sendbuf_size - (mem::size_of::() * 2 + RESERVED_SIZE); @@ -278,16 +286,10 @@ impl UnixSender { if result <= 0 { let error = UnixError::last(); - if error.0 == libc::ENOBUFS && bytes_to_send > 2000 { + if error.0 == libc::ENOBUFS + && downsize(&mut sendbuf_size, bytes_to_send).is_ok() { // If the kernel failed to allocate a buffer large enough for the packet, - // retry with a smaller size. - // - // (If the packet was already significantly smaller - // than the memory page size though, - // if means something else must have gone wrong; - // so there is no point in further downsizing, - // and we error out instead.) - downsize = true; + // retry with a smaller size (if possible). continue } else { libc::free(msghdr.msg_control); From 37d9bb78dcfb45e047b72f5598cafce1f921a02d Mon Sep 17 00:00:00 2001 From: Olaf Buddenhagen Date: Sun, 17 Apr 2016 17:04:11 +0200 Subject: [PATCH 13/33] Linux: Refactor: Split out `UnixSender::fragment_size()` Move fragment size calculation into a separate (static) method. This should help keeping these calculations consistent when certain implementation details change. --- platform/linux/mod.rs | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/platform/linux/mod.rs b/platform/linux/mod.rs index dea9d27cc..3a896d4f7 100644 --- a/platform/linux/mod.rs +++ b/platform/linux/mod.rs @@ -138,6 +138,21 @@ impl UnixSender { } } + /// Calculate maximum payload data size per fragment. + /// + /// This is the size of the main data chunk only -- + /// it's independent of any auxiliary data (FDs) transferred along with it. + /// It is the total size of the kernel buffer, + /// minus the part reserved by the kernel, + /// and with the size of the fragment header also deducted from it. + /// + /// The `sendbuf_size` passed in should usually be the maximum kernel buffer size, + /// as obtained with `get_system_sendbuf_size()` -- + /// except after getting ENOBUFS, in which case it needs to be reduced. + fn fragment_size(sendbuf_size: usize) -> usize { + sendbuf_size - RESERVED_SIZE - mem::size_of::() * 2 + } + pub fn send(&self, data: &[u8], channels: Vec, @@ -249,8 +264,7 @@ impl UnixSender { let mut byte_position = 0; let mut this_fragment_id = 0; while byte_position < data.len() { - let bytes_per_fragment = sendbuf_size - - (mem::size_of::() * 2 + RESERVED_SIZE); + let bytes_per_fragment = Self::fragment_size(sendbuf_size); let end_byte_position = cmp::min(data.len(), byte_position + bytes_per_fragment); let next_fragment_id = if end_byte_position == data.len() { From 1e9ac09c896c437680ee5f720c771f4da69aef31 Mon Sep 17 00:00:00 2001 From: Olaf Buddenhagen Date: Sun, 17 Apr 2016 18:02:05 +0200 Subject: [PATCH 14/33] Linux: Cleanup: Provide a `get_max_fragment_size()` method This combines `fragment_size()` with `get_system_sendbuf_size()` for uses that require the *maximum* possible fragment size. (Currently only `platform/tests.rs`) Making this one public instead of `get_system_sendbuf_size()`, and adapting tests accordingly. --- platform/linux/mod.rs | 15 ++++++++++++++- platform/test.rs | 11 +---------- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/platform/linux/mod.rs b/platform/linux/mod.rs index 3a896d4f7..51a11fcf8 100644 --- a/platform/linux/mod.rs +++ b/platform/linux/mod.rs @@ -123,7 +123,7 @@ impl UnixSender { /// /// Note: This is *not* the actual maximal packet size we are allowed to use... /// Some of it is reserved by the kernel for bookkeeping. - pub fn get_system_sendbuf_size(&self) -> Result { + fn get_system_sendbuf_size(&self) -> Result { unsafe { let mut socket_sendbuf_size: usize = 0; let mut socket_sendbuf_size_len = mem::size_of::() as socklen_t; @@ -153,6 +153,19 @@ impl UnixSender { sendbuf_size - RESERVED_SIZE - mem::size_of::() * 2 } + /// Maximum data size that can be transferred over this channel in a single packet. + /// + /// This is the size of the main data chunk only -- + /// it's independent of any auxiliary data (FDs) transferred along with it. + /// + /// A send on this channel won't block for transfers up to this size + /// under normal circumstances. + /// (It might still block if heavy memory pressure causes ENOBUFS, + /// forcing us to reduce the packet size.) + pub fn get_max_fragment_size(&self) -> Result { + Ok(Self::fragment_size(try!(self.get_system_sendbuf_size()))) + } + pub fn send(&self, data: &[u8], channels: Vec, diff --git a/platform/test.rs b/platform/test.rs index 8d0bb7ead..107bb2a10 100644 --- a/platform/test.rs +++ b/platform/test.rs @@ -207,20 +207,11 @@ fn with_n_fds(n: usize, size: usize) { #[cfg(target_os="linux")] mod fragment_tests { use platform; - use std::mem; use super::with_n_fds; lazy_static! { static ref FRAGMENT_SIZE: usize = { - // Should be the biggest size that just fits in a single packet. - // - // 32 is the empirical size reseved by the kernel; - // the rest is for the fragment header. - // - // Note that this calculation might become imprecise - // when certain implementation details of the send() method change... - platform::channel().and_then(|(tx, _)| tx.get_system_sendbuf_size()).unwrap() - - 32 - mem::size_of::() * 2 + platform::channel().and_then(|(tx, _)| tx.get_max_fragment_size()).unwrap() }; } From e554671e4f017ba78866d76fbd9a8026de3c367a Mon Sep 17 00:00:00 2001 From: Olaf Buddenhagen Date: Wed, 4 May 2016 01:02:18 +0200 Subject: [PATCH 15/33] Linux: Cleanup: Make `msghdr.iovec` a `*const` While `recvmsg()` mutates the buffers referenced by the `iovec`, the `iovec` itself is never modified by either `sendmsg()` or `recvmsg()`. This field is indeed marked `*const` in `libc::msghdr` as well. --- platform/linux/mod.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/platform/linux/mod.rs b/platform/linux/mod.rs index 51a11fcf8..28c861787 100644 --- a/platform/linux/mod.rs +++ b/platform/linux/mod.rs @@ -201,7 +201,7 @@ impl UnixSender { fds.len()); // Put this on the heap so address remains stable across function return. - let mut iovec = Box::new(iovec { + let iovec = Box::new(iovec { iov_base: data_buffer.as_ptr() as *const c_char as *mut c_char, iov_len: data_buffer.len(), }); @@ -209,7 +209,7 @@ impl UnixSender { let msghdr = msghdr { msg_name: ptr::null_mut(), msg_namelen: 0, - msg_iov: &mut *iovec, + msg_iov: &*iovec, msg_iovlen: 1, msg_control: cmsg_buffer as *mut c_void, msg_controllen: CMSG_SPACE(cmsg_length), @@ -868,11 +868,11 @@ impl UnixCmsg { assert!(maximum_recv_size > cmsg_length); let mut data_buffer: Vec = vec![0; maximum_recv_size]; let cmsg_buffer = libc::malloc(cmsg_length) as *mut cmsghdr; - let mut iovec = Box::new(iovec { + let iovec = Box::new(iovec { iov_base: &mut data_buffer[0] as *mut _ as *mut c_char, iov_len: data_buffer.len(), }); - let iovec_ptr: *mut iovec = &mut *iovec; + let iovec_ptr: *const iovec = &*iovec; UnixCmsg { data_buffer: data_buffer, cmsg_buffer: cmsg_buffer, @@ -988,7 +988,7 @@ extern { struct msghdr { msg_name: *mut c_void, msg_namelen: socklen_t, - msg_iov: *mut iovec, + msg_iov: *const iovec, msg_iovlen: size_t, msg_control: *mut c_void, msg_controllen: size_t, From a43aefd9e9e746ae11c3fbe58047bd0319835e10 Mon Sep 17 00:00:00 2001 From: Olaf Buddenhagen Date: Wed, 13 Apr 2016 00:46:51 +0200 Subject: [PATCH 16/33] Linux: Cleanup: Move header construction right before use When using fragmentation, the `msghdr` structure is only used for the first packet, which is already sent in a separate conditional arm anyway -- so we can just as well create (and deallocate) the header within this conditional too. This should make the code more robust and much easier to follow. (Note that this means the header will be recreated when we have to resend on ENOBUFS. The performance impact should be negligible though; and it's an exceptional case anyways.) --- platform/linux/mod.rs | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/platform/linux/mod.rs b/platform/linux/mod.rs index 28c861787..51e198fc5 100644 --- a/platform/linux/mod.rs +++ b/platform/linux/mod.rs @@ -271,7 +271,6 @@ impl UnixSender { let (dedicated_tx, dedicated_rx) = try!(channel()); // Extract FD handle without consuming the Receiver, so the FD doesn't get closed. fds.push(dedicated_rx.fd); - let (msghdr, mut iovec) = construct_header(&fds[..], &data_buffer[..]); // Split up the packet into fragments. let mut byte_position = 0; @@ -297,12 +296,12 @@ impl UnixSender { let result = if byte_position == 0 { // First one. This fragment includes the file descriptors. - // Better reset this in case `data_buffer` moved around -- iterator - // invalidation! - iovec.iov_base = data_buffer.as_ptr() as *const c_char as *mut c_char; - iovec.iov_len = bytes_to_send; + let (msghdr, _iovec) = construct_header(&fds[..], + &data_buffer[..bytes_to_send]); - sendmsg(self.fd, &msghdr, 0) + let result = sendmsg(self.fd, &msghdr, 0); + libc::free(msghdr.msg_control); + result } else { // Trailing fragment. libc::send(dedicated_tx.fd, @@ -319,7 +318,6 @@ impl UnixSender { // retry with a smaller size (if possible). continue } else { - libc::free(msghdr.msg_control); return Err(error) } } @@ -328,7 +326,6 @@ impl UnixSender { this_fragment_id = next_fragment_id; } - libc::free(msghdr.msg_control); Ok(()) } } From a7a13731d0c1765cc5cadea89ba4c9fde1e8b152 Mon Sep 17 00:00:00 2001 From: Olaf Buddenhagen Date: Sun, 17 Apr 2016 21:19:24 +0200 Subject: [PATCH 17/33] Linux: Refactor: Turn `construct_header()` into `send_first_fragment()` Moving the actual sending of the message into the `construct_header()` function, and renaming it to `send_first_fragment()` to reflect that change. With the previous changes, we always send the message right after constructing the header -- so it makes sense to put the common sequence in one place. Removing the need to pass the header structs through the caller also gets rid of the ugly `iovec` return hack. What's more, this helps isolating the unsafe operations: invoking `send_first_fragment()` is in fact not unsafe at all. --- platform/linux/mod.rs | 86 +++++++++++++++++++++---------------------- 1 file changed, 43 insertions(+), 43 deletions(-) diff --git a/platform/linux/mod.rs b/platform/linux/mod.rs index 51e198fc5..939d9cdbe 100644 --- a/platform/linux/mod.rs +++ b/platform/linux/mod.rs @@ -188,8 +188,9 @@ impl UnixSender { data_buffer.write(data).unwrap(); } - unsafe { - unsafe fn construct_header(fds: &[c_int], data_buffer: &[u8]) -> (msghdr, Box) { + fn send_first_fragment(sender_fd: c_int, fds: &[c_int], data_buffer: &[u8]) + -> Result<(),UnixError> { + let result = unsafe { let cmsg_length = mem::size_of_val(fds); let cmsg_buffer = libc::malloc(CMSG_SPACE(cmsg_length)) as *mut cmsghdr; (*cmsg_buffer).cmsg_len = CMSG_LEN(cmsg_length); @@ -200,27 +201,33 @@ impl UnixSender { cmsg_buffer.offset(1) as *mut _ as *mut c_int, fds.len()); - // Put this on the heap so address remains stable across function return. - let iovec = Box::new(iovec { + let iovec = iovec { iov_base: data_buffer.as_ptr() as *const c_char as *mut c_char, iov_len: data_buffer.len(), - }); - + }; let msghdr = msghdr { msg_name: ptr::null_mut(), msg_namelen: 0, - msg_iov: &*iovec, + msg_iov: &iovec, msg_iovlen: 1, msg_control: cmsg_buffer as *mut c_void, msg_controllen: CMSG_SPACE(cmsg_length), msg_flags: 0, }; - // Be sure to always return iovec -- whether the caller uses it or not -- - // to prevent premature deallocation! - (msghdr, iovec) + let result = sendmsg(sender_fd, &msghdr, 0); + libc::free(cmsg_buffer as *mut c_void); + result }; + if result > 0 { + Ok(()) + } else { + Err(UnixError::last()) + } + }; + + unsafe { let mut sendbuf_size = try!(self.get_system_sendbuf_size()); /// Reduce send buffer size after getting ENOBUFS, @@ -240,25 +247,20 @@ impl UnixSender { } } - let (msghdr, _iovec) = construct_header(&fds[..], &data_buffer[..]); - - let result = sendmsg(self.fd, &msghdr, 0); - libc::free(msghdr.msg_control); - - if result > 0 { - return Ok(()) - } else { - let error = UnixError::last(); - if error.0 == libc::ENOBUFS - && downsize(&mut sendbuf_size, data_buffer.len()).is_ok() { - // If we get this error, - // it means the message was small enough to fit the maximum send size, - // but the kernel failed to allocate a buffer large enough - // to actually transfer the message -- - // so we have to proceed with a fragmented send nevertheless. - } else if error.0 != libc::EMSGSIZE { - return Err(error) - } + match send_first_fragment(self.fd, &fds[..], &data_buffer[..]) { + Ok(_) => return Ok(()), + Err(error) => { + if error.0 == libc::ENOBUFS + && downsize(&mut sendbuf_size, data_buffer.len()).is_ok() { + // If we get this error, + // it means the message was small enough to fit the maximum send size, + // but the kernel failed to allocate a buffer large enough + // to actually transfer the message -- + // so we have to proceed with a fragmented send nevertheless. + } else if error.0 != libc::EMSGSIZE { + return Err(error) + } + }, } // The packet is too big. Fragmentation time! @@ -293,25 +295,23 @@ impl UnixSender { } let bytes_to_send = end_byte_position - byte_position + mem::size_of::() * 2; - let result = if byte_position == 0 { - // First one. This fragment includes the file descriptors. - - let (msghdr, _iovec) = construct_header(&fds[..], - &data_buffer[..bytes_to_send]); - let result = sendmsg(self.fd, &msghdr, 0); - libc::free(msghdr.msg_control); - result + let result = if byte_position == 0 { + send_first_fragment(self.fd, &fds[..], &data_buffer[..bytes_to_send]) } else { // Trailing fragment. - libc::send(dedicated_tx.fd, - data_buffer.as_ptr() as *const c_void, - bytes_to_send, - 0) + let result = libc::send(dedicated_tx.fd, + data_buffer.as_ptr() as *const c_void, + bytes_to_send, + 0); + if result > 0 { + Ok(()) + } else { + Err(UnixError::last()) + } }; - if result <= 0 { - let error = UnixError::last(); + if let Err(error) = result { if error.0 == libc::ENOBUFS && downsize(&mut sendbuf_size, bytes_to_send).is_ok() { // If the kernel failed to allocate a buffer large enough for the packet, From 08dcd10d34ac4cbd3c7df0ca6db5c00a8ff577bc Mon Sep 17 00:00:00 2001 From: Olaf Buddenhagen Date: Sun, 17 Apr 2016 23:23:56 +0200 Subject: [PATCH 18/33] Linux: Refactor: Split out new `send_followup_fragment()` function Moving the `send()` call for followup fragments into a sub-function along the lines of `send_first_fragment()`. While this one is only invoked once, the commont pattern should make the code easier to read; and just like with `send_first_fragment()`, it also helps isolating the unsafe code. --- platform/linux/mod.rs | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/platform/linux/mod.rs b/platform/linux/mod.rs index 939d9cdbe..2ac7bfb9d 100644 --- a/platform/linux/mod.rs +++ b/platform/linux/mod.rs @@ -227,6 +227,21 @@ impl UnixSender { } }; + fn send_followup_fragment(sender_fd: c_int, data_buffer: &[u8]) -> Result<(),UnixError> { + let result = unsafe { + libc::send(sender_fd, + data_buffer.as_ptr() as *const c_void, + data_buffer.len(), + 0) + }; + + if result > 0 { + Ok(()) + } else { + Err(UnixError::last()) + } + } + unsafe { let mut sendbuf_size = try!(self.get_system_sendbuf_size()); @@ -299,16 +314,7 @@ impl UnixSender { let result = if byte_position == 0 { send_first_fragment(self.fd, &fds[..], &data_buffer[..bytes_to_send]) } else { - // Trailing fragment. - let result = libc::send(dedicated_tx.fd, - data_buffer.as_ptr() as *const c_void, - bytes_to_send, - 0); - if result > 0 { - Ok(()) - } else { - Err(UnixError::last()) - } + send_followup_fragment(dedicated_tx.fd, &data_buffer[..bytes_to_send]) }; if let Err(error) = result { From 06ed0c3a51883499ecce2c522244c45c7b88941e Mon Sep 17 00:00:00 2001 From: Olaf Buddenhagen Date: Sun, 17 Apr 2016 23:28:35 +0200 Subject: [PATCH 19/33] Linux: Cleanup: Remove no longer needed `unsafe` block Now that all the unsafe code is isolated in `send_*_fragment()`, the rest of the `send()` method doesn't need to be marked unsafe anymore. --- platform/linux/mod.rs | 160 +++++++++++++++++++++--------------------- 1 file changed, 79 insertions(+), 81 deletions(-) diff --git a/platform/linux/mod.rs b/platform/linux/mod.rs index 2ac7bfb9d..efc086cc9 100644 --- a/platform/linux/mod.rs +++ b/platform/linux/mod.rs @@ -242,98 +242,96 @@ impl UnixSender { } } - unsafe { - let mut sendbuf_size = try!(self.get_system_sendbuf_size()); - - /// Reduce send buffer size after getting ENOBUFS, - /// i.e. when the kernel failed to allocate a large enough buffer. - /// - /// (If the buffer already was significantly smaller - /// than the memory page size though, - /// if means something else must have gone wrong; - /// so there is no point in further downsizing, - /// and we error out instead.) - fn downsize(sendbuf_size: &mut usize, sent_size: usize) -> Result<(),()> { - if sent_size > 2000 { - *sendbuf_size /= 2; - Ok(()) - } else { - Err(()) - } + let mut sendbuf_size = try!(self.get_system_sendbuf_size()); + + /// Reduce send buffer size after getting ENOBUFS, + /// i.e. when the kernel failed to allocate a large enough buffer. + /// + /// (If the buffer already was significantly smaller + /// than the memory page size though, + /// if means something else must have gone wrong; + /// so there is no point in further downsizing, + /// and we error out instead.) + fn downsize(sendbuf_size: &mut usize, sent_size: usize) -> Result<(),()> { + if sent_size > 2000 { + *sendbuf_size /= 2; + Ok(()) + } else { + Err(()) } + } - match send_first_fragment(self.fd, &fds[..], &data_buffer[..]) { - Ok(_) => return Ok(()), - Err(error) => { - if error.0 == libc::ENOBUFS - && downsize(&mut sendbuf_size, data_buffer.len()).is_ok() { - // If we get this error, - // it means the message was small enough to fit the maximum send size, - // but the kernel failed to allocate a buffer large enough - // to actually transfer the message -- - // so we have to proceed with a fragmented send nevertheless. - } else if error.0 != libc::EMSGSIZE { - return Err(error) - } - }, - } + match send_first_fragment(self.fd, &fds[..], &data_buffer[..]) { + Ok(_) => return Ok(()), + Err(error) => { + if error.0 == libc::ENOBUFS + && downsize(&mut sendbuf_size, data_buffer.len()).is_ok() { + // If we get this error, + // it means the message was small enough to fit the maximum send size, + // but the kernel failed to allocate a buffer large enough + // to actually transfer the message -- + // so we have to proceed with a fragmented send nevertheless. + } else if error.0 != libc::EMSGSIZE { + return Err(error) + } + }, + } - // The packet is too big. Fragmentation time! - // - // Create dedicated channel to send all but the first fragment. - // This way we avoid fragments of different messages interleaving in the receiver. - // - // The receiver end of the channel is sent with the first fragment - // along any other file descriptors that are to be transferred in the message. - let (dedicated_tx, dedicated_rx) = try!(channel()); - // Extract FD handle without consuming the Receiver, so the FD doesn't get closed. - fds.push(dedicated_rx.fd); - - // Split up the packet into fragments. - let mut byte_position = 0; - let mut this_fragment_id = 0; - while byte_position < data.len() { - let bytes_per_fragment = Self::fragment_size(sendbuf_size); - - let end_byte_position = cmp::min(data.len(), byte_position + bytes_per_fragment); - let next_fragment_id = if end_byte_position == data.len() { - 0 - } else { - (LAST_FRAGMENT_ID.fetch_add(1, Ordering::SeqCst) + 1) as u32 - }; + // The packet is too big. Fragmentation time! + // + // Create dedicated channel to send all but the first fragment. + // This way we avoid fragments of different messages interleaving in the receiver. + // + // The receiver end of the channel is sent with the first fragment + // along any other file descriptors that are to be transferred in the message. + let (dedicated_tx, dedicated_rx) = try!(channel()); + // Extract FD handle without consuming the Receiver, so the FD doesn't get closed. + fds.push(dedicated_rx.fd); + + // Split up the packet into fragments. + let mut byte_position = 0; + let mut this_fragment_id = 0; + while byte_position < data.len() { + let bytes_per_fragment = Self::fragment_size(sendbuf_size); + + let end_byte_position = cmp::min(data.len(), byte_position + bytes_per_fragment); + let next_fragment_id = if end_byte_position == data.len() { + 0 + } else { + (LAST_FRAGMENT_ID.fetch_add(1, Ordering::SeqCst) + 1) as u32 + }; - { - let mut data_buffer = &mut data_buffer[..]; - data_buffer.write_u32::(this_fragment_id).unwrap(); - data_buffer.write_u32::(next_fragment_id).unwrap(); - data_buffer.write(&data[byte_position..end_byte_position]).unwrap(); - } + { + let mut data_buffer = &mut data_buffer[..]; + data_buffer.write_u32::(this_fragment_id).unwrap(); + data_buffer.write_u32::(next_fragment_id).unwrap(); + data_buffer.write(&data[byte_position..end_byte_position]).unwrap(); + } - let bytes_to_send = end_byte_position - byte_position + mem::size_of::() * 2; + let bytes_to_send = end_byte_position - byte_position + mem::size_of::() * 2; - let result = if byte_position == 0 { - send_first_fragment(self.fd, &fds[..], &data_buffer[..bytes_to_send]) - } else { - send_followup_fragment(dedicated_tx.fd, &data_buffer[..bytes_to_send]) - }; + let result = if byte_position == 0 { + send_first_fragment(self.fd, &fds[..], &data_buffer[..bytes_to_send]) + } else { + send_followup_fragment(dedicated_tx.fd, &data_buffer[..bytes_to_send]) + }; - if let Err(error) = result { - if error.0 == libc::ENOBUFS - && downsize(&mut sendbuf_size, bytes_to_send).is_ok() { - // If the kernel failed to allocate a buffer large enough for the packet, - // retry with a smaller size (if possible). - continue - } else { - return Err(error) - } + if let Err(error) = result { + if error.0 == libc::ENOBUFS + && downsize(&mut sendbuf_size, bytes_to_send).is_ok() { + // If the kernel failed to allocate a buffer large enough for the packet, + // retry with a smaller size (if possible). + continue + } else { + return Err(error) } - - byte_position += bytes_per_fragment; - this_fragment_id = next_fragment_id; } - Ok(()) + byte_position += bytes_per_fragment; + this_fragment_id = next_fragment_id; } + + Ok(()) } pub fn connect(name: String) -> Result { From 89510147f5f06772869819a535746cd673626c41 Mon Sep 17 00:00:00 2001 From: Olaf Buddenhagen Date: Sun, 14 Feb 2016 23:40:13 +0100 Subject: [PATCH 20/33] Add benchmark tests for low-level transfers Measure performance of transfers of various sizes, to keep track of the performance impact of upcoming optimisations. This makes use of `get_max_fragment_size()` outside of platform code; so it additionally necessitates adding stub implementations of this method for all platforms. The benchmark results are not as consistent as one would hope for -- but it should be good enough to judge the impact of any major changes. (See also the code comment in `benches/bench.rs` for an explanation of the `_invoke_chaos` benchmark pass...) Below are the numbers from a typical run on my system. (Which is an Intel Core2 Quad at 2.5 GHz running a 32 Bit GNU/Linux system, i.e. a pretty old system from 2008 or thereabouts.) These numbers were obtained with the cpufreq governor set to `performance` (rather than the default `ondemand`) for more reproducible results. Unfortunately, this doesn't exclude other external factors, such as memory pressure -- so it's still tricky to compare different test runs. The numbers presented here (and in the following bunch of optimisation commits) were all obtained in a single large test series; so they should be comparable -- but it's still tricky to compare results when checking the impact of any new patches... The second block of results is with `ITERATIONS` in `benches/bench.rs` increased to 100. Aside from somewhat reducing randomness in general, this is important because for fragmented transfers we need to spawn a thread in the benchmark suite, which is significantly affecting the results for medium-large transfers (especially 256 KiB and the next few ones) when using only a single iteration -- in fact dominating them once some optimisations are applied. (And also introducing lots of randomness.) Still presenting the result for one iteration as well: mostly because the absolute values have the right magnitude in this case, while adding more iterations shifts them accordingly. The largest size doesn't actually produce a result with 100 iterations, because of some integer overflow in `cargo bench` I presume. We will get results for this one as well though once some optimisations are applied. test _invoke_chaos ... bench: 4,283,639 ns/iter (+/- 369,532) test size_00_1 ... bench: 11,893 ns/iter (+/- 233) test size_01_2 ... bench: 11,786 ns/iter (+/- 111) test size_02_4 ... bench: 11,766 ns/iter (+/- 103) test size_03_8 ... bench: 11,731 ns/iter (+/- 71) test size_04_16 ... bench: 11,777 ns/iter (+/- 67) test size_05_32 ... bench: 11,823 ns/iter (+/- 87) test size_06_64 ... bench: 12,085 ns/iter (+/- 94) test size_07_128 ... bench: 12,358 ns/iter (+/- 108) test size_08_256 ... bench: 12,710 ns/iter (+/- 110) test size_09_512 ... bench: 13,674 ns/iter (+/- 151) test size_10_1k ... bench: 15,634 ns/iter (+/- 151) test size_11_2k ... bench: 19,289 ns/iter (+/- 182) test size_12_4k ... bench: 26,931 ns/iter (+/- 92) test size_13_8k ... bench: 42,751 ns/iter (+/- 234) test size_14_16k ... bench: 74,066 ns/iter (+/- 432) test size_15_32k ... bench: 137,961 ns/iter (+/- 694) test size_16_64k ... bench: 262,229 ns/iter (+/- 2,664) test size_17_128k ... bench: 509,617 ns/iter (+/- 7,176) test size_18_256k ... bench: 1,202,057 ns/iter (+/- 261,359) test size_19_512k ... bench: 2,267,058 ns/iter (+/- 403,483) test size_20_1m ... bench: 6,033,593 ns/iter (+/- 332,782) test size_21_2m ... bench: 12,403,937 ns/iter (+/- 626,731) test size_22_4m ... bench: 25,218,893 ns/iter (+/- 1,290,866) test size_23_8m ... bench: 45,120,983 ns/iter (+/- 1,714,226) test _invoke_chaos ... bench: 419,861,129 ns/iter (+/- 8,342,705) test size_00_1 ... bench: 1,172,231 ns/iter (+/- 6,850) test size_01_2 ... bench: 1,176,073 ns/iter (+/- 6,664) test size_02_4 ... bench: 1,179,213 ns/iter (+/- 10,001) test size_03_8 ... bench: 1,179,364 ns/iter (+/- 9,985) test size_04_16 ... bench: 1,182,618 ns/iter (+/- 9,154) test size_05_32 ... bench: 1,183,845 ns/iter (+/- 6,272) test size_06_64 ... bench: 1,192,917 ns/iter (+/- 6,473) test size_07_128 ... bench: 1,219,179 ns/iter (+/- 9,096) test size_08_256 ... bench: 1,266,088 ns/iter (+/- 14,919) test size_09_512 ... bench: 1,349,183 ns/iter (+/- 21,996) test size_10_1k ... bench: 1,548,835 ns/iter (+/- 12,133) test size_11_2k ... bench: 1,929,276 ns/iter (+/- 17,447) test size_12_4k ... bench: 2,649,545 ns/iter (+/- 52,515) test size_13_8k ... bench: 4,233,634 ns/iter (+/- 27,626) test size_14_16k ... bench: 7,410,534 ns/iter (+/- 16,211) test size_15_32k ... bench: 13,733,377 ns/iter (+/- 28,703) test size_16_64k ... bench: 26,113,539 ns/iter (+/- 73,558) test size_17_128k ... bench: 50,787,086 ns/iter (+/- 96,875) test size_18_256k ... bench: 93,566,074 ns/iter (+/- 3,746,932) test size_19_512k ... bench: 226,470,336 ns/iter (+/- 24,565,318) test size_20_1m ... bench: 519,162,370 ns/iter (+/- 7,462,605) test size_21_2m ... bench: 1,099,614,726 ns/iter (+/- 7,175,544) test size_22_4m ... bench: 2,108,188,068 ns/iter (+/- 39,768,687) test size_23_8m ... bench: 0 ns/iter (+/- 30,262,382) --- Cargo.toml | 3 + benches/bench.rs | 183 ++++++++++++++++++++++++++++++++++++++ platform/inprocess/mod.rs | 5 ++ platform/macos/mod.rs | 5 ++ 4 files changed, 196 insertions(+) create mode 100644 benches/bench.rs diff --git a/Cargo.toml b/Cargo.toml index cd278f4df..e87c178df 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -15,3 +15,6 @@ rand = "0.3" serde = ">=0.6, <0.8" serde_macros = ">=0.6, <0.8" uuid = { version = "0.2", features = ["v4"] } + +[dev-dependencies] +crossbeam = "0.2" diff --git a/benches/bench.rs b/benches/bench.rs new file mode 100644 index 000000000..4aaf5e704 --- /dev/null +++ b/benches/bench.rs @@ -0,0 +1,183 @@ +#![feature(test)] + +extern crate crossbeam; +extern crate ipc_channel; +extern crate test; + +use ipc_channel::platform; + +use std::sync::{mpsc, Mutex}; + +/// Allows doing multiple inner iterations per bench.iter() run. +/// +/// This is mostly to amortise the overhead of spawning a thread in the benchmark +/// when sending larger messages (that might be fragmented). +/// +/// Note that you need to compensate the displayed results +/// for the proportionally longer runs yourself, +/// as the benchmark framework doesn't know about the inner iterations... +const ITERATIONS: usize = 1; + +fn bench_size(b: &mut test::Bencher, size: usize) { + let data: Vec = (0..size).map(|i| (i % 251) as u8).collect(); + let (tx, rx) = platform::channel().unwrap(); + + let (wait_tx, wait_rx) = mpsc::channel(); + let wait_rx = Mutex::new(wait_rx); + + if size > tx.get_max_fragment_size().unwrap() { + b.iter(|| { + crossbeam::scope(|scope| { + scope.spawn(|| { + let wait_rx = wait_rx.lock().unwrap(); + for _ in 0..ITERATIONS { + tx.send(&data, vec![], vec![]).unwrap(); + if ITERATIONS > 1 { + // Prevent beginning of the next send + // from overlapping with receive of last fragment, + // as otherwise results of runs with a large tail fragment + // are significantly skewed. + wait_rx.recv().unwrap(); + } + } + }); + for _ in 0..ITERATIONS { + rx.recv().unwrap(); + if ITERATIONS > 1 { + wait_tx.send(()).unwrap(); + } + } + // For reasons mysterious to me, + // not returning a value *from every branch* + // adds some 100 ns or so of overhead to all results -- + // which is quite significant for very short tests... + 0 + }) + }); + } else { + b.iter(|| { + for _ in 0..ITERATIONS { + tx.send(&data, vec![], vec![]).unwrap(); + rx.recv().unwrap(); + } + 0 + }); + } +} + +// It turns out the results have some crazy jumps between sizes, +// probably related to some allocator alignment stuff. +// What's more, these fluctuate strongly in seemingly random ways +// in response to various changes to the test setup etc. +// +// Warming up the memory somewhat mitigates these issues. +// The specific size used here was determined empirically +// to produce comparatively sane results -- on my system at least. +// (Maybe because it doesn't align well with 2's complements... +// Or maybe it's just an entirely random effect.) +// +// There might be more elegant and/or more effective ways to do the warm-up, +// using random allocations or something along these lines... +// However, I don't think it's really *that* important -- +// and I already spent way too much time trying to figure this out :-( +#[bench] +fn _invoke_chaos(b: &mut test::Bencher) { + bench_size(b, 777_777); // 111-up the beast ;-) +} + +#[bench] +fn size_00_1(b: &mut test::Bencher) { + bench_size(b, 1); +} +#[bench] +fn size_01_2(b: &mut test::Bencher) { + bench_size(b, 2); +} +#[bench] +fn size_02_4(b: &mut test::Bencher) { + bench_size(b, 4); +} +#[bench] +fn size_03_8(b: &mut test::Bencher) { + bench_size(b, 8); +} +#[bench] +fn size_04_16(b: &mut test::Bencher) { + bench_size(b, 16); +} +#[bench] +fn size_05_32(b: &mut test::Bencher) { + bench_size(b, 32); +} +#[bench] +fn size_06_64(b: &mut test::Bencher) { + bench_size(b, 64); +} +#[bench] +fn size_07_128(b: &mut test::Bencher) { + bench_size(b, 128); +} +#[bench] +fn size_08_256(b: &mut test::Bencher) { + bench_size(b, 256); +} +#[bench] +fn size_09_512(b: &mut test::Bencher) { + bench_size(b, 512); +} +#[bench] +fn size_10_1k(b: &mut test::Bencher) { + bench_size(b, 1 * 1024); +} +#[bench] +fn size_11_2k(b: &mut test::Bencher) { + bench_size(b, 2 * 1024); +} +#[bench] +fn size_12_4k(b: &mut test::Bencher) { + bench_size(b, 4 * 1024); +} +#[bench] +fn size_13_8k(b: &mut test::Bencher) { + bench_size(b, 8 * 1024); +} +#[bench] +fn size_14_16k(b: &mut test::Bencher) { + bench_size(b, 16 * 1024); +} +#[bench] +fn size_15_32k(b: &mut test::Bencher) { + bench_size(b, 32 * 1024); +} +#[bench] +fn size_16_64k(b: &mut test::Bencher) { + bench_size(b, 64 * 1024); +} +#[bench] +fn size_17_128k(b: &mut test::Bencher) { + bench_size(b, 128 * 1024); +} +#[bench] +fn size_18_256k(b: &mut test::Bencher) { + bench_size(b, 256 * 1024); +} +#[bench] +fn size_19_512k(b: &mut test::Bencher) { + bench_size(b, 512 * 1024); +} +#[bench] +fn size_20_1m(b: &mut test::Bencher) { + bench_size(b, 1 * 1024 * 1024); +} +#[bench] +fn size_21_2m(b: &mut test::Bencher) { + bench_size(b, 2 * 1024 * 1024); +} +#[bench] +fn size_22_4m(b: &mut test::Bencher) { + bench_size(b, 4 * 1024 * 1024); +} +#[bench] +fn size_23_8m(b: &mut test::Bencher) { + bench_size(b, 8 * 1024 * 1024); +} diff --git a/platform/inprocess/mod.rs b/platform/inprocess/mod.rs index e2f426170..99e6af629 100644 --- a/platform/inprocess/mod.rs +++ b/platform/inprocess/mod.rs @@ -18,6 +18,7 @@ use std::fmt::{self, Debug, Formatter}; use std::cmp::{PartialEq}; use std::ops::Deref; use std::mem; +use std::usize; use uuid::Uuid; @@ -148,6 +149,10 @@ impl MpscSender { Ok(record.sender) } + pub fn get_max_fragment_size(&self) -> Result { + Ok(usize::MAX) + } + pub fn send(&self, data: &[u8], ports: Vec, diff --git a/platform/macos/mod.rs b/platform/macos/mod.rs index b8a822397..6d1ace156 100644 --- a/platform/macos/mod.rs +++ b/platform/macos/mod.rs @@ -23,6 +23,7 @@ use std::mem; use std::ops::Deref; use std::ptr; use std::slice; +use std::usize; mod mach_sys; @@ -365,6 +366,10 @@ impl MachSender { } } + pub fn get_max_fragment_size(&self) -> Result { + Ok(usize::MAX) + } + pub fn send(&self, data: &[u8], ports: Vec, From 1769906b64a60a3c59a2fd911c1a22ec8df80ede Mon Sep 17 00:00:00 2001 From: Olaf Buddenhagen Date: Sun, 28 Feb 2016 19:05:05 +0100 Subject: [PATCH 21/33] Linux: Drop fragment headers in favour of a total length header Now that we no longer have to deal with fragments of different messages interleaving, there is no need for the complicated fragment ID handling. The only information we strictly need is whether we have fragmentation at all. (So the receiver knows whether to retrieve and use the dedicated channel.) However, rather than only sending a boolean flag, we can just as well send an `usize` announcing the total size of payload data in this message. This way the receiver knows when fragmentation occurs -- and additionally knows exactly when all fragments have been received. This avoids the need to check in the receiver for the channel being closed by the sender; and knowing the total size in advance will also enable further optimisations/simplifications in the future. As a side effect, getting rid of the fragment headers removes the need in the sender to copy the data again when preparing send buffers for the followup fragments. (One copy operation is still needed for assembling the initial send buffer from the size header and main data.) This almost doubles performance (send + receive) of large transfers. Note though that this is a temporary effect: upcoming, more thorough optimisations will make this change mostly meaningless. (In terms of performance, that is -- the simplification is still worthwhile of course!) Regarding the results below, note that the single-iteration numbers for 256k and especially 512k experience a *huge* random fluctuation here (the latter ranging between 1.2 ms and 1.6 ms from one test run to another...) -- so they are really useful only as a very rough orientation, rather than for comparing against other results. The results for 100 iterations on the other hand are pretty stable, with fluctuations usually below 2% for all sizes. test _invoke_chaos ... bench: 2,779,348 ns/iter (+/- 353,869) test size_00_1 ... bench: 11,958 ns/iter (+/- 131) test size_01_2 ... bench: 11,895 ns/iter (+/- 89) test size_02_4 ... bench: 11,646 ns/iter (+/- 99) test size_03_8 ... bench: 11,648 ns/iter (+/- 57) test size_04_16 ... bench: 11,675 ns/iter (+/- 64) test size_05_32 ... bench: 11,715 ns/iter (+/- 47) test size_06_64 ... bench: 11,885 ns/iter (+/- 84) test size_07_128 ... bench: 12,159 ns/iter (+/- 103) test size_08_256 ... bench: 12,536 ns/iter (+/- 155) test size_09_512 ... bench: 13,544 ns/iter (+/- 156) test size_10_1k ... bench: 15,472 ns/iter (+/- 123) test size_11_2k ... bench: 19,217 ns/iter (+/- 128) test size_12_4k ... bench: 26,548 ns/iter (+/- 401) test size_13_8k ... bench: 42,123 ns/iter (+/- 436) test size_14_16k ... bench: 73,710 ns/iter (+/- 259) test size_15_32k ... bench: 139,332 ns/iter (+/- 1,106) test size_16_64k ... bench: 267,651 ns/iter (+/- 2,688) test size_17_128k ... bench: 517,987 ns/iter (+/- 7,714) test size_18_256k ... bench: 934,841 ns/iter (+/- 272,889) test size_19_512k ... bench: 1,327,214 ns/iter (+/- 417,956) test size_20_1m ... bench: 3,786,214 ns/iter (+/- 429,365) test size_21_2m ... bench: 7,559,035 ns/iter (+/- 738,997) test size_22_4m ... bench: 15,069,971 ns/iter (+/- 1,203,609) test size_23_8m ... bench: 24,633,162 ns/iter (+/- 1,969,078) test _invoke_chaos ... bench: 277,756,398 ns/iter (+/- 8,038,558) test size_00_1 ... bench: 1,187,442 ns/iter (+/- 5,224) test size_01_2 ... bench: 1,189,368 ns/iter (+/- 7,582) test size_02_4 ... bench: 1,168,775 ns/iter (+/- 9,093) test size_03_8 ... bench: 1,173,062 ns/iter (+/- 10,568) test size_04_16 ... bench: 1,171,706 ns/iter (+/- 8,885) test size_05_32 ... bench: 1,176,364 ns/iter (+/- 5,833) test size_06_64 ... bench: 1,190,941 ns/iter (+/- 7,975) test size_07_128 ... bench: 1,224,023 ns/iter (+/- 7,265) test size_08_256 ... bench: 1,267,395 ns/iter (+/- 7,898) test size_09_512 ... bench: 1,360,957 ns/iter (+/- 9,707) test size_10_1k ... bench: 1,538,531 ns/iter (+/- 7,787) test size_11_2k ... bench: 1,921,562 ns/iter (+/- 19,868) test size_12_4k ... bench: 2,655,408 ns/iter (+/- 42,143) test size_13_8k ... bench: 4,250,711 ns/iter (+/- 61,767) test size_14_16k ... bench: 7,446,110 ns/iter (+/- 246,966) test size_15_32k ... bench: 13,971,866 ns/iter (+/- 20,793) test size_16_64k ... bench: 26,721,142 ns/iter (+/- 49,591) test size_17_128k ... bench: 51,645,773 ns/iter (+/- 109,419) test size_18_256k ... bench: 67,393,172 ns/iter (+/- 2,902,754) test size_19_512k ... bench: 138,950,061 ns/iter (+/- 27,886,102) test size_20_1m ... bench: 344,968,619 ns/iter (+/- 6,519,044) test size_21_2m ... bench: 685,842,845 ns/iter (+/- 16,417,853) test size_22_4m ... bench: 1,154,950,689 ns/iter (+/- 14,661,441) test size_23_8m ... bench: 2,335,253,136 ns/iter (+/- 25,108,022) --- platform/linux/mod.rs | 77 +++++++++++++++++-------------------------- 1 file changed, 30 insertions(+), 47 deletions(-) diff --git a/platform/linux/mod.rs b/platform/linux/mod.rs index efc086cc9..f1db552c4 100644 --- a/platform/linux/mod.rs +++ b/platform/linux/mod.rs @@ -20,7 +20,6 @@ use std::mem; use std::ops::Deref; use std::ptr; use std::slice; -use std::sync::atomic::{ATOMIC_USIZE_INIT, AtomicUsize, Ordering}; use std::thread; const MAX_FDS_IN_CMSG: u32 = 64; @@ -33,8 +32,6 @@ const MAP_FAILED: *mut u8 = (!0usize) as *mut u8; // Empirically, we have to deduct 32 bytes from that. const RESERVED_SIZE: usize = 32; -static LAST_FRAGMENT_ID: AtomicUsize = ATOMIC_USIZE_INIT; - pub fn channel() -> Result<(UnixSender, UnixReceiver),UnixError> { let mut results = [0, 0]; unsafe { @@ -150,7 +147,7 @@ impl UnixSender { /// as obtained with `get_system_sendbuf_size()` -- /// except after getting ENOBUFS, in which case it needs to be reduced. fn fragment_size(sendbuf_size: usize) -> usize { - sendbuf_size - RESERVED_SIZE - mem::size_of::() * 2 + sendbuf_size - RESERVED_SIZE - mem::size_of::() } /// Maximum data size that can be transferred over this channel in a single packet. @@ -180,11 +177,15 @@ impl UnixSender { fds.push(shared_memory_region.fd); } - let mut data_buffer = vec![0; data.len() + mem::size_of::() * 2]; + let mut data_buffer = vec![0; data.len() + mem::size_of::()]; { let mut data_buffer = &mut data_buffer[..]; - data_buffer.write_u32::(0u32).unwrap(); - data_buffer.write_u32::(0u32).unwrap(); + // Message begins with a header recording the total data length. + // + // The receiver uses this to determine whether it already got the entire message, + // or needs to receive additional fragments -- and if so, how much. + data_buffer.write_uint::(data.len() as u64, mem::size_of::()) + .unwrap(); data_buffer.write(data).unwrap(); } @@ -290,30 +291,24 @@ impl UnixSender { // Split up the packet into fragments. let mut byte_position = 0; - let mut this_fragment_id = 0; while byte_position < data.len() { let bytes_per_fragment = Self::fragment_size(sendbuf_size); let end_byte_position = cmp::min(data.len(), byte_position + bytes_per_fragment); - let next_fragment_id = if end_byte_position == data.len() { - 0 - } else { - (LAST_FRAGMENT_ID.fetch_add(1, Ordering::SeqCst) + 1) as u32 - }; - - { - let mut data_buffer = &mut data_buffer[..]; - data_buffer.write_u32::(this_fragment_id).unwrap(); - data_buffer.write_u32::(next_fragment_id).unwrap(); - data_buffer.write(&data[byte_position..end_byte_position]).unwrap(); - } - - let bytes_to_send = end_byte_position - byte_position + mem::size_of::() * 2; + let bytes_to_send; let result = if byte_position == 0 { + // First fragment. No offset; but contains message header (total size). + // The auxiliary data (FDs) is also sent along with this one. + + bytes_to_send = end_byte_position + mem::size_of::(); send_first_fragment(self.fd, &fds[..], &data_buffer[..bytes_to_send]) } else { - send_followup_fragment(dedicated_tx.fd, &data_buffer[..bytes_to_send]) + // Followup fragment. No header; but offset by amount of data already sent. + + bytes_to_send = end_byte_position - byte_position; + let remainder = &data_buffer[byte_position + mem::size_of::() ..]; + send_followup_fragment(dedicated_tx.fd, &remainder[..bytes_to_send]) }; if let Err(error) = result { @@ -328,7 +323,6 @@ impl UnixSender { } byte_position += bytes_per_fragment; - this_fragment_id = next_fragment_id; } Ok(()) @@ -748,18 +742,15 @@ fn recv(fd: c_int, blocking_mode: BlockingMode) shared_memory_regions.push(UnixSharedMemory::from_fd(fd)); } - // Separate out the fragmentation frame. - let (fragment_info_buffer, main_data_buffer) = cmsg.data_buffer - .split_at(mem::size_of::() * 2); + // Separate out the header. + let (header, main_data_buffer) = cmsg.data_buffer.split_at(mem::size_of::()); let mut main_data_buffer: Vec = - main_data_buffer[0..(bytes_read - mem::size_of::() * 2)].iter() - .cloned() - .collect(); - let mut next_fragment_id = - (&fragment_info_buffer[mem::size_of::().. - (mem::size_of::() * 2)]).read_u32::() - .unwrap(); - if next_fragment_id == 0 { + main_data_buffer[0..(bytes_read - mem::size_of::())].iter() + .cloned() + .collect(); + let total_size = (&header[..]).read_uint::(mem::size_of::()) + .unwrap() as usize; + if total_size == main_data_buffer.len() { // Fast path: no fragments. return Ok((main_data_buffer, channels, shared_memory_regions)) } @@ -769,23 +760,15 @@ fn recv(fd: c_int, blocking_mode: BlockingMode) // The initial fragment carries the receive end of a dedicated channel // through which all the remaining fragments will be coming in. let dedicated_rx = channels.pop().unwrap().to_receiver(); - while next_fragment_id != 0 { - let mut cmsg = UnixCmsg::new(maximum_recv_size - RESERVED_SIZE); + while main_data_buffer.len() < total_size { + let mut cmsg = UnixCmsg::new(UnixSender::fragment_size(maximum_recv_size)); // Always use blocking mode for followup fragments, // to make sure that once we start receiving a multi-fragment message, // we don't abort in the middle of it... let bytes_read = try!(cmsg.recv(dedicated_rx.fd, BlockingMode::Blocking)); - let this_fragment_id = - (&cmsg.data_buffer[0..mem::size_of::()]).read_u32::().unwrap(); - assert!(this_fragment_id == next_fragment_id); - - next_fragment_id = - (&cmsg.data_buffer[mem::size_of::().. - (mem::size_of::() * 2)]).read_u32::() - .unwrap(); - main_data_buffer.extend( - cmsg.data_buffer[(mem::size_of::() * 2)..bytes_read].iter().cloned()) + // Followup fragemnts do not have a header -- the entire buffer is payload. + main_data_buffer.extend_from_slice(&cmsg.data_buffer[0..bytes_read]); } Ok((main_data_buffer, channels, shared_memory_regions)) From 31235f60c48b9b54c0b7901fb4d0f624aab25fab Mon Sep 17 00:00:00 2001 From: Olaf Buddenhagen Date: Sun, 28 Feb 2016 19:34:20 +0100 Subject: [PATCH 22/33] Linux: Use separate send buffers for header and main data Using the scatter-gather functionality of the sendmsg() system call, we can avoid the need for copying data into a dedicated buffer altogether. This gets another sizeable performance increase (on top of the fragment header change) for large transfers, resulting in a total speedup of about 2x - 3x (depending on size) compared to the original version. Medium-sized (non-fragemented) transfers also gain a few per cent. (The gains on the sender side are actually even bigger: we still haven't optimised the receiver side at all -- so that is now the main bottleneck...) Note that comparing against the original variant indeed makes more sense here than looking at the most recent results, because this new approach effectively obsoletes the performance gains of the previous change: using scatter-gather, we could have achieved the same zero-copy effect even with the old fragment header approach, with only some small overhead for handling the actual headers. (The code is considerably simpler without the fragment headers, though.) test _invoke_chaos ... bench: 2,681,822 ns/iter (+/- 219,773) test size_00_1 ... bench: 11,815 ns/iter (+/- 121) test size_01_2 ... bench: 11,798 ns/iter (+/- 57) test size_02_4 ... bench: 11,727 ns/iter (+/- 72) test size_03_8 ... bench: 11,717 ns/iter (+/- 53) test size_04_16 ... bench: 11,783 ns/iter (+/- 128) test size_05_32 ... bench: 11,798 ns/iter (+/- 59) test size_06_64 ... bench: 11,947 ns/iter (+/- 103) test size_07_128 ... bench: 12,203 ns/iter (+/- 89) test size_08_256 ... bench: 12,779 ns/iter (+/- 94) test size_09_512 ... bench: 13,866 ns/iter (+/- 122) test size_10_1k ... bench: 15,288 ns/iter (+/- 231) test size_11_2k ... bench: 19,603 ns/iter (+/- 256) test size_12_4k ... bench: 28,476 ns/iter (+/- 232) test size_13_8k ... bench: 46,229 ns/iter (+/- 200) test size_14_16k ... bench: 73,107 ns/iter (+/- 314) test size_15_32k ... bench: 133,618 ns/iter (+/- 1,121) test size_16_64k ... bench: 254,531 ns/iter (+/- 4,815) test size_17_128k ... bench: 495,497 ns/iter (+/- 10,201) test size_18_256k ... bench: 886,351 ns/iter (+/- 199,942) test size_19_512k ... bench: 1,123,621 ns/iter (+/- 305,389) test size_20_1m ... bench: 3,036,919 ns/iter (+/- 285,712) test size_21_2m ... bench: 5,222,443 ns/iter (+/- 347,878) test size_22_4m ... bench: 7,551,218 ns/iter (+/- 914,100) test size_23_8m ... bench: 14,977,983 ns/iter (+/- 1,302,684) test _invoke_chaos ... bench: 244,491,008 ns/iter (+/- 7,239,693) test size_00_1 ... bench: 1,183,187 ns/iter (+/- 5,943) test size_01_2 ... bench: 1,183,225 ns/iter (+/- 9,967) test size_02_4 ... bench: 1,180,223 ns/iter (+/- 6,720) test size_03_8 ... bench: 1,181,466 ns/iter (+/- 6,136) test size_04_16 ... bench: 1,183,006 ns/iter (+/- 7,598) test size_05_32 ... bench: 1,191,722 ns/iter (+/- 9,888) test size_06_64 ... bench: 1,198,561 ns/iter (+/- 8,227) test size_07_128 ... bench: 1,217,393 ns/iter (+/- 5,343) test size_08_256 ... bench: 1,269,855 ns/iter (+/- 7,813) test size_09_512 ... bench: 1,393,586 ns/iter (+/- 7,397) test size_10_1k ... bench: 1,524,854 ns/iter (+/- 15,853) test size_11_2k ... bench: 1,959,964 ns/iter (+/- 24,707) test size_12_4k ... bench: 2,858,032 ns/iter (+/- 11,557) test size_13_8k ... bench: 4,629,783 ns/iter (+/- 11,677) test size_14_16k ... bench: 7,321,471 ns/iter (+/- 13,709) test size_15_32k ... bench: 13,397,902 ns/iter (+/- 16,635) test size_16_64k ... bench: 25,558,619 ns/iter (+/- 112,384) test size_17_128k ... bench: 49,717,629 ns/iter (+/- 1,668,997) test size_18_256k ... bench: 67,053,276 ns/iter (+/- 2,203,769) test size_19_512k ... bench: 125,389,098 ns/iter (+/- 20,284,576) test size_20_1m ... bench: 276,946,251 ns/iter (+/- 8,119,682) test size_21_2m ... bench: 487,328,628 ns/iter (+/- 12,586,919) test size_22_4m ... bench: 715,961,795 ns/iter (+/- 14,266,485) test size_23_8m ... bench: 1,156,209,396 ns/iter (+/- 20,298,143) --- platform/linux/mod.rs | 71 +++++++++++++++++++++++-------------------- 1 file changed, 38 insertions(+), 33 deletions(-) diff --git a/platform/linux/mod.rs b/platform/linux/mod.rs index f1db552c4..d7874c277 100644 --- a/platform/linux/mod.rs +++ b/platform/linux/mod.rs @@ -15,7 +15,7 @@ use std::cmp; use std::collections::HashSet; use std::ffi::{CStr, CString}; use std::fmt::{self, Debug, Formatter}; -use std::io::{Error, Write}; +use std::io::Error; use std::mem; use std::ops::Deref; use std::ptr; @@ -177,19 +177,13 @@ impl UnixSender { fds.push(shared_memory_region.fd); } - let mut data_buffer = vec![0; data.len() + mem::size_of::()]; - { - let mut data_buffer = &mut data_buffer[..]; - // Message begins with a header recording the total data length. - // - // The receiver uses this to determine whether it already got the entire message, - // or needs to receive additional fragments -- and if so, how much. - data_buffer.write_uint::(data.len() as u64, mem::size_of::()) - .unwrap(); - data_buffer.write(data).unwrap(); - } - - fn send_first_fragment(sender_fd: c_int, fds: &[c_int], data_buffer: &[u8]) + // `len` is the total length of the message. + // Its value will be sent as a message header before the payload data. + // + // Not to be confused with the length of the data to send in this packet + // (i.e. the length of the data buffer passed in), + // which in a fragmented send will be smaller than the total message length. + fn send_first_fragment(sender_fd: c_int, fds: &[c_int], data_buffer: &[u8], len: usize) -> Result<(),UnixError> { let result = unsafe { let cmsg_length = mem::size_of_val(fds); @@ -202,15 +196,33 @@ impl UnixSender { cmsg_buffer.offset(1) as *mut _ as *mut c_int, fds.len()); - let iovec = iovec { - iov_base: data_buffer.as_ptr() as *const c_char as *mut c_char, - iov_len: data_buffer.len(), - }; + // First fragment begins with a header recording the total data length. + // + // The receiver uses this to determine whether it already got the entire message, + // or needs to receive additional fragments -- and if so, how much. + let mut len_buffer = vec![0; mem::size_of_val(&len)]; + { + let mut len_buffer = &mut len_buffer[..]; + len_buffer.write_uint::(len as u64, mem::size_of_val(&len)) + .unwrap(); + } + + let iovec = [ + iovec { + iov_base: len_buffer.as_ptr() as *const c_char as *mut c_char, + iov_len: len_buffer.len(), + }, + iovec { + iov_base: data_buffer.as_ptr() as *const c_char as *mut c_char, + iov_len: data_buffer.len(), + }, + ]; + let msghdr = msghdr { msg_name: ptr::null_mut(), msg_namelen: 0, - msg_iov: &iovec, - msg_iovlen: 1, + msg_iov: iovec.as_ptr(), + msg_iovlen: iovec.len(), msg_control: cmsg_buffer as *mut c_void, msg_controllen: CMSG_SPACE(cmsg_length), msg_flags: 0, @@ -262,11 +274,10 @@ impl UnixSender { } } - match send_first_fragment(self.fd, &fds[..], &data_buffer[..]) { + match send_first_fragment(self.fd, &fds[..], data, data.len()) { Ok(_) => return Ok(()), Err(error) => { - if error.0 == libc::ENOBUFS - && downsize(&mut sendbuf_size, data_buffer.len()).is_ok() { + if error.0 == libc::ENOBUFS && downsize(&mut sendbuf_size, data.len()).is_ok() { // If we get this error, // it means the message was small enough to fit the maximum send size, // but the kernel failed to allocate a buffer large enough @@ -296,24 +307,18 @@ impl UnixSender { let end_byte_position = cmp::min(data.len(), byte_position + bytes_per_fragment); - let bytes_to_send; let result = if byte_position == 0 { // First fragment. No offset; but contains message header (total size). // The auxiliary data (FDs) is also sent along with this one. - - bytes_to_send = end_byte_position + mem::size_of::(); - send_first_fragment(self.fd, &fds[..], &data_buffer[..bytes_to_send]) + send_first_fragment(self.fd, &fds[..], &data[..end_byte_position], data.len()) } else { // Followup fragment. No header; but offset by amount of data already sent. - - bytes_to_send = end_byte_position - byte_position; - let remainder = &data_buffer[byte_position + mem::size_of::() ..]; - send_followup_fragment(dedicated_tx.fd, &remainder[..bytes_to_send]) + send_followup_fragment(dedicated_tx.fd, &data[byte_position..end_byte_position]) }; if let Err(error) = result { if error.0 == libc::ENOBUFS - && downsize(&mut sendbuf_size, bytes_to_send).is_ok() { + && downsize(&mut sendbuf_size, end_byte_position - byte_position).is_ok() { // If the kernel failed to allocate a buffer large enough for the packet, // retry with a smaller size (if possible). continue @@ -322,7 +327,7 @@ impl UnixSender { } } - byte_position += bytes_per_fragment; + byte_position = end_byte_position; } Ok(()) From 4050b26fba6972bb8a57fced92588f67bf4c30d5 Mon Sep 17 00:00:00 2001 From: Olaf Buddenhagen Date: Thu, 7 Apr 2016 03:02:30 +0200 Subject: [PATCH 23/33] Linux: Don't initialise memory of receive buffer The receiver always has to allocate a buffer large enough to fit the maximal packet size, as it doesn't know how large the next message will be. Up till now, the entire buffer was being 0-filled on allocation -- which was inflicting considerable overhead for small messages: skipping the initialisation almost quadruples(!) the performance of small transfers on my system; and has a noticable effect on larger transfers too, if the last fragment is relatively small. (About 10% for 512KiB transfers for example, where the last fragment is only about 32 KiB.) We truncate the length of the receive buffer (vector) to the actual size of the data received, right after the receive call -- so given that we don't do anything else between allocating the buffer and receiving, having it temporarily uninitialised shouldn't be terribly unsafe. test _invoke_chaos ... bench: 2,646,830 ns/iter (+/- 221,495) test size_00_1 ... bench: 3,070 ns/iter (+/- 48) test size_01_2 ... bench: 3,076 ns/iter (+/- 63) test size_02_4 ... bench: 3,040 ns/iter (+/- 64) test size_03_8 ... bench: 3,020 ns/iter (+/- 55) test size_04_16 ... bench: 3,072 ns/iter (+/- 65) test size_05_32 ... bench: 3,162 ns/iter (+/- 56) test size_06_64 ... bench: 3,249 ns/iter (+/- 63) test size_07_128 ... bench: 3,447 ns/iter (+/- 74) test size_08_256 ... bench: 3,959 ns/iter (+/- 101) test size_09_512 ... bench: 5,199 ns/iter (+/- 77) test size_10_1k ... bench: 6,577 ns/iter (+/- 124) test size_11_2k ... bench: 11,090 ns/iter (+/- 157) test size_12_4k ... bench: 19,702 ns/iter (+/- 80) test size_13_8k ... bench: 37,433 ns/iter (+/- 234) test size_14_16k ... bench: 64,489 ns/iter (+/- 461) test size_15_32k ... bench: 125,141 ns/iter (+/- 1,305) test size_16_64k ... bench: 247,282 ns/iter (+/- 2,844) test size_17_128k ... bench: 489,099 ns/iter (+/- 2,951) test size_18_256k ... bench: 838,078 ns/iter (+/- 130,191) test size_19_512k ... bench: 1,071,451 ns/iter (+/- 193,737) test size_20_1m ... bench: 2,935,692 ns/iter (+/- 255,588) test size_21_2m ... bench: 5,183,219 ns/iter (+/- 295,405) test size_22_4m ... bench: 7,396,433 ns/iter (+/- 808,390) test size_23_8m ... bench: 14,341,520 ns/iter (+/- 1,275,331) test _invoke_chaos ... bench: 238,710,985 ns/iter (+/- 16,477,582) test size_00_1 ... bench: 312,659 ns/iter (+/- 8,490) test size_01_2 ... bench: 311,919 ns/iter (+/- 5,871) test size_02_4 ... bench: 305,455 ns/iter (+/- 3,955) test size_03_8 ... bench: 310,709 ns/iter (+/- 6,244) test size_04_16 ... bench: 309,599 ns/iter (+/- 5,053) test size_05_32 ... bench: 313,935 ns/iter (+/- 5,827) test size_06_64 ... bench: 325,553 ns/iter (+/- 4,227) test size_07_128 ... bench: 352,797 ns/iter (+/- 7,614) test size_08_256 ... bench: 396,614 ns/iter (+/- 11,826) test size_09_512 ... bench: 482,741 ns/iter (+/- 8,917) test size_10_1k ... bench: 664,678 ns/iter (+/- 9,785) test size_11_2k ... bench: 1,132,642 ns/iter (+/- 14,503) test size_12_4k ... bench: 1,942,667 ns/iter (+/- 22,892) test size_13_8k ... bench: 3,706,023 ns/iter (+/- 16,474) test size_14_16k ... bench: 6,410,924 ns/iter (+/- 12,465) test size_15_32k ... bench: 12,494,806 ns/iter (+/- 23,242) test size_16_64k ... bench: 24,591,965 ns/iter (+/- 81,764) test size_17_128k ... bench: 48,608,297 ns/iter (+/- 106,295) test size_18_256k ... bench: 65,057,222 ns/iter (+/- 1,918,700) test size_19_512k ... bench: 114,772,423 ns/iter (+/- 18,717,560) test size_20_1m ... bench: 261,632,538 ns/iter (+/- 6,904,648) test size_21_2m ... bench: 483,410,889 ns/iter (+/- 10,294,448) test size_22_4m ... bench: 708,794,834 ns/iter (+/- 12,841,685) test size_23_8m ... bench: 1,139,315,776 ns/iter (+/- 25,330,167) --- platform/linux/mod.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/platform/linux/mod.rs b/platform/linux/mod.rs index d7874c277..3489e0b49 100644 --- a/platform/linux/mod.rs +++ b/platform/linux/mod.rs @@ -855,7 +855,11 @@ impl UnixCmsg { let cmsg_length = mem::size_of::() + (MAX_FDS_IN_CMSG as usize) * mem::size_of::(); assert!(maximum_recv_size > cmsg_length); - let mut data_buffer: Vec = vec![0; maximum_recv_size]; + + // Allocate a buffer without initialising the memory. + let mut data_buffer = Vec::with_capacity(maximum_recv_size); + data_buffer.set_len(maximum_recv_size); + let cmsg_buffer = libc::malloc(cmsg_length) as *mut cmsghdr; let iovec = Box::new(iovec { iov_base: &mut data_buffer[0] as *mut _ as *mut c_char, @@ -887,6 +891,8 @@ impl UnixCmsg { } let result = recvmsg(fd, &mut self.msghdr, 0); + self.data_buffer.set_len(cmp::max(result, 0) as usize); + let result = if result > 0 { Ok(result as usize) } else if result == 0 { From 4ef8256fa81f381cea5b0d63b9eede37c4341afe Mon Sep 17 00:00:00 2001 From: Olaf Buddenhagen Date: Sun, 20 Mar 2016 23:52:38 +0100 Subject: [PATCH 24/33] Linux: Avoid copying data when receiving followup fragments Rather then receiving each fragment into an individual buffer first, and concatenating it to the main buffer afterwards, preallocate space in the main buffer, and receive the followup fragements directly into it. (Only the initial fragment is still being copied, while separating out the message size header.) This results in another huge performance boost for large transfers, showing improvements of 50% and more at most sizes. (It's hard to assess an overall number, because of very strong variance between the individual sizes...) The biggest gain is at 2 MiB, with performance improving by about 2.5x (moving the drop-off in performance to the 4 MiB data point) -- probably because getting rid of the surplus data copies allows everything to fit in the last-level cache now at this size; only needing to go to slower main memory for even larger sizes. 512 KiB also gains about 2x, probably for similar reasons. The total speedup compared to the original version for transfers of 512 KiB and more now amounts to about 3x, and some 4.5x on average for even larger ones. As an interesting side effect, the benchmark results get much more consistent with this change, avoiding the need for a warmup. There are still some weird jumps at certain sizes; but these are less severe now over all -- and no longer affected by random other factors... test size_00_1 ... bench: 3,066 ns/iter (+/- 59) test size_01_2 ... bench: 3,116 ns/iter (+/- 46) test size_02_4 ... bench: 3,003 ns/iter (+/- 40) test size_03_8 ... bench: 3,071 ns/iter (+/- 44) test size_04_16 ... bench: 3,088 ns/iter (+/- 52) test size_05_32 ... bench: 3,143 ns/iter (+/- 42) test size_06_64 ... bench: 3,219 ns/iter (+/- 73) test size_07_128 ... bench: 3,499 ns/iter (+/- 92) test size_08_256 ... bench: 3,992 ns/iter (+/- 78) test size_09_512 ... bench: 5,002 ns/iter (+/- 84) test size_10_1k ... bench: 6,691 ns/iter (+/- 102) test size_11_2k ... bench: 11,398 ns/iter (+/- 175) test size_12_4k ... bench: 19,907 ns/iter (+/- 116) test size_13_8k ... bench: 37,206 ns/iter (+/- 158) test size_14_16k ... bench: 63,840 ns/iter (+/- 369) test size_15_32k ... bench: 124,472 ns/iter (+/- 1,409) test size_16_64k ... bench: 246,473 ns/iter (+/- 2,767) test size_17_128k ... bench: 487,915 ns/iter (+/- 11,440) test size_18_256k ... bench: 781,964 ns/iter (+/- 59,461) test size_19_512k ... bench: 984,189 ns/iter (+/- 86,029) test size_20_1m ... bench: 2,037,886 ns/iter (+/- 214,774) test size_21_2m ... bench: 2,374,924 ns/iter (+/- 596,728) test size_22_4m ... bench: 5,573,282 ns/iter (+/- 756,270) test size_23_8m ... bench: 10,058,920 ns/iter (+/- 1,767,761) test size_00_1 ... bench: 307,074 ns/iter (+/- 3,333) test size_01_2 ... bench: 306,568 ns/iter (+/- 3,736) test size_02_4 ... bench: 299,714 ns/iter (+/- 4,773) test size_03_8 ... bench: 310,657 ns/iter (+/- 5,220) test size_04_16 ... bench: 306,247 ns/iter (+/- 4,279) test size_05_32 ... bench: 311,436 ns/iter (+/- 4,693) test size_06_64 ... bench: 321,380 ns/iter (+/- 5,674) test size_07_128 ... bench: 347,893 ns/iter (+/- 3,311) test size_08_256 ... bench: 398,704 ns/iter (+/- 4,745) test size_09_512 ... bench: 483,585 ns/iter (+/- 6,268) test size_10_1k ... bench: 664,508 ns/iter (+/- 12,941) test size_11_2k ... bench: 1,126,760 ns/iter (+/- 26,853) test size_12_4k ... bench: 1,946,401 ns/iter (+/- 34,891) test size_13_8k ... bench: 3,655,198 ns/iter (+/- 32,872) test size_14_16k ... bench: 6,374,230 ns/iter (+/- 12,787) test size_15_32k ... bench: 12,480,807 ns/iter (+/- 44,458) test size_16_64k ... bench: 24,633,454 ns/iter (+/- 94,243) test size_17_128k ... bench: 48,691,887 ns/iter (+/- 228,529) test size_18_256k ... bench: 60,997,701 ns/iter (+/- 1,925,834) test size_19_512k ... bench: 74,230,547 ns/iter (+/- 4,206,076) test size_20_1m ... bench: 163,075,989 ns/iter (+/- 6,646,674) test size_21_2m ... bench: 193,855,222 ns/iter (+/- 9,828,518) test size_22_4m ... bench: 511,140,039 ns/iter (+/- 10,467,888) test size_23_8m ... bench: 942,365,953 ns/iter (+/- 19,146,340) --- benches/bench.rs | 20 -------------------- platform/linux/mod.rs | 30 ++++++++++++++++++++++++------ 2 files changed, 24 insertions(+), 26 deletions(-) diff --git a/benches/bench.rs b/benches/bench.rs index 4aaf5e704..7033aca7d 100644 --- a/benches/bench.rs +++ b/benches/bench.rs @@ -65,26 +65,6 @@ fn bench_size(b: &mut test::Bencher, size: usize) { } } -// It turns out the results have some crazy jumps between sizes, -// probably related to some allocator alignment stuff. -// What's more, these fluctuate strongly in seemingly random ways -// in response to various changes to the test setup etc. -// -// Warming up the memory somewhat mitigates these issues. -// The specific size used here was determined empirically -// to produce comparatively sane results -- on my system at least. -// (Maybe because it doesn't align well with 2's complements... -// Or maybe it's just an entirely random effect.) -// -// There might be more elegant and/or more effective ways to do the warm-up, -// using random allocations or something along these lines... -// However, I don't think it's really *that* important -- -// and I already spent way too much time trying to figure this out :-( -#[bench] -fn _invoke_chaos(b: &mut test::Bencher) { - bench_size(b, 777_777); // 111-up the beast ;-) -} - #[bench] fn size_00_1(b: &mut test::Bencher) { bench_size(b, 1); diff --git a/platform/linux/mod.rs b/platform/linux/mod.rs index 3489e0b49..fc222ce72 100644 --- a/platform/linux/mod.rs +++ b/platform/linux/mod.rs @@ -765,15 +765,33 @@ fn recv(fd: c_int, blocking_mode: BlockingMode) // The initial fragment carries the receive end of a dedicated channel // through which all the remaining fragments will be coming in. let dedicated_rx = channels.pop().unwrap().to_receiver(); + + // Extend the buffer to hold the entire message, without initialising the memory. + let len = main_data_buffer.len(); + main_data_buffer.reserve(total_size - len); + + // Receive followup fragments directly into the main buffer. while main_data_buffer.len() < total_size { - let mut cmsg = UnixCmsg::new(UnixSender::fragment_size(maximum_recv_size)); - // Always use blocking mode for followup fragments, + let write_pos = main_data_buffer.len(); + let end_pos = cmp::min(write_pos + UnixSender::fragment_size(maximum_recv_size), + total_size); + assert!(end_pos <= main_data_buffer.capacity()); + main_data_buffer.set_len(end_pos); + + // Note: we always use blocking mode for followup fragments, // to make sure that once we start receiving a multi-fragment message, // we don't abort in the middle of it... - let bytes_read = try!(cmsg.recv(dedicated_rx.fd, BlockingMode::Blocking)); - - // Followup fragemnts do not have a header -- the entire buffer is payload. - main_data_buffer.extend_from_slice(&cmsg.data_buffer[0..bytes_read]); + let result = libc::recv(dedicated_rx.fd, + main_data_buffer[write_pos..].as_mut_ptr() as *mut c_void, + end_pos - write_pos, + 0); + main_data_buffer.set_len(write_pos + cmp::max(result, 0) as usize); + + if result == 0 { + return Err(UnixError(libc::ECONNRESET)) + } else if result < 0 { + return Err(UnixError::last()) + }; } Ok((main_data_buffer, channels, shared_memory_regions)) From f2536dfaa2e5f44e7b0eb3c772521e845113a453 Mon Sep 17 00:00:00 2001 From: Olaf Buddenhagen Date: Thu, 7 Apr 2016 19:13:55 +0200 Subject: [PATCH 25/33] Linux: Use separate receive buffers for header and main message Just like on the sender side, we can use the scatter-gather functionality of recvmsg(), to put the data directly into the final place -- rather than having to copy it around -- even for the initial fragment. This removes the last major piece of unnecessary overhead; and consequently results in very large gains mostly for medium-sized transfers, where the speedup exceeds 10x on my system. Larger transfers up to a few MiB are still affected quite significantly, improving by about 45% at 2 MiB and some 13% at 4 MiB. All in all, transfers of all sizes are now several times faster than on the first measured version, before applying optimisations -- with the lowest speedup on my system at about 4x for small transfers; about 4.5x for very large ones; and the largest boost of >11x for medium-sized ones. A quick check on a more modern system (64 bit; fairly recent Intel CPU) showed even larger gains: while very big transfers were similar (about 5x speedup), small ones gained >9x, and medium-sized ones >20x. One interesting observation is that on the modern system, the optimised version shows even more strongly pronounced jumps at specific sizes. (Especially 256 KiB and at 1 MiB.) While I haven't verified whether these are more like spikes or more like steps, I suspect it's the latter: with more streamlined memory access patterns in the optimised version, it becomes pretty obvious that these jumps are simply successive levels of the cache hierarchy being exhausted... Below are the numbers of a typical run on my old system, as usual. (Note that the numbers shown for medium-large transfers of 256 KiB and above are now pretty much entirely useless, as on this system the thread launching overhead is larger than the actual benchmark time... On the newer system on the other hand launching the extra thread doesn't seem to have a strongly pronounced effect: the 256 KiB data point shows an equally strong slowdown with one iteration as with 100...) test size_00_1 ... bench: 3,050 ns/iter (+/- 50) test size_01_2 ... bench: 3,018 ns/iter (+/- 59) test size_02_4 ... bench: 3,098 ns/iter (+/- 55) test size_03_8 ... bench: 3,112 ns/iter (+/- 35) test size_04_16 ... bench: 3,104 ns/iter (+/- 54) test size_05_32 ... bench: 3,094 ns/iter (+/- 80) test size_06_64 ... bench: 3,060 ns/iter (+/- 49) test size_07_128 ... bench: 3,101 ns/iter (+/- 30) test size_08_256 ... bench: 3,142 ns/iter (+/- 50) test size_09_512 ... bench: 3,171 ns/iter (+/- 49) test size_10_1k ... bench: 3,322 ns/iter (+/- 58) test size_11_2k ... bench: 4,755 ns/iter (+/- 69) test size_12_4k ... bench: 6,277 ns/iter (+/- 77) test size_13_8k ... bench: 10,070 ns/iter (+/- 96) test size_14_16k ... bench: 9,735 ns/iter (+/- 99) test size_15_32k ... bench: 14,330 ns/iter (+/- 114) test size_16_64k ... bench: 22,990 ns/iter (+/- 857) test size_17_128k ... bench: 44,556 ns/iter (+/- 1,693) test size_18_256k ... bench: 222,732 ns/iter (+/- 47,581) test size_19_512k ... bench: 447,259 ns/iter (+/- 178,507) test size_20_1m ... bench: 1,369,545 ns/iter (+/- 239,571) test size_21_2m ... bench: 1,737,641 ns/iter (+/- 515,468) test size_22_4m ... bench: 4,923,732 ns/iter (+/- 1,204,576) test size_23_8m ... bench: 9,373,281 ns/iter (+/- 1,282,587) test size_00_1 ... bench: 284,262 ns/iter (+/- 5,333) test size_01_2 ... bench: 287,241 ns/iter (+/- 5,105) test size_02_4 ... bench: 291,753 ns/iter (+/- 3,840) test size_03_8 ... bench: 294,802 ns/iter (+/- 8,149) test size_04_16 ... bench: 291,475 ns/iter (+/- 4,166) test size_05_32 ... bench: 292,328 ns/iter (+/- 4,728) test size_06_64 ... bench: 289,378 ns/iter (+/- 5,618) test size_07_128 ... bench: 293,067 ns/iter (+/- 5,312) test size_08_256 ... bench: 308,579 ns/iter (+/- 4,699) test size_09_512 ... bench: 301,040 ns/iter (+/- 5,636) test size_10_1k ... bench: 312,662 ns/iter (+/- 10,609) test size_11_2k ... bench: 446,448 ns/iter (+/- 5,646) test size_12_4k ... bench: 607,197 ns/iter (+/- 7,551) test size_13_8k ... bench: 997,677 ns/iter (+/- 10,513) test size_14_16k ... bench: 956,131 ns/iter (+/- 7,722) test size_15_32k ... bench: 1,437,060 ns/iter (+/- 6,730) test size_16_64k ... bench: 2,269,055 ns/iter (+/- 23,130) test size_17_128k ... bench: 4,413,551 ns/iter (+/- 19,655) test size_18_256k ... bench: 8,628,218 ns/iter (+/- 2,455,944) test size_19_512k ... bench: 19,452,160 ns/iter (+/- 3,474,999) test size_20_1m ... bench: 103,957,847 ns/iter (+/- 8,015,338) test size_21_2m ... bench: 134,416,502 ns/iter (+/- 15,466,457) test size_22_4m ... bench: 446,838,335 ns/iter (+/- 31,417,044) test size_23_8m ... bench: 895,111,420 ns/iter (+/- 16,740,773) --- platform/linux/mod.rs | 57 ++++++++++++++++++++----------------------- 1 file changed, 27 insertions(+), 30 deletions(-) diff --git a/platform/linux/mod.rs b/platform/linux/mod.rs index fc222ce72..c95998161 100644 --- a/platform/linux/mod.rs +++ b/platform/linux/mod.rs @@ -727,8 +727,29 @@ fn recv(fd: c_int, blocking_mode: BlockingMode) return Err(UnixError::last()) } - let mut cmsg = UnixCmsg::new(maximum_recv_size); + // First fragment begins with a header recording the total data length. + // + // We use this to determine whether we already got the entire message, + // or need to receive additional fragments -- and if so, how much. + let mut len_buffer = vec![0; mem::size_of::()]; + // Allocate a buffer without initialising the memory. + let mut main_data_buffer = Vec::with_capacity(maximum_recv_size - len_buffer.len()); + main_data_buffer.set_len(maximum_recv_size); + + let iovec = [ + iovec { + iov_base: len_buffer.as_mut_ptr() as *mut c_char, + iov_len: len_buffer.len(), + }, + iovec { + iov_base: main_data_buffer.as_mut_ptr() as *mut c_char, + iov_len: main_data_buffer.len(), + }, + ]; + let mut cmsg = UnixCmsg::new(&iovec); + let bytes_read = try!(cmsg.recv(fd, blocking_mode)); + main_data_buffer.set_len(bytes_read - len_buffer.len()); let cmsg_fds = cmsg.cmsg_buffer.offset(1) as *const u8 as *const c_int; let cmsg_length = cmsg.msghdr.msg_controllen; @@ -747,14 +768,8 @@ fn recv(fd: c_int, blocking_mode: BlockingMode) shared_memory_regions.push(UnixSharedMemory::from_fd(fd)); } - // Separate out the header. - let (header, main_data_buffer) = cmsg.data_buffer.split_at(mem::size_of::()); - let mut main_data_buffer: Vec = - main_data_buffer[0..(bytes_read - mem::size_of::())].iter() - .cloned() - .collect(); - let total_size = (&header[..]).read_uint::(mem::size_of::()) - .unwrap() as usize; + let total_size = (&len_buffer[..]).read_uint::(mem::size_of::()) + .unwrap() as usize; if total_size == main_data_buffer.len() { // Fast path: no fragments. return Ok((main_data_buffer, channels, shared_memory_regions)) @@ -851,10 +866,7 @@ unsafe fn map_file(fd: c_int, length: Option) -> (*mut u8, size_t) { } struct UnixCmsg { - data_buffer: Vec, cmsg_buffer: *mut cmsghdr, - #[allow(dead_code)] - iovec: Box, msghdr: msghdr, } @@ -869,30 +881,17 @@ impl Drop for UnixCmsg { } impl UnixCmsg { - unsafe fn new(maximum_recv_size: usize) -> UnixCmsg { + unsafe fn new(iovec: &[iovec]) -> UnixCmsg { let cmsg_length = mem::size_of::() + (MAX_FDS_IN_CMSG as usize) * mem::size_of::(); - assert!(maximum_recv_size > cmsg_length); - - // Allocate a buffer without initialising the memory. - let mut data_buffer = Vec::with_capacity(maximum_recv_size); - data_buffer.set_len(maximum_recv_size); - let cmsg_buffer = libc::malloc(cmsg_length) as *mut cmsghdr; - let iovec = Box::new(iovec { - iov_base: &mut data_buffer[0] as *mut _ as *mut c_char, - iov_len: data_buffer.len(), - }); - let iovec_ptr: *const iovec = &*iovec; UnixCmsg { - data_buffer: data_buffer, cmsg_buffer: cmsg_buffer, - iovec: iovec, msghdr: msghdr { msg_name: ptr::null_mut(), msg_namelen: 0, - msg_iov: iovec_ptr, - msg_iovlen: 1, + msg_iov: iovec.as_ptr(), + msg_iovlen: iovec.len(), msg_control: cmsg_buffer as *mut c_void, msg_controllen: cmsg_length, msg_flags: 0, @@ -909,8 +908,6 @@ impl UnixCmsg { } let result = recvmsg(fd, &mut self.msghdr, 0); - self.data_buffer.set_len(cmp::max(result, 0) as usize); - let result = if result > 0 { Ok(result as usize) } else if result == 0 { From 07fbcffbd694a59ac32a12f7374c9348ceb922db Mon Sep 17 00:00:00 2001 From: Olaf Buddenhagen Date: Fri, 8 Apr 2016 23:44:02 +0200 Subject: [PATCH 26/33] Linux: Don't check maximum send size individually for each channel The buffer size shouldn't change from one channel to another; so instead of using a syscall to check the size each time, just check it once and store it using a `lazy_static`. This also means `get_max_fragment_size()` becomes a static method now, as it no longer fetches the value for a specific channel, but rather just refers to the stored value. Note that we only check the send size now, and use it for the size of the receive buffer too. This is indeed more correct than the previous implementation, as the receive buffer needs to hold exactly as much as we might sent at most. (Normally they are the same anyway; but if for some reason the maximum receive size happened to be larger, the previous code would use a larger buffer than necessary. If the receive size happened to be *smaller*, either version would fail horribly...) This doesn't have a noticable performance impact on the sender, as the present implementation only checks the size *after* failing to send the whole message in one packet, i.e. only for large transfers, where the cost of the extra system call is insignificant. The receiver side on the other hand always does the check -- and thus the saved call actually yields a significant improvement for small messages: on my system, small transfers (send + receive) gain more than 20% performance. Along with the other improvements, they are now almost five times faster than the original implementation. test size_00_1 ... bench: 2,289 ns/iter (+/- 38) test size_01_2 ... bench: 2,346 ns/iter (+/- 22) test size_02_4 ... bench: 2,357 ns/iter (+/- 38) test size_03_8 ... bench: 2,374 ns/iter (+/- 42) test size_04_16 ... bench: 2,471 ns/iter (+/- 40) test size_05_32 ... bench: 2,371 ns/iter (+/- 45) test size_06_64 ... bench: 2,422 ns/iter (+/- 44) test size_07_128 ... bench: 2,385 ns/iter (+/- 30) test size_08_256 ... bench: 2,406 ns/iter (+/- 28) test size_09_512 ... bench: 2,499 ns/iter (+/- 56) test size_10_1k ... bench: 2,727 ns/iter (+/- 88) test size_11_2k ... bench: 3,924 ns/iter (+/- 47) test size_12_4k ... bench: 5,555 ns/iter (+/- 60) test size_13_8k ... bench: 9,455 ns/iter (+/- 107) test size_14_16k ... bench: 8,999 ns/iter (+/- 90) test size_15_32k ... bench: 13,647 ns/iter (+/- 105) test size_16_64k ... bench: 22,213 ns/iter (+/- 489) test size_17_128k ... bench: 43,666 ns/iter (+/- 17,217) test size_18_256k ... bench: 221,851 ns/iter (+/- 69,636) test size_19_512k ... bench: 451,801 ns/iter (+/- 113,742) test size_20_1m ... bench: 1,330,491 ns/iter (+/- 182,352) test size_21_2m ... bench: 1,790,956 ns/iter (+/- 489,327) test size_22_4m ... bench: 4,989,840 ns/iter (+/- 1,188,114) test size_23_8m ... bench: 9,349,559 ns/iter (+/- 1,334,978) test size_00_1 ... bench: 231,706 ns/iter (+/- 4,892) test size_01_2 ... bench: 235,017 ns/iter (+/- 6,437) test size_02_4 ... bench: 240,197 ns/iter (+/- 4,068) test size_03_8 ... bench: 244,404 ns/iter (+/- 6,090) test size_04_16 ... bench: 239,248 ns/iter (+/- 4,041) test size_05_32 ... bench: 243,360 ns/iter (+/- 5,237) test size_06_64 ... bench: 236,956 ns/iter (+/- 5,098) test size_07_128 ... bench: 243,579 ns/iter (+/- 7,305) test size_08_256 ... bench: 247,605 ns/iter (+/- 5,047) test size_09_512 ... bench: 276,882 ns/iter (+/- 6,950) test size_10_1k ... bench: 261,665 ns/iter (+/- 4,985) test size_11_2k ... bench: 395,244 ns/iter (+/- 5,495) test size_12_4k ... bench: 558,647 ns/iter (+/- 8,908) test size_13_8k ... bench: 941,395 ns/iter (+/- 7,215) test size_14_16k ... bench: 907,290 ns/iter (+/- 9,087) test size_15_32k ... bench: 1,360,839 ns/iter (+/- 9,137) test size_16_64k ... bench: 2,224,395 ns/iter (+/- 362,003) test size_17_128k ... bench: 4,351,960 ns/iter (+/- 1,726,184) test size_18_256k ... bench: 8,627,702 ns/iter (+/- 2,335,525) test size_19_512k ... bench: 19,018,116 ns/iter (+/- 2,757,467) test size_20_1m ... bench: 102,819,410 ns/iter (+/- 7,050,372) test size_21_2m ... bench: 133,774,605 ns/iter (+/- 14,188,872) test size_22_4m ... bench: 450,259,095 ns/iter (+/- 12,859,207) test size_23_8m ... bench: 875,984,486 ns/iter (+/- 21,168,557) --- benches/bench.rs | 2 +- platform/inprocess/mod.rs | 4 ++-- platform/linux/mod.rs | 32 ++++++++++++++------------------ platform/macos/mod.rs | 4 ++-- platform/test.rs | 2 +- 5 files changed, 20 insertions(+), 24 deletions(-) diff --git a/benches/bench.rs b/benches/bench.rs index 7033aca7d..aeb321928 100644 --- a/benches/bench.rs +++ b/benches/bench.rs @@ -25,7 +25,7 @@ fn bench_size(b: &mut test::Bencher, size: usize) { let (wait_tx, wait_rx) = mpsc::channel(); let wait_rx = Mutex::new(wait_rx); - if size > tx.get_max_fragment_size().unwrap() { + if size > platform::OsIpcSender::get_max_fragment_size() { b.iter(|| { crossbeam::scope(|scope| { scope.spawn(|| { diff --git a/platform/inprocess/mod.rs b/platform/inprocess/mod.rs index 99e6af629..c7aee05ac 100644 --- a/platform/inprocess/mod.rs +++ b/platform/inprocess/mod.rs @@ -149,8 +149,8 @@ impl MpscSender { Ok(record.sender) } - pub fn get_max_fragment_size(&self) -> Result { - Ok(usize::MAX) + pub fn get_max_fragment_size() -> usize { + usize::MAX } pub fn send(&self, diff --git a/platform/linux/mod.rs b/platform/linux/mod.rs index c95998161..64141bbd7 100644 --- a/platform/linux/mod.rs +++ b/platform/linux/mod.rs @@ -27,6 +27,13 @@ const MAX_FDS_IN_CMSG: u32 = 64; // Yes, really! const MAP_FAILED: *mut u8 = (!0usize) as *mut u8; +lazy_static! { + static ref SYSTEM_SENDBUF_SIZE: usize = { + let (tx, _) = channel().expect("Failed to obtain a socket for checking maximum send size"); + tx.get_system_sendbuf_size().expect("Failed to obtain maximum send size for socket") + }; +} + // The value Linux returns for SO_SNDBUF // is not the size we are actually allowed to use... // Empirically, we have to deduct 32 bytes from that. @@ -144,7 +151,7 @@ impl UnixSender { /// and with the size of the fragment header also deducted from it. /// /// The `sendbuf_size` passed in should usually be the maximum kernel buffer size, - /// as obtained with `get_system_sendbuf_size()` -- + /// i.e. the value of *SYSTEM_SENDBUF_SIZE -- /// except after getting ENOBUFS, in which case it needs to be reduced. fn fragment_size(sendbuf_size: usize) -> usize { sendbuf_size - RESERVED_SIZE - mem::size_of::() @@ -159,8 +166,8 @@ impl UnixSender { /// under normal circumstances. /// (It might still block if heavy memory pressure causes ENOBUFS, /// forcing us to reduce the packet size.) - pub fn get_max_fragment_size(&self) -> Result { - Ok(Self::fragment_size(try!(self.get_system_sendbuf_size()))) + pub fn get_max_fragment_size() -> usize { + Self::fragment_size(*SYSTEM_SENDBUF_SIZE) } pub fn send(&self, @@ -255,7 +262,7 @@ impl UnixSender { } } - let mut sendbuf_size = try!(self.get_system_sendbuf_size()); + let mut sendbuf_size = *SYSTEM_SENDBUF_SIZE; /// Reduce send buffer size after getting ENOBUFS, /// i.e. when the kernel failed to allocate a large enough buffer. @@ -717,24 +724,14 @@ enum BlockingMode { fn recv(fd: c_int, blocking_mode: BlockingMode) -> Result<(Vec, Vec, Vec),UnixError> { unsafe { - let mut maximum_recv_size: usize = 0; - let mut maximum_recv_size_len = mem::size_of::() as socklen_t; - if getsockopt(fd, - libc::SOL_SOCKET, - libc::SO_RCVBUF, - &mut maximum_recv_size as *mut usize as *mut c_void, - &mut maximum_recv_size_len as *mut socklen_t) < 0 { - return Err(UnixError::last()) - } - // First fragment begins with a header recording the total data length. // // We use this to determine whether we already got the entire message, // or need to receive additional fragments -- and if so, how much. let mut len_buffer = vec![0; mem::size_of::()]; // Allocate a buffer without initialising the memory. - let mut main_data_buffer = Vec::with_capacity(maximum_recv_size - len_buffer.len()); - main_data_buffer.set_len(maximum_recv_size); + let mut main_data_buffer = Vec::with_capacity(*SYSTEM_SENDBUF_SIZE - len_buffer.len()); + main_data_buffer.set_len(*SYSTEM_SENDBUF_SIZE - len_buffer.len()); let iovec = [ iovec { @@ -788,8 +785,7 @@ fn recv(fd: c_int, blocking_mode: BlockingMode) // Receive followup fragments directly into the main buffer. while main_data_buffer.len() < total_size { let write_pos = main_data_buffer.len(); - let end_pos = cmp::min(write_pos + UnixSender::fragment_size(maximum_recv_size), - total_size); + let end_pos = cmp::min(write_pos + UnixSender::get_max_fragment_size(), total_size); assert!(end_pos <= main_data_buffer.capacity()); main_data_buffer.set_len(end_pos); diff --git a/platform/macos/mod.rs b/platform/macos/mod.rs index 6d1ace156..cad48880f 100644 --- a/platform/macos/mod.rs +++ b/platform/macos/mod.rs @@ -366,8 +366,8 @@ impl MachSender { } } - pub fn get_max_fragment_size(&self) -> Result { - Ok(usize::MAX) + pub fn get_max_fragment_size() -> usize { + usize::MAX } pub fn send(&self, diff --git a/platform/test.rs b/platform/test.rs index 107bb2a10..f2e66fd04 100644 --- a/platform/test.rs +++ b/platform/test.rs @@ -211,7 +211,7 @@ mod fragment_tests { lazy_static! { static ref FRAGMENT_SIZE: usize = { - platform::channel().and_then(|(tx, _)| tx.get_max_fragment_size()).unwrap() + platform::OsIpcSender::get_max_fragment_size() }; } From 6cf99d910a50dd03230dccefddf08aac89e877fe Mon Sep 17 00:00:00 2001 From: Olaf Buddenhagen Date: Tue, 12 Apr 2016 07:49:54 +0200 Subject: [PATCH 27/33] Linux: Only send a control message if necessary Don't send a control message if we have no actual auxiliary data (channels / shared memory regions) to transfer. This shaves off another 6 or 7 per cent from small transfers (without FDs) on my system. test size_00_1 ... bench: 2,154 ns/iter (+/- 33) test size_01_2 ... bench: 2,203 ns/iter (+/- 42) test size_02_4 ... bench: 2,231 ns/iter (+/- 51) test size_03_8 ... bench: 2,231 ns/iter (+/- 28) test size_04_16 ... bench: 2,290 ns/iter (+/- 47) test size_05_32 ... bench: 2,261 ns/iter (+/- 57) test size_06_64 ... bench: 2,316 ns/iter (+/- 51) test size_07_128 ... bench: 2,247 ns/iter (+/- 38) test size_08_256 ... bench: 2,266 ns/iter (+/- 45) test size_09_512 ... bench: 2,572 ns/iter (+/- 52) test size_10_1k ... bench: 2,488 ns/iter (+/- 45) test size_11_2k ... bench: 3,791 ns/iter (+/- 51) test size_12_4k ... bench: 5,365 ns/iter (+/- 54) test size_13_8k ... bench: 9,235 ns/iter (+/- 84) test size_14_16k ... bench: 8,833 ns/iter (+/- 102) test size_15_32k ... bench: 13,370 ns/iter (+/- 117) test size_16_64k ... bench: 22,004 ns/iter (+/- 717) test size_17_128k ... bench: 43,369 ns/iter (+/- 976) test size_18_256k ... bench: 224,096 ns/iter (+/- 75,219) test size_19_512k ... bench: 458,353 ns/iter (+/- 149,531) test size_20_1m ... bench: 1,357,956 ns/iter (+/- 187,198) test size_21_2m ... bench: 1,781,991 ns/iter (+/- 512,027) test size_22_4m ... bench: 4,940,065 ns/iter (+/- 1,099,861) test size_23_8m ... bench: 9,345,216 ns/iter (+/- 1,557,181) test size_00_1 ... bench: 222,064 ns/iter (+/- 8,292) test size_01_2 ... bench: 224,589 ns/iter (+/- 4,033) test size_02_4 ... bench: 226,667 ns/iter (+/- 4,774) test size_03_8 ... bench: 229,002 ns/iter (+/- 5,107) test size_04_16 ... bench: 224,895 ns/iter (+/- 3,323) test size_05_32 ... bench: 230,973 ns/iter (+/- 3,265) test size_06_64 ... bench: 224,377 ns/iter (+/- 5,778) test size_07_128 ... bench: 229,364 ns/iter (+/- 8,282) test size_08_256 ... bench: 235,654 ns/iter (+/- 3,860) test size_09_512 ... bench: 235,874 ns/iter (+/- 6,021) test size_10_1k ... bench: 246,200 ns/iter (+/- 2,626) test size_11_2k ... bench: 386,233 ns/iter (+/- 5,313) test size_12_4k ... bench: 542,364 ns/iter (+/- 8,671) test size_13_8k ... bench: 929,892 ns/iter (+/- 12,989) test size_14_16k ... bench: 868,966 ns/iter (+/- 7,768) test size_15_32k ... bench: 1,342,400 ns/iter (+/- 6,781) test size_16_64k ... bench: 2,202,955 ns/iter (+/- 86,042) test size_17_128k ... bench: 4,323,643 ns/iter (+/- 41,162) test size_18_256k ... bench: 8,708,255 ns/iter (+/- 2,162,868) test size_19_512k ... bench: 19,281,153 ns/iter (+/- 5,449,494) test size_20_1m ... bench: 102,875,141 ns/iter (+/- 10,584,288) test size_21_2m ... bench: 134,820,099 ns/iter (+/- 13,015,545) test size_22_4m ... bench: 446,291,502 ns/iter (+/- 12,073,997) test size_23_8m ... bench: 876,656,087 ns/iter (+/- 16,439,303) --- platform/linux/mod.rs | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/platform/linux/mod.rs b/platform/linux/mod.rs index 64141bbd7..31c890765 100644 --- a/platform/linux/mod.rs +++ b/platform/linux/mod.rs @@ -194,14 +194,19 @@ impl UnixSender { -> Result<(),UnixError> { let result = unsafe { let cmsg_length = mem::size_of_val(fds); - let cmsg_buffer = libc::malloc(CMSG_SPACE(cmsg_length)) as *mut cmsghdr; - (*cmsg_buffer).cmsg_len = CMSG_LEN(cmsg_length); - (*cmsg_buffer).cmsg_level = libc::SOL_SOCKET; - (*cmsg_buffer).cmsg_type = SCM_RIGHTS; - - ptr::copy_nonoverlapping(fds.as_ptr(), - cmsg_buffer.offset(1) as *mut _ as *mut c_int, - fds.len()); + let (cmsg_buffer, cmsg_space) = if cmsg_length > 0 { + let cmsg_buffer = libc::malloc(CMSG_SPACE(cmsg_length)) as *mut cmsghdr; + (*cmsg_buffer).cmsg_len = CMSG_LEN(cmsg_length); + (*cmsg_buffer).cmsg_level = libc::SOL_SOCKET; + (*cmsg_buffer).cmsg_type = SCM_RIGHTS; + + ptr::copy_nonoverlapping(fds.as_ptr(), + cmsg_buffer.offset(1) as *mut _ as *mut c_int, + fds.len()); + (cmsg_buffer, CMSG_SPACE(cmsg_length)) + } else { + (ptr::null_mut(), 0) + }; // First fragment begins with a header recording the total data length. // @@ -231,7 +236,7 @@ impl UnixSender { msg_iov: iovec.as_ptr(), msg_iovlen: iovec.len(), msg_control: cmsg_buffer as *mut c_void, - msg_controllen: CMSG_SPACE(cmsg_length), + msg_controllen: cmsg_space, msg_flags: 0, }; From 224a65240e4ce2dcec9883056c97931741cb86bc Mon Sep 17 00:00:00 2001 From: Olaf Buddenhagen Date: Sat, 16 Apr 2016 00:47:20 +0200 Subject: [PATCH 28/33] Linux: Align fragment size On a 32 bit system, the size header is only 4 bytes; so if we fully use the rest of the available buffer for payload data, the size of the latter won't be a multiple of 8 bytes -- and consequently, every second fragment is read from the source data buffer with poor alignment. Fixing this by always aligning the payload data size sent per fragment to 8 byte boundaries. (On 64 bit systems, this is a no-op. According to my testing, aligning to more than 8 byte boundaries doesn't benefit either 32 or 64 bit systems.) On my 32 bit x86 system, this produces quite a sizeable performance improvement for large transfers: peaking at 20% or so around 640 KiB, and staying above 10% for most of the range from 320 KiB to 2 MiB. test size_00_1 ... bench: 2,250 ns/iter (+/- 42) test size_01_2 ... bench: 2,279 ns/iter (+/- 46) test size_02_4 ... bench: 2,335 ns/iter (+/- 53) test size_03_8 ... bench: 2,351 ns/iter (+/- 67) test size_04_16 ... bench: 2,357 ns/iter (+/- 46) test size_05_32 ... bench: 2,327 ns/iter (+/- 64) test size_06_64 ... bench: 2,308 ns/iter (+/- 39) test size_07_128 ... bench: 2,277 ns/iter (+/- 43) test size_08_256 ... bench: 2,453 ns/iter (+/- 35) test size_09_512 ... bench: 2,402 ns/iter (+/- 57) test size_10_1k ... bench: 2,576 ns/iter (+/- 81) test size_11_2k ... bench: 3,883 ns/iter (+/- 66) test size_12_4k ... bench: 5,492 ns/iter (+/- 75) test size_13_8k ... bench: 9,368 ns/iter (+/- 83) test size_14_16k ... bench: 8,857 ns/iter (+/- 107) test size_15_32k ... bench: 14,132 ns/iter (+/- 140) test size_16_64k ... bench: 22,049 ns/iter (+/- 298) test size_17_128k ... bench: 43,577 ns/iter (+/- 1,947) test size_18_256k ... bench: 203,597 ns/iter (+/- 34,495) test size_19_512k ... bench: 407,050 ns/iter (+/- 246,606) test size_20_1m ... bench: 1,334,506 ns/iter (+/- 186,923) test size_21_2m ... bench: 1,630,275 ns/iter (+/- 481,481) test size_22_4m ... bench: 4,826,184 ns/iter (+/- 980,708) test size_23_8m ... bench: 9,390,020 ns/iter (+/- 1,655,050) test size_00_1 ... bench: 223,092 ns/iter (+/- 3,807) test size_01_2 ... bench: 223,918 ns/iter (+/- 3,535) test size_02_4 ... bench: 223,102 ns/iter (+/- 4,907) test size_03_8 ... bench: 230,394 ns/iter (+/- 4,700) test size_04_16 ... bench: 224,395 ns/iter (+/- 4,482) test size_05_32 ... bench: 231,436 ns/iter (+/- 4,214) test size_06_64 ... bench: 225,216 ns/iter (+/- 3,584) test size_07_128 ... bench: 228,905 ns/iter (+/- 4,260) test size_08_256 ... bench: 233,108 ns/iter (+/- 2,998) test size_09_512 ... bench: 236,013 ns/iter (+/- 4,803) test size_10_1k ... bench: 248,637 ns/iter (+/- 5,386) test size_11_2k ... bench: 383,750 ns/iter (+/- 6,134) test size_12_4k ... bench: 542,666 ns/iter (+/- 9,026) test size_13_8k ... bench: 934,623 ns/iter (+/- 9,200) test size_14_16k ... bench: 894,028 ns/iter (+/- 7,425) test size_15_32k ... bench: 1,350,185 ns/iter (+/- 6,316) test size_16_64k ... bench: 2,213,693 ns/iter (+/- 24,549) test size_17_128k ... bench: 4,348,762 ns/iter (+/- 90,784) test size_18_256k ... bench: 7,825,637 ns/iter (+/- 2,243,873) test size_19_512k ... bench: 17,792,873 ns/iter (+/- 3,775,904) test size_20_1m ... bench: 99,769,568 ns/iter (+/- 8,760,588) test size_21_2m ... bench: 126,575,377 ns/iter (+/- 12,528,236) test size_22_4m ... bench: 440,368,689 ns/iter (+/- 16,627,208) test size_23_8m ... bench: 859,509,896 ns/iter (+/- 21,848,024) --- platform/linux/mod.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/platform/linux/mod.rs b/platform/linux/mod.rs index 31c890765..abccba33f 100644 --- a/platform/linux/mod.rs +++ b/platform/linux/mod.rs @@ -154,7 +154,8 @@ impl UnixSender { /// i.e. the value of *SYSTEM_SENDBUF_SIZE -- /// except after getting ENOBUFS, in which case it needs to be reduced. fn fragment_size(sendbuf_size: usize) -> usize { - sendbuf_size - RESERVED_SIZE - mem::size_of::() + (sendbuf_size - RESERVED_SIZE - mem::size_of::()) + & (!8usize + 1) // Ensure optimal alignment. } /// Maximum data size that can be transferred over this channel in a single packet. From 02902dd0cf9f49ebb87563e4531b9f780a5065d1 Mon Sep 17 00:00:00 2001 From: Olaf Buddenhagen Date: Sun, 24 Apr 2016 22:54:25 +0200 Subject: [PATCH 29/33] Linux: Drop use of `byteorder` Using `byteorder` has neven been actually *necessary* (the serialised data doesn't ever cross machine boundaries) -- it was only being (ab-)used as a convenient way to write/read the header information to/from the shared buffer. Now that the header gets separate send/receive buffers, this isn't actually a simplification anymore -- on the contrary: just using the header data's backing storage as the send/receive buffer directly is indeed simpler now. (And not significantly more unsafe either.) The simpler code also improves performance of small transfers by another two or three per cent. test size_00_1 ... bench: 2,154 ns/iter (+/- 83) test size_01_2 ... bench: 2,154 ns/iter (+/- 75) test size_02_4 ... bench: 2,234 ns/iter (+/- 34) test size_03_8 ... bench: 2,212 ns/iter (+/- 21) test size_04_16 ... bench: 2,298 ns/iter (+/- 49) test size_05_32 ... bench: 2,211 ns/iter (+/- 32) test size_06_64 ... bench: 2,225 ns/iter (+/- 76) test size_07_128 ... bench: 2,202 ns/iter (+/- 40) test size_08_256 ... bench: 2,239 ns/iter (+/- 48) test size_09_512 ... bench: 2,316 ns/iter (+/- 30) test size_10_1k ... bench: 2,446 ns/iter (+/- 29) test size_11_2k ... bench: 3,797 ns/iter (+/- 67) test size_12_4k ... bench: 5,381 ns/iter (+/- 62) test size_13_8k ... bench: 9,225 ns/iter (+/- 96) test size_14_16k ... bench: 8,739 ns/iter (+/- 60) test size_15_32k ... bench: 13,243 ns/iter (+/- 76) test size_16_64k ... bench: 21,879 ns/iter (+/- 141) test size_17_128k ... bench: 43,193 ns/iter (+/- 398) test size_18_256k ... bench: 205,695 ns/iter (+/- 52,004) test size_19_512k ... bench: 409,146 ns/iter (+/- 68,651) test size_20_1m ... bench: 1,341,949 ns/iter (+/- 240,066) test size_21_2m ... bench: 1,662,774 ns/iter (+/- 527,172) test size_22_4m ... bench: 4,885,677 ns/iter (+/- 1,113,293) test size_23_8m ... bench: 9,300,784 ns/iter (+/- 1,806,111) test size_00_1 ... bench: 211,985 ns/iter (+/- 3,389) test size_01_2 ... bench: 210,848 ns/iter (+/- 5,040) test size_02_4 ... bench: 218,757 ns/iter (+/- 3,885) test size_03_8 ... bench: 218,317 ns/iter (+/- 5,671) test size_04_16 ... bench: 219,027 ns/iter (+/- 5,073) test size_05_32 ... bench: 218,795 ns/iter (+/- 4,695) test size_06_64 ... bench: 218,217 ns/iter (+/- 3,875) test size_07_128 ... bench: 224,165 ns/iter (+/- 4,367) test size_08_256 ... bench: 227,112 ns/iter (+/- 3,922) test size_09_512 ... bench: 225,733 ns/iter (+/- 3,931) test size_10_1k ... bench: 239,269 ns/iter (+/- 4,323) test size_11_2k ... bench: 371,675 ns/iter (+/- 6,760) test size_12_4k ... bench: 529,841 ns/iter (+/- 7,052) test size_13_8k ... bench: 910,285 ns/iter (+/- 7,308) test size_14_16k ... bench: 860,518 ns/iter (+/- 7,659) test size_15_32k ... bench: 1,331,114 ns/iter (+/- 5,774) test size_16_64k ... bench: 2,193,192 ns/iter (+/- 22,878) test size_17_128k ... bench: 4,324,455 ns/iter (+/- 86,997) test size_18_256k ... bench: 7,973,472 ns/iter (+/- 1,688,190) test size_19_512k ... bench: 17,325,137 ns/iter (+/- 6,222,526) test size_20_1m ... bench: 100,037,281 ns/iter (+/- 8,976,164) test size_21_2m ... bench: 127,489,104 ns/iter (+/- 14,776,106) test size_22_4m ... bench: 438,418,131 ns/iter (+/- 13,543,391) test size_23_8m ... bench: 858,248,355 ns/iter (+/- 14,316,633) --- Cargo.toml | 1 - lib.rs | 1 - platform/linux/mod.rs | 36 ++++++++++++++---------------------- 3 files changed, 14 insertions(+), 24 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index e87c178df..ed7ff7ebf 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -8,7 +8,6 @@ path = "lib.rs" [dependencies] bincode = ">=0.4.1, <0.6" -byteorder = "0.5" lazy_static = "0.2" libc = "0.2" rand = "0.3" diff --git a/lib.rs b/lib.rs index fc1fa1c7e..4a8dd2970 100644 --- a/lib.rs +++ b/lib.rs @@ -16,7 +16,6 @@ extern crate lazy_static; extern crate bincode; -extern crate byteorder; extern crate libc; extern crate rand; extern crate serde; diff --git a/platform/linux/mod.rs b/platform/linux/mod.rs index abccba33f..e7c60aae8 100644 --- a/platform/linux/mod.rs +++ b/platform/linux/mod.rs @@ -8,7 +8,6 @@ // except according to those terms. use bincode::serde::DeserializeError; -use byteorder::{LittleEndian, ReadBytesExt, WriteBytesExt}; use libc::{self, MAP_SHARED, PROT_READ, PROT_WRITE, c_char, c_int, c_short, c_ulong}; use libc::{c_ushort, c_void, mode_t, off_t, size_t, sockaddr, sockaddr_un, socklen_t, ssize_t}; use std::cmp; @@ -209,21 +208,15 @@ impl UnixSender { (ptr::null_mut(), 0) }; - // First fragment begins with a header recording the total data length. - // - // The receiver uses this to determine whether it already got the entire message, - // or needs to receive additional fragments -- and if so, how much. - let mut len_buffer = vec![0; mem::size_of_val(&len)]; - { - let mut len_buffer = &mut len_buffer[..]; - len_buffer.write_uint::(len as u64, mem::size_of_val(&len)) - .unwrap(); - } - let iovec = [ + // First fragment begins with a header recording the total data length. + // + // The receiver uses this to determine + // whether it already got the entire message, + // or needs to receive additional fragments -- and if so, how much. iovec { - iov_base: len_buffer.as_ptr() as *const c_char as *mut c_char, - iov_len: len_buffer.len(), + iov_base: &len as *const _ as *mut c_char, + iov_len: mem::size_of_val(&len), }, iovec { iov_base: data_buffer.as_ptr() as *const c_char as *mut c_char, @@ -734,15 +727,16 @@ fn recv(fd: c_int, blocking_mode: BlockingMode) // // We use this to determine whether we already got the entire message, // or need to receive additional fragments -- and if so, how much. - let mut len_buffer = vec![0; mem::size_of::()]; + let mut total_size = 0usize; // Allocate a buffer without initialising the memory. - let mut main_data_buffer = Vec::with_capacity(*SYSTEM_SENDBUF_SIZE - len_buffer.len()); - main_data_buffer.set_len(*SYSTEM_SENDBUF_SIZE - len_buffer.len()); + let mut main_data_buffer = Vec::with_capacity(*SYSTEM_SENDBUF_SIZE + - mem::size_of_val(&total_size)); + main_data_buffer.set_len(*SYSTEM_SENDBUF_SIZE - mem::size_of_val(&total_size)); let iovec = [ iovec { - iov_base: len_buffer.as_mut_ptr() as *mut c_char, - iov_len: len_buffer.len(), + iov_base: &mut total_size as *mut _ as *mut c_char, + iov_len: mem::size_of_val(&total_size), }, iovec { iov_base: main_data_buffer.as_mut_ptr() as *mut c_char, @@ -752,7 +746,7 @@ fn recv(fd: c_int, blocking_mode: BlockingMode) let mut cmsg = UnixCmsg::new(&iovec); let bytes_read = try!(cmsg.recv(fd, blocking_mode)); - main_data_buffer.set_len(bytes_read - len_buffer.len()); + main_data_buffer.set_len(bytes_read - mem::size_of_val(&total_size)); let cmsg_fds = cmsg.cmsg_buffer.offset(1) as *const u8 as *const c_int; let cmsg_length = cmsg.msghdr.msg_controllen; @@ -771,8 +765,6 @@ fn recv(fd: c_int, blocking_mode: BlockingMode) shared_memory_regions.push(UnixSharedMemory::from_fd(fd)); } - let total_size = (&len_buffer[..]).read_uint::(mem::size_of::()) - .unwrap() as usize; if total_size == main_data_buffer.len() { // Fast path: no fragments. return Ok((main_data_buffer, channels, shared_memory_regions)) From af0223d3ef9ce80f9cef0192724da69ccb0bd5e0 Mon Sep 17 00:00:00 2001 From: Olaf Buddenhagen Date: Wed, 13 Apr 2016 00:26:16 +0200 Subject: [PATCH 30/33] Linux: Precise calculation of followup fragment size Followup fragments don't have a header; so they can use a few bytes more for payload. While this is not likely ever to make a noticable performance difference, having exact calculations in each case seems cleaner, hopefully avoiding potential confusion... --- platform/linux/mod.rs | 31 +++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/platform/linux/mod.rs b/platform/linux/mod.rs index e7c60aae8..89ffde043 100644 --- a/platform/linux/mod.rs +++ b/platform/linux/mod.rs @@ -143,17 +143,20 @@ impl UnixSender { /// Calculate maximum payload data size per fragment. /// - /// This is the size of the main data chunk only -- - /// it's independent of any auxiliary data (FDs) transferred along with it. - /// It is the total size of the kernel buffer, - /// minus the part reserved by the kernel, - /// and with the size of the fragment header also deducted from it. + /// It is the total size of the kernel buffer, minus the part reserved by the kernel. /// /// The `sendbuf_size` passed in should usually be the maximum kernel buffer size, /// i.e. the value of *SYSTEM_SENDBUF_SIZE -- /// except after getting ENOBUFS, in which case it needs to be reduced. fn fragment_size(sendbuf_size: usize) -> usize { - (sendbuf_size - RESERVED_SIZE - mem::size_of::()) + sendbuf_size - RESERVED_SIZE + } + + /// Calculate maximum payload data size of first fragment. + /// + /// This one is smaller than regular fragments, because it carries the message (size) header. + fn first_fragment_size(sendbuf_size: usize) -> usize { + (Self::fragment_size(sendbuf_size) - mem::size_of::()) & (!8usize + 1) // Ensure optimal alignment. } @@ -167,7 +170,7 @@ impl UnixSender { /// (It might still block if heavy memory pressure causes ENOBUFS, /// forcing us to reduce the packet size.) pub fn get_max_fragment_size() -> usize { - Self::fragment_size(*SYSTEM_SENDBUF_SIZE) + Self::first_fragment_size(*SYSTEM_SENDBUF_SIZE) } pub fn send(&self, @@ -309,16 +312,19 @@ impl UnixSender { // Split up the packet into fragments. let mut byte_position = 0; while byte_position < data.len() { - let bytes_per_fragment = Self::fragment_size(sendbuf_size); - - let end_byte_position = cmp::min(data.len(), byte_position + bytes_per_fragment); - + let end_byte_position; let result = if byte_position == 0 { // First fragment. No offset; but contains message header (total size). // The auxiliary data (FDs) is also sent along with this one. + + // This fragment always uses the full allowable buffer size. + end_byte_position = Self::first_fragment_size(sendbuf_size); send_first_fragment(self.fd, &fds[..], &data[..end_byte_position], data.len()) } else { // Followup fragment. No header; but offset by amount of data already sent. + + end_byte_position = cmp::min(byte_position + Self::fragment_size(sendbuf_size), + data.len()); send_followup_fragment(dedicated_tx.fd, &data[byte_position..end_byte_position]) }; @@ -783,7 +789,8 @@ fn recv(fd: c_int, blocking_mode: BlockingMode) // Receive followup fragments directly into the main buffer. while main_data_buffer.len() < total_size { let write_pos = main_data_buffer.len(); - let end_pos = cmp::min(write_pos + UnixSender::get_max_fragment_size(), total_size); + let end_pos = cmp::min(write_pos + UnixSender::fragment_size(*SYSTEM_SENDBUF_SIZE), + total_size); assert!(end_pos <= main_data_buffer.capacity()); main_data_buffer.set_len(end_pos); From 5f27e85f90b396bd13960db158904b1c9ef16eef Mon Sep 17 00:00:00 2001 From: Olaf Buddenhagen Date: Sat, 9 Apr 2016 20:05:42 +0200 Subject: [PATCH 31/33] Linux: Check maximum send size up front Only try sending the entire message in one packet if it will actually fit. This saves a syscall and some processing, but only for large (fragmented) messages -- so it doesn't have a noticable performance impact. However, it should make behaviour clearer and more predictable; and it is also required in order to enable further cleanups. --- platform/linux/mod.rs | 31 ++++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/platform/linux/mod.rs b/platform/linux/mod.rs index 89ffde043..cdee96512 100644 --- a/platform/linux/mod.rs +++ b/platform/linux/mod.rs @@ -283,19 +283,24 @@ impl UnixSender { } } - match send_first_fragment(self.fd, &fds[..], data, data.len()) { - Ok(_) => return Ok(()), - Err(error) => { - if error.0 == libc::ENOBUFS && downsize(&mut sendbuf_size, data.len()).is_ok() { - // If we get this error, - // it means the message was small enough to fit the maximum send size, - // but the kernel failed to allocate a buffer large enough - // to actually transfer the message -- - // so we have to proceed with a fragmented send nevertheless. - } else if error.0 != libc::EMSGSIZE { - return Err(error) - } - }, + // If the message is small enough, try sending it in a single fragment. + if data.len() <= Self::get_max_fragment_size() { + match send_first_fragment(self.fd, &fds[..], data, data.len()) { + Ok(_) => return Ok(()), + Err(error) => { + // ENOBUFS means the kernel failed to allocate a buffer large enough + // to actually transfer the message, + // although the message was small enough to fit the maximum send size -- + // so we have to proceed with a fragmented send nevertheless, + // using a reduced send buffer size. + // + // Any other errors we might get here are non-recoverable. + if !(error.0 == libc::ENOBUFS + && downsize(&mut sendbuf_size, data.len()).is_ok()) { + return Err(error) + } + }, + } } // The packet is too big. Fragmentation time! From 0bda93aa93ef5bcf2043090435a64a766c7e6788 Mon Sep 17 00:00:00 2001 From: Olaf Buddenhagen Date: Wed, 27 Apr 2016 23:45:36 +0200 Subject: [PATCH 32/33] Linux: Don't over-allocate first fragment receive buffer Now that we fully control the size of the first packet even in the non-fragmented case, we can rely on this size on the receiver side as well, rather than having to allocate a larger buffer just in case. --- platform/linux/mod.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/platform/linux/mod.rs b/platform/linux/mod.rs index cdee96512..54c23c433 100644 --- a/platform/linux/mod.rs +++ b/platform/linux/mod.rs @@ -740,9 +740,8 @@ fn recv(fd: c_int, blocking_mode: BlockingMode) // or need to receive additional fragments -- and if so, how much. let mut total_size = 0usize; // Allocate a buffer without initialising the memory. - let mut main_data_buffer = Vec::with_capacity(*SYSTEM_SENDBUF_SIZE - - mem::size_of_val(&total_size)); - main_data_buffer.set_len(*SYSTEM_SENDBUF_SIZE - mem::size_of_val(&total_size)); + let mut main_data_buffer = Vec::with_capacity(UnixSender::get_max_fragment_size()); + main_data_buffer.set_len(UnixSender::get_max_fragment_size()); let iovec = [ iovec { From 7c2466e13f8b20f0596b073f9ad25765ef614de0 Mon Sep 17 00:00:00 2001 From: Olaf Buddenhagen Date: Thu, 28 Apr 2016 00:01:00 +0200 Subject: [PATCH 33/33] Linux: Cleanup: Trim `unsafe` blocks in `recv()` This requires some shuffling around of declarations, to faciliate untangling the actually unsafe operations from those not affecting safety. (As far as reasonably possible...) Also added a new assertion to make sure that the trimmed `unsafe` blocks really do not rely on any conditions being upheld outside. --- platform/linux/mod.rs | 77 ++++++++++++++++++++++++------------------- 1 file changed, 43 insertions(+), 34 deletions(-) diff --git a/platform/linux/mod.rs b/platform/linux/mod.rs index 54c23c433..7eb1dda4a 100644 --- a/platform/linux/mod.rs +++ b/platform/linux/mod.rs @@ -733,14 +733,18 @@ enum BlockingMode { fn recv(fd: c_int, blocking_mode: BlockingMode) -> Result<(Vec, Vec, Vec),UnixError> { + + let (mut channels, mut shared_memory_regions) = (Vec::new(), Vec::new()); + + // First fragments begins with a header recording the total data length. + // + // We use this to determine whether we already got the entire message, + // or need to receive additional fragments -- and if so, how much. + let mut total_size = 0usize; + let mut main_data_buffer; unsafe { - // First fragment begins with a header recording the total data length. - // - // We use this to determine whether we already got the entire message, - // or need to receive additional fragments -- and if so, how much. - let mut total_size = 0usize; // Allocate a buffer without initialising the memory. - let mut main_data_buffer = Vec::with_capacity(UnixSender::get_max_fragment_size()); + main_data_buffer = Vec::with_capacity(UnixSender::get_max_fragment_size()); main_data_buffer.set_len(UnixSender::get_max_fragment_size()); let iovec = [ @@ -765,7 +769,6 @@ fn recv(fd: c_int, blocking_mode: BlockingMode) } else { (cmsg.cmsg_len() - mem::size_of::()) / mem::size_of::() }; - let (mut channels, mut shared_memory_regions) = (Vec::new(), Vec::new()); for index in 0..channel_length { let fd = *cmsg_fds.offset(index as isize); if is_socket(fd) { @@ -774,30 +777,35 @@ fn recv(fd: c_int, blocking_mode: BlockingMode) } shared_memory_regions.push(UnixSharedMemory::from_fd(fd)); } + } - if total_size == main_data_buffer.len() { - // Fast path: no fragments. - return Ok((main_data_buffer, channels, shared_memory_regions)) - } + if total_size == main_data_buffer.len() { + // Fast path: no fragments. + return Ok((main_data_buffer, channels, shared_memory_regions)) + } - // Reassemble fragments. - // - // The initial fragment carries the receive end of a dedicated channel - // through which all the remaining fragments will be coming in. - let dedicated_rx = channels.pop().unwrap().to_receiver(); - - // Extend the buffer to hold the entire message, without initialising the memory. - let len = main_data_buffer.len(); - main_data_buffer.reserve(total_size - len); - - // Receive followup fragments directly into the main buffer. - while main_data_buffer.len() < total_size { - let write_pos = main_data_buffer.len(); - let end_pos = cmp::min(write_pos + UnixSender::fragment_size(*SYSTEM_SENDBUF_SIZE), - total_size); + // Reassemble fragments. + // + // The initial fragment carries the receive end of a dedicated channel + // through which all the remaining fragments will be coming in. + let dedicated_rx = channels.pop().unwrap().to_receiver(); + + // Extend the buffer to hold the entire message, without initialising the memory. + let len = main_data_buffer.len(); + main_data_buffer.reserve(total_size - len); + + // Receive followup fragments directly into the main buffer. + while main_data_buffer.len() < total_size { + let write_pos = main_data_buffer.len(); + let end_pos = cmp::min(write_pos + UnixSender::fragment_size(*SYSTEM_SENDBUF_SIZE), + total_size); + let result = unsafe { assert!(end_pos <= main_data_buffer.capacity()); main_data_buffer.set_len(end_pos); + // Integer underflow could make the following code unsound... + assert!(end_pos >= write_pos); + // Note: we always use blocking mode for followup fragments, // to make sure that once we start receiving a multi-fragment message, // we don't abort in the middle of it... @@ -806,16 +814,17 @@ fn recv(fd: c_int, blocking_mode: BlockingMode) end_pos - write_pos, 0); main_data_buffer.set_len(write_pos + cmp::max(result, 0) as usize); + result + }; - if result == 0 { - return Err(UnixError(libc::ECONNRESET)) - } else if result < 0 { - return Err(UnixError::last()) - }; - } - - Ok((main_data_buffer, channels, shared_memory_regions)) + if result == 0 { + return Err(UnixError(libc::ECONNRESET)) + } else if result < 0 { + return Err(UnixError::last()) + }; } + + Ok((main_data_buffer, channels, shared_memory_regions)) } #[cfg(target_os="android")]