x11rb::atom_manager's output size is very large #912
Comments
|
Using That doesn't look worse than what I would expected...? Perhaps LLVM decided to inline everything? I might be on a different Rust version than you, but I looked at However, there is a linear increase of... I don't know what after every call to |
|
I copied the definition of the The idea is to first create an array of all the However, to make that work, I had to jump through some hoops. But at least it's a start.
Ideas? (The patch obviously cannot be applied by anyone, but it should get the idea across) diff --git a/src/platform_impl/linux/x11/atoms.rs b/src/platform_impl/linux/x11/atoms.rs
index 6b9ab49e..23aaeee3 100644
--- a/src/platform_impl/linux/x11/atoms.rs
+++ b/src/platform_impl/linux/x11/atoms.rs
@@ -36,13 +36,24 @@ macro_rules! x11rb_atom_manager {
$vis fn new<C: x11rb::protocol::xproto::ConnectionExt>(
_conn: &C,
) -> ::std::result::Result<$cookie_name<'_, C>, x11rb::errors::ConnectionError> {
+ // Locally create an enum that allows to get the index for an atom
+ #[allow(non_camel_case_types)]
+ enum Indicies { $($field_name,)* }
+
+ let cookies = [
+ $(_conn.intern_atom(
+ false,
+ __atom_manager_atom_value!($field_name$(: $atom_value)?),
+ ),
+ )*
+ ];
+ let atoms: Result<Vec<_>, _> = cookies.into_iter().collect();
+ let mut atoms: Vec<_> = atoms?.into_iter().map(Some).collect();
+
Ok($cookie_name {
phantom: std::marker::PhantomData,
$(
- $field_name: _conn.intern_atom(
- false,
- __atom_manager_atom_value!($field_name$(: $atom_value)?),
- )?,
+ $field_name: atoms[Indicies::$field_name as usize].take().expect("TODO good text"),
)*
})
} |
|
Idea to avoid the diff --git a/src/platform_impl/linux/x11/atoms.rs b/src/platform_impl/linux/x11/atoms.rs
index 6b9ab49e..849c4cdb 100644
--- a/src/platform_impl/linux/x11/atoms.rs
+++ b/src/platform_impl/linux/x11/atoms.rs
@@ -36,13 +36,20 @@ macro_rules! x11rb_atom_manager {
$vis fn new<C: x11rb::protocol::xproto::ConnectionExt>(
_conn: &C,
) -> ::std::result::Result<$cookie_name<'_, C>, x11rb::errors::ConnectionError> {
+ let cookies = [
+ $(_conn.intern_atom(
+ false,
+ __atom_manager_atom_value!($field_name$(: $atom_value)?),
+ ),
+ )*
+ ];
+ let atoms: Result<Vec<_>, _> = cookies.into_iter().collect();
+ let mut atoms = atoms?;
+
Ok($cookie_name {
phantom: std::marker::PhantomData,
$(
- $field_name: _conn.intern_atom(
- false,
- __atom_manager_atom_value!($field_name$(: $atom_value)?),
- )?,
+ $field_name: atoms.remove(0),
)*
})
}The resulting assembly isn't too bad, but contains a bunch of "randomly moving stuff around" and a call to |
|
Since the fields of the diff --git a/src/platform_impl/linux/x11/atoms.rs b/src/platform_impl/linux/x11/atoms.rs
index 6b9ab49e..93fdf003 100644
--- a/src/platform_impl/linux/x11/atoms.rs
+++ b/src/platform_impl/linux/x11/atoms.rs
@@ -17,9 +17,7 @@ macro_rules! x11rb_atom_manager {
$(#[$cookie_meta])*
$vis struct $cookie_name<'a, C: x11rb::protocol::xproto::ConnectionExt> {
phantom: std::marker::PhantomData<&'a C>,
- $(
- $field_name: x11rb::cookie::Cookie<'a, C, x11rb::protocol::xproto::InternAtomReply>,
- )*
+ cookies: Vec<x11rb::cookie::Cookie<'a, C, x11rb::protocol::xproto::InternAtomReply>>,
}
// Replies
@@ -36,23 +34,27 @@ macro_rules! x11rb_atom_manager {
$vis fn new<C: x11rb::protocol::xproto::ConnectionExt>(
_conn: &C,
) -> ::std::result::Result<$cookie_name<'_, C>, x11rb::errors::ConnectionError> {
- Ok($cookie_name {
- phantom: std::marker::PhantomData,
- $(
- $field_name: _conn.intern_atom(
+ let cookies = [
+ $(_conn.intern_atom(
false,
__atom_manager_atom_value!($field_name$(: $atom_value)?),
- )?,
+ ),
)*
+ ];
+ let cookies: Result<Vec<_>, _> = cookies.into_iter().collect();
+
+ Ok($cookie_name {
+ phantom: std::marker::PhantomData,
+ cookies: cookies?,
})
}
}
impl<'a, C: x11rb::protocol::xproto::ConnectionExt> $cookie_name<'a, C> {
- $vis fn reply(self) -> ::std::result::Result<$struct_name, x11rb::errors::ReplyError> {
+ $vis fn reply(mut self) -> ::std::result::Result<$struct_name, x11rb::errors::ReplyError> {
Ok($struct_name {
$(
- $field_name: self.$field_name.reply()?.atom,
+ $field_name: self.cookies.remove(0).reply()?.atom,
)*
})
} |
|
The Although I think that, instead of |
winit uses atom_manager! with 59 atom names. Due to some sub-optimal code generation, this causes the generated new() function to end up with a size of 111.6 KiB, or almost 5% of the .text size of a winit example. The issue is that each "intern_atom()?" can cause an early exit and needs to call drop() of the already generated Cookies. This leads to some quadratic growth. To work around this, in this commit I change the implementation of new(). It now iterates over the atom names, using .map() to send the InternAtom requests. Finally, collect() is then used to change this Iterator<Item=Result<Cookie, Error>> into Result<Vec<Cookie>, Error>. This even preserves the early-exit behaviour in case of errors. The reply() function is then changed to turn the Vec into an iterator, using next() to pull out the items. This relies on the evaluation of this code to be well-defined (which e.g. wouldn't be guaranteed in C). Luckily, Rust provides the needed guarantee: rust-lang/reference#888 Fixes: #912 Signed-off-by: Uli Schlachter <psychon@znc.in>
I noticed this while running
cargo-bloaton awinitexample:Note the one at the top. It looks like creating a new atom set ends up being over 1% of a release mode executable.
The text was updated successfully, but these errors were encountered: