Merge pull request #914 from psychon/atom-manager-binary-size

Optimise atom_manager! implementation
psychon · Feb 15, 2024 · 6592b38 · 6592b38
2 parents 2b2d115 + f431b67
commit 6592b38
Show file tree

Hide file tree

Showing 2 changed files with 54 additions and 36 deletions.
diff --git a/x11rb/src/x11_utils.rs b/x11rb/src/x11_utils.rs
@@ -44,17 +44,19 @@ pub use x11rb_protocol::x11_utils::{
 /// #[derive(Debug)]
 /// /// A handle to a response from the X11 server.
 /// struct AtomCollectionCookie<'c, C: ConnectionExt> {
-///     phantom: std::marker::PhantomData<&'c C>,
-///     _NET_WM_NAME: Cookie<'c, C, InternAtomReply>,
-///     _NET_WM_ICON: Cookie<'c, C, InternAtomReply>,
-///     ATOM_WITH_SPACES: Cookie<'c, C, InternAtomReply>,
-///     WHATEVER: Cookie<'c, C, InternAtomReply>,
+///     // please treat the actual members as private
+///     # phantom: std::marker::PhantomData<&'c C>,
+///     # _NET_WM_NAME: Cookie<'c, C, InternAtomReply>,
+///     # _NET_WM_ICON: Cookie<'c, C, InternAtomReply>,
+///     # ATOM_WITH_SPACES: Cookie<'c, C, InternAtomReply>,
+///     # WHATEVER: Cookie<'c, C, InternAtomReply>,
 /// }
 ///
 /// impl AtomCollection {
 ///     pub fn new<C: ConnectionExt>(
 ///         conn: &C,
 ///     ) -> Result<AtomCollectionCookie<'_, C>, ConnectionError> {
+///         // This is just an example for readability; the actual code is more efficient.
 ///         Ok(AtomCollectionCookie {
 ///             phantom: std::marker::PhantomData,
 ///             _NET_WM_NAME: conn.intern_atom(false, b"_NET_WM_NAME")?,
@@ -70,6 +72,7 @@ pub use x11rb_protocol::x11_utils::{
 ///     C: ConnectionExt,
 /// {
 ///     pub fn reply(self) -> Result<AtomCollection, ReplyError> {
+///         // This is just an example for readability; the actual code is different.
 ///         Ok(AtomCollection {
 ///             _NET_WM_NAME: self._NET_WM_NAME.reply()?.atom,
 ///             _NET_WM_ICON: self._NET_WM_ICON.reply()?.atom,
@@ -94,10 +97,8 @@ macro_rules! atom_manager {
         #[derive(Debug)]
         $(#[$cookie_meta])*
         $vis struct $cookie_name<'a, C: $crate::protocol::xproto::ConnectionExt> {
-            phantom: std::marker::PhantomData<&'a C>,
-            $(
-                $field_name: $crate::cookie::Cookie<'a, C, $crate::protocol::xproto::InternAtomReply>,
-            )*
+            __private_phantom: ::std::marker::PhantomData<&'a C>,
+            __private_cookies: ::std::vec::Vec<$crate::cookie::Cookie<'a, C, $crate::protocol::xproto::InternAtomReply>>,
         }
 
         // Replies
@@ -114,23 +115,24 @@ macro_rules! atom_manager {
             $vis fn new<C: $crate::protocol::xproto::ConnectionExt>(
                 _conn: &C,
             ) -> ::std::result::Result<$cookie_name<'_, C>, $crate::errors::ConnectionError> {
+                let names = [
+                    $($crate::__atom_manager_atom_value!($field_name$(: $atom_value)?),)*
+                ];
+                let cookies: ::std::result::Result<::std::vec::Vec<_>, _>
+                    = names.into_iter().map(|name| _conn.intern_atom(false, name)).collect();
                 Ok($cookie_name {
-                    phantom: std::marker::PhantomData,
-                    $(
-                        $field_name: _conn.intern_atom(
-                            false,
-                            $crate::__atom_manager_atom_value!($field_name$(: $atom_value)?),
-                        )?,
-                    )*
+                    __private_phantom: ::std::marker::PhantomData,
+                    __private_cookies: cookies?,
                 })
             }
         }
 
         impl<'a, C: $crate::protocol::xproto::ConnectionExt> $cookie_name<'a, C> {
             $vis fn reply(self) -> ::std::result::Result<$struct_name, $crate::errors::ReplyError> {
+                let mut replies = self.__private_cookies.into_iter();
                 Ok($struct_name {
                     $(
-                        $field_name: self.$field_name.reply()?.atom,
+                        $field_name: replies.next().expect("new() should have constructed a Vec of the correct size").reply()?.atom,
                     )*
                 })
             }

diff --git a/x11rb/tests/x11_utils.rs b/x11rb/tests/x11_utils.rs
@@ -1,5 +1,8 @@
-use std::{collections::HashMap, io::IoSlice};
-use x11rb::{
+use ::std::{
+    collections::HashMap, io::IoSlice, option::Option as RealOption, result::Result as RealResult,
+    vec::Vec as RealVec,
+};
+use ::x11rb::{
     atom_manager,
     connection::{
         BufWithFds, DiscardMode, ReplyOrError, RequestConnection, RequestKind, SequenceNumber,
@@ -11,6 +14,19 @@ use x11rb::{
     x11_utils::{ExtensionInformation, Serialize, TryParse, TryParseFd},
 };
 
+// Just to be annoying, overwrite some standard types.
+// We want to make sure atom_manager! does not use these directly.
+mod std {}
+mod x11rb {}
+#[allow(dead_code)]
+type Vec = ();
+#[allow(dead_code)]
+type Result = ();
+#[allow(dead_code)]
+type PhantomData = ();
+#[allow(dead_code)]
+type Option = ();
+
 atom_manager! {
     Atoms: AtomsCookies {
         FIRST,
@@ -24,21 +40,21 @@ struct AtomFakeConnection {
     // Mapping from byte string to the corresponding atom value and sequence number.
     // If no entry is found, sending the InternAtom request fails.
     // If the entry does not fit into u16, fetching the reply fails.
-    atoms_and_cookies: HashMap<Vec<u8>, u32>,
+    atoms_and_cookies: HashMap<RealVec<u8>, u32>,
 }
 
 impl RequestConnection for AtomFakeConnection {
-    type Buf = Vec<u8>;
+    type Buf = RealVec<u8>;
 
     fn send_request_with_reply<R>(
         &self,
         bufs: &[IoSlice],
-        _fds: Vec<RawFdContainer>,
-    ) -> Result<Cookie<Self, R>, ConnectionError>
+        _fds: RealVec<RawFdContainer>,
+    ) -> RealResult<Cookie<Self, R>, ConnectionError>
     where
         R: TryParse,
     {
-        let bytes: Vec<u8> = bufs.iter().flat_map(|buf| buf.iter().copied()).collect();
+        let bytes: RealVec<u8> = bufs.iter().flat_map(|buf| buf.iter().copied()).collect();
         // request type and only-if-exists flag
         assert_eq!(bytes[..2], [INTERN_ATOM_REQUEST, 0]);
         // We ignore the length field
@@ -55,8 +71,8 @@ impl RequestConnection for AtomFakeConnection {
     fn send_request_with_reply_with_fds<R>(
         &self,
         _bufs: &[IoSlice],
-        _fds: Vec<RawFdContainer>,
-    ) -> Result<CookieWithFds<Self, R>, ConnectionError>
+        _fds: RealVec<RawFdContainer>,
+    ) -> RealResult<CookieWithFds<Self, R>, ConnectionError>
     where
         R: TryParseFd,
     {
@@ -66,8 +82,8 @@ impl RequestConnection for AtomFakeConnection {
     fn send_request_without_reply(
         &self,
         _bufs: &[IoSlice],
-        _fds: Vec<RawFdContainer>,
-    ) -> Result<VoidCookie<Self>, ConnectionError> {
+        _fds: RealVec<RawFdContainer>,
+    ) -> RealResult<VoidCookie<Self>, ConnectionError> {
         unimplemented!()
     }
 
@@ -78,21 +94,21 @@ impl RequestConnection for AtomFakeConnection {
     fn prefetch_extension_information(
         &self,
         _extension_name: &'static str,
-    ) -> Result<(), ConnectionError> {
+    ) -> RealResult<(), ConnectionError> {
         unimplemented!();
     }
 
     fn extension_information(
         &self,
         _extension_name: &'static str,
-    ) -> Result<Option<ExtensionInformation>, ConnectionError> {
+    ) -> RealResult<RealOption<ExtensionInformation>, ConnectionError> {
         unimplemented!()
     }
 
     fn wait_for_reply_or_raw_error(
         &self,
         sequence: SequenceNumber,
-    ) -> Result<ReplyOrError<Vec<u8>>, ConnectionError> {
+    ) -> RealResult<ReplyOrError<RealVec<u8>>, ConnectionError> {
         let sequence_u16 = match u16::try_from(sequence) {
             Ok(value) => value,
             Err(_) => return Err(ConnectionError::UnsupportedExtension),
@@ -110,21 +126,21 @@ impl RequestConnection for AtomFakeConnection {
     fn wait_for_reply(
         &self,
         _sequence: SequenceNumber,
-    ) -> Result<Option<Vec<u8>>, ConnectionError> {
+    ) -> RealResult<RealOption<RealVec<u8>>, ConnectionError> {
         unimplemented!()
     }
 
     fn wait_for_reply_with_fds_raw(
         &self,
         _sequence: SequenceNumber,
-    ) -> Result<ReplyOrError<BufWithFds<Vec<u8>>, Vec<u8>>, ConnectionError> {
+    ) -> RealResult<ReplyOrError<BufWithFds<RealVec<u8>>, RealVec<u8>>, ConnectionError> {
         unimplemented!()
     }
 
     fn check_for_raw_error(
         &self,
         _sequence: SequenceNumber,
-    ) -> Result<Option<Vec<u8>>, ConnectionError> {
+    ) -> RealResult<RealOption<RealVec<u8>>, ConnectionError> {
         unimplemented!()
     }
 
@@ -136,11 +152,11 @@ impl RequestConnection for AtomFakeConnection {
         unimplemented!()
     }
 
-    fn parse_error(&self, _error: &[u8]) -> Result<x11rb::x11_utils::X11Error, ParseError> {
+    fn parse_error(&self, _error: &[u8]) -> RealResult<::x11rb::x11_utils::X11Error, ParseError> {
         unimplemented!()
     }
 
-    fn parse_event(&self, _event: &[u8]) -> Result<x11rb::protocol::Event, ParseError> {
+    fn parse_event(&self, _event: &[u8]) -> RealResult<::x11rb::protocol::Event, ParseError> {
         unimplemented!()
     }
 }