From 9dc5a4d28f7f889cbae6fc56b7652a9c034682a2 Mon Sep 17 00:00:00 2001 From: Ruihan Li Date: Mon, 27 Nov 2023 23:58:41 +0800 Subject: [PATCH] Box the large structure `AnyUnboundSocket` So far it is quite common to put the `AnyUnboundSocket` in an enum variant, e.g. the following code snippet. ```rust enum Inner { Unbound(AlwaysSome>), Bound(AlwaysSome>), // ... } ``` However, this pattern is very memory inefficient because the size difference between two enum variants is significant. The size of `AnyUnboundSocket` is much larger than the size of `Arc`, where the latter is simply a pointer and a reference counter. In fact, we're about to trigger Clippy's large_enum_variant warning. We're just below its threshold, so the warning doesn't appear. The solution is simple: If we put `AnyBoundSocket` in `Arc`, we should also put `AnyUnboundSocket` in `Box`. This way the sizes of different enum variants will be similar. --- services/libs/jinux-std/src/net/iface/common.rs | 4 ++-- services/libs/jinux-std/src/net/iface/mod.rs | 4 ++-- services/libs/jinux-std/src/net/socket/ip/common.rs | 4 ++-- services/libs/jinux-std/src/net/socket/ip/datagram.rs | 11 ++++++----- .../libs/jinux-std/src/net/socket/ip/stream/init.rs | 4 ++-- .../libs/jinux-std/src/net/socket/ip/stream/listen.rs | 2 +- 6 files changed, 15 insertions(+), 14 deletions(-) diff --git a/services/libs/jinux-std/src/net/iface/common.rs b/services/libs/jinux-std/src/net/iface/common.rs index f4f8ee939..7123f429b 100644 --- a/services/libs/jinux-std/src/net/iface/common.rs +++ b/services/libs/jinux-std/src/net/iface/common.rs @@ -98,9 +98,9 @@ impl IfaceCommon { pub(super) fn bind_socket( &self, iface: Arc, - socket: AnyUnboundSocket, + socket: Box, config: BindPortConfig, - ) -> core::result::Result, (Error, AnyUnboundSocket)> { + ) -> core::result::Result, (Error, Box)> { let port = if let Some(port) = config.port() { port } else { diff --git a/services/libs/jinux-std/src/net/iface/mod.rs b/services/libs/jinux-std/src/net/iface/mod.rs index 04fb0b4bb..56c2ff55f 100644 --- a/services/libs/jinux-std/src/net/iface/mod.rs +++ b/services/libs/jinux-std/src/net/iface/mod.rs @@ -37,9 +37,9 @@ pub trait Iface: internal::IfaceInternal + Send + Sync { /// See discussion at https://github.com/smoltcp-rs/smoltcp/issues/779. fn bind_socket( &self, - socket: AnyUnboundSocket, + socket: Box, config: BindPortConfig, - ) -> core::result::Result, (Error, AnyUnboundSocket)> { + ) -> core::result::Result, (Error, Box)> { let common = self.common(); let socket_type_inner = socket.socket_family(); common.bind_socket(self.arc_self(), socket, config) diff --git a/services/libs/jinux-std/src/net/socket/ip/common.rs b/services/libs/jinux-std/src/net/socket/ip/common.rs index a7caea3cf..0e18ff42a 100644 --- a/services/libs/jinux-std/src/net/socket/ip/common.rs +++ b/services/libs/jinux-std/src/net/socket/ip/common.rs @@ -40,10 +40,10 @@ fn get_ephemeral_iface(remote_ip_addr: &IpAddress) -> Arc { } pub(super) fn bind_socket( - unbound_socket: AnyUnboundSocket, + unbound_socket: Box, endpoint: IpEndpoint, can_reuse: bool, -) -> core::result::Result, (Error, AnyUnboundSocket)> { +) -> core::result::Result, (Error, Box)> { let iface = match get_iface_to_bind(&endpoint.addr) { Some(iface) => iface, None => { diff --git a/services/libs/jinux-std/src/net/socket/ip/datagram.rs b/services/libs/jinux-std/src/net/socket/ip/datagram.rs index 718e29795..3e423798d 100644 --- a/services/libs/jinux-std/src/net/socket/ip/datagram.rs +++ b/services/libs/jinux-std/src/net/socket/ip/datagram.rs @@ -27,7 +27,7 @@ pub struct DatagramSocket { } enum Inner { - Unbound(AlwaysSome), + Unbound(AlwaysSome>), Bound(Arc), } @@ -148,7 +148,7 @@ impl Inner { impl DatagramSocket { pub fn new(nonblocking: bool) -> Self { - let udp_socket = AnyUnboundSocket::new_udp(); + let udp_socket = Box::new(AnyUnboundSocket::new_udp()); Self { inner: RwLock::new(Inner::Unbound(AlwaysSome::new(udp_socket))), nonblocking: AtomicBool::new(nonblocking), @@ -176,16 +176,17 @@ impl DatagramSocket { } fn try_bind_empheral(&self, remote_endpoint: &IpEndpoint) -> Result> { + // Fast path if let Inner::Bound(bound) = &*self.inner.read() { return Ok(bound.clone()); } + // Slow path let mut inner = self.inner.write(); if let Inner::Bound(bound) = &*inner { - Ok(bound.clone()) - } else { - inner.bind_to_ephemeral_endpoint(remote_endpoint) + return Ok(bound.clone()); } + inner.bind_to_ephemeral_endpoint(remote_endpoint) } } diff --git a/services/libs/jinux-std/src/net/socket/ip/stream/init.rs b/services/libs/jinux-std/src/net/socket/ip/stream/init.rs index f184ae220..2da70549b 100644 --- a/services/libs/jinux-std/src/net/socket/ip/stream/init.rs +++ b/services/libs/jinux-std/src/net/socket/ip/stream/init.rs @@ -16,7 +16,7 @@ pub struct InitStream { } enum Inner { - Unbound(AlwaysSome), + Unbound(AlwaysSome>), Bound(AlwaysSome>), Connecting { bound_socket: Arc, @@ -114,7 +114,7 @@ impl Inner { impl InitStream { pub fn new(nonblocking: bool) -> Self { - let socket = AnyUnboundSocket::new_tcp(); + let socket = Box::new(AnyUnboundSocket::new_tcp()); let inner = Inner::Unbound(AlwaysSome::new(socket)); Self { is_nonblocking: AtomicBool::new(nonblocking), diff --git a/services/libs/jinux-std/src/net/socket/ip/stream/listen.rs b/services/libs/jinux-std/src/net/socket/ip/stream/listen.rs index 207fa9678..06cce98e9 100644 --- a/services/libs/jinux-std/src/net/socket/ip/stream/listen.rs +++ b/services/libs/jinux-std/src/net/socket/ip/stream/listen.rs @@ -133,7 +133,7 @@ impl BacklogSocket { Errno::EINVAL, "the socket is not bound", ))?; - let unbound_socket = AnyUnboundSocket::new_tcp(); + let unbound_socket = Box::new(AnyUnboundSocket::new_tcp()); let bound_socket = { let iface = bound_socket.iface(); let bind_port_config = BindPortConfig::new(local_endpoint.port, true)?;