Skip to content

Commit

Permalink
commons: use smallvec to store message arguments
Browse files Browse the repository at this point in the history
Any message with 4 argument or less will have these arguments stored
inline rather than in an heap-allocated vec.

The number 4 was chose because almost all messages have 4 arguments
or less, and some potentially very spammy messages (wl_touch.move)
have exactly 4 arguments. Avoiding allocations in these cases should
generally be a gain.

Moreother, to avoid bloating the size of Message due to this,
String and Array arguments are further boxed, reducing the size
of Argument from 4*usize to 2*usize. These kind of arguments are
generally pretty rare, so the double allocation should overall not
counter the size gain.

Closes #268, #249
  • Loading branch information
elinorbgr authored and vberger committed Sep 12, 2019
1 parent b1d5ec2 commit b933030
Show file tree
Hide file tree
Showing 19 changed files with 106 additions and 68 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
- [client] the `cursor` and `egl` features are split into their own crates: `wayland-egl` and `wayland-cursor`
- [server] All dependencies on `calloop` are now removed, `wayland-server` now only exposes a `dispatch(..)`
and `get_poll_fd()` method, that you are responsible for integrating into your event loop.
- [commons] Use `smallvec` to store the arguments of messages having 4 or less, drastically reducing the
number of allocations when using the rust implementation of the protocol.

## 0.23.6 -- 2019-09-06

Expand Down
7 changes: 4 additions & 3 deletions tests/protocol_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use helpers::{roundtrip, wayc, ways, TestClient, TestServer};
extern crate nix;
extern crate wayland_commons as wc;

use wc::smallvec;
use wc::socket::{BufferedSocket, Socket};
use wc::wire::{Argument, Message};

Expand All @@ -28,7 +29,7 @@ fn client_wrong_id() {
.write_message(&Message {
sender_id: 1, // wl_display
opcode: 1, // wl_registry
args: vec![
args: smallvec![
Argument::NewId(3), // should be 2
],
})
Expand All @@ -54,7 +55,7 @@ fn client_wrong_opcode() {
.write_message(&Message {
sender_id: 1, // wl_display
opcode: 42, // inexistant
args: vec![],
args: smallvec![],
})
.unwrap();
socket.flush().unwrap();
Expand All @@ -78,7 +79,7 @@ fn client_wrong_sender() {
.write_message(&Message {
sender_id: 54, // wl_display
opcode: 0, // inexistant
args: vec![],
args: smallvec![],
})
.unwrap();
socket.flush().unwrap();
Expand Down
34 changes: 19 additions & 15 deletions tests/scanner_assets/client_code.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ pub mod wl_foo {
use super::sys::client::*;
use super::sys::common::{wl_argument, wl_array, wl_interface, wl_message};
use super::{
types_null, AnonymousObject, Argument, ArgumentType, Interface, Main, Message, MessageDesc,
smallvec, types_null, AnonymousObject, Argument, ArgumentType, Interface, Main, Message, MessageDesc,
MessageGroup, Object, ObjectMetadata, Proxy, NULLPTR,
};
use std::os::raw::c_char;
Expand Down Expand Up @@ -134,18 +134,20 @@ pub mod wl_foo {
} => Message {
sender_id: sender_id,
opcode: 0,
args: vec![
args: smallvec![
Argument::Int(number),
Argument::Uint(unumber),
Argument::Str(unsafe { ::std::ffi::CString::from_vec_unchecked(text.into()) }),
Argument::Str(Box::new(unsafe {
::std::ffi::CString::from_vec_unchecked(text.into())
})),
Argument::Fixed((float * 256.) as i32),
Argument::Fd(file),
],
},
Request::CreateBar {} => Message {
sender_id: sender_id,
opcode: 1,
args: vec![Argument::NewId(0)],
args: smallvec![Argument::NewId(0),],
},
}
}
Expand Down Expand Up @@ -369,7 +371,7 @@ pub mod wl_bar {
use super::sys::client::*;
use super::sys::common::{wl_argument, wl_array, wl_interface, wl_message};
use super::{
types_null, AnonymousObject, Argument, ArgumentType, Interface, Main, Message, MessageDesc,
smallvec, types_null, AnonymousObject, Argument, ArgumentType, Interface, Main, Message, MessageDesc,
MessageGroup, Object, ObjectMetadata, Proxy, NULLPTR,
};
use std::os::raw::c_char;
Expand Down Expand Up @@ -475,17 +477,17 @@ pub mod wl_bar {
} => Message {
sender_id: sender_id,
opcode: 0,
args: vec![
args: smallvec![
Argument::Uint(kind.to_raw()),
Argument::Object(target.as_ref().id()),
Argument::Array(metadata),
Argument::Array(metametadata.unwrap_or_else(Vec::new)),
Argument::Array(Box::new(metadata)),
Argument::Array(Box::new(metametadata.unwrap_or_else(Vec::new))),
],
},
Request::Release => Message {
sender_id: sender_id,
opcode: 1,
args: vec![],
args: smallvec![],
},
Request::_Self {
_self,
Expand All @@ -499,7 +501,7 @@ pub mod wl_bar {
} => Message {
sender_id: sender_id,
opcode: 2,
args: vec![
args: smallvec![
Argument::Uint(_self),
Argument::Uint(_mut),
Argument::Uint(object),
Expand Down Expand Up @@ -861,7 +863,7 @@ pub mod wl_display {
use super::sys::client::*;
use super::sys::common::{wl_argument, wl_array, wl_interface, wl_message};
use super::{
types_null, AnonymousObject, Argument, ArgumentType, Interface, Main, Message, MessageDesc,
smallvec, types_null, AnonymousObject, Argument, ArgumentType, Interface, Main, Message, MessageDesc,
MessageGroup, Object, ObjectMetadata, Proxy, NULLPTR,
};
use std::os::raw::c_char;
Expand Down Expand Up @@ -1012,7 +1014,7 @@ pub mod wl_registry {
use super::sys::client::*;
use super::sys::common::{wl_argument, wl_array, wl_interface, wl_message};
use super::{
types_null, AnonymousObject, Argument, ArgumentType, Interface, Main, Message, MessageDesc,
smallvec, types_null, AnonymousObject, Argument, ArgumentType, Interface, Main, Message, MessageDesc,
MessageGroup, Object, ObjectMetadata, Proxy, NULLPTR,
};
use std::os::raw::c_char;
Expand Down Expand Up @@ -1062,9 +1064,11 @@ pub mod wl_registry {
Request::Bind { name, id } => Message {
sender_id: sender_id,
opcode: 0,
args: vec![
args: smallvec![
Argument::Uint(name),
Argument::Str(unsafe { ::std::ffi::CString::from_vec_unchecked(id.0.into()) }),
Argument::Str(Box::new(unsafe {
::std::ffi::CString::from_vec_unchecked(id.0.into())
})),
Argument::Uint(id.1),
Argument::NewId(0),
],
Expand Down Expand Up @@ -1213,7 +1217,7 @@ pub mod wl_callback {
use super::sys::client::*;
use super::sys::common::{wl_argument, wl_array, wl_interface, wl_message};
use super::{
types_null, AnonymousObject, Argument, ArgumentType, Interface, Main, Message, MessageDesc,
smallvec, types_null, AnonymousObject, Argument, ArgumentType, Interface, Main, Message, MessageDesc,
MessageGroup, Object, ObjectMetadata, Proxy, NULLPTR,
};
use std::os::raw::c_char;
Expand Down
16 changes: 8 additions & 8 deletions tests/scanner_assets/server_code.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ pub mod wl_foo {
use super::sys::common::{wl_argument, wl_array, wl_interface, wl_message};
use super::sys::server::*;
use super::{
types_null, AnonymousObject, Argument, ArgumentType, Interface, Main, Message, MessageDesc,
smallvec, types_null, AnonymousObject, Argument, ArgumentType, Interface, Main, Message, MessageDesc,
MessageGroup, Object, ObjectMetadata, Resource, NULLPTR,
};
use std::os::raw::c_char;
Expand Down Expand Up @@ -264,7 +264,7 @@ pub mod wl_foo {
Event::Cake { kind, amount } => Message {
sender_id: sender_id,
opcode: 0,
args: vec![Argument::Uint(kind.to_raw()), Argument::Uint(amount)],
args: smallvec![Argument::Uint(kind.to_raw()), Argument::Uint(amount),],
},
}
}
Expand Down Expand Up @@ -371,7 +371,7 @@ pub mod wl_bar {
use super::sys::common::{wl_argument, wl_array, wl_interface, wl_message};
use super::sys::server::*;
use super::{
types_null, AnonymousObject, Argument, ArgumentType, Interface, Main, Message, MessageDesc,
smallvec, types_null, AnonymousObject, Argument, ArgumentType, Interface, Main, Message, MessageDesc,
MessageGroup, Object, ObjectMetadata, Resource, NULLPTR,
};
use std::os::raw::c_char;
Expand Down Expand Up @@ -484,7 +484,7 @@ pub mod wl_bar {
},
metadata: {
if let Some(Argument::Array(val)) = args.next() {
val
*val
} else {
return Err(());
}
Expand All @@ -494,7 +494,7 @@ pub mod wl_bar {
if val.len() == 0 {
None
} else {
Some(val)
Some(*val)
}
} else {
return Err(());
Expand Down Expand Up @@ -692,7 +692,7 @@ pub mod wl_bar {
} => Message {
sender_id: sender_id,
opcode: 0,
args: vec![
args: smallvec![
Argument::Uint(_self),
Argument::Uint(_mut),
Argument::Uint(object),
Expand Down Expand Up @@ -850,7 +850,7 @@ pub mod wl_callback {
use super::sys::common::{wl_argument, wl_array, wl_interface, wl_message};
use super::sys::server::*;
use super::{
types_null, AnonymousObject, Argument, ArgumentType, Interface, Main, Message, MessageDesc,
smallvec, types_null, AnonymousObject, Argument, ArgumentType, Interface, Main, Message, MessageDesc,
MessageGroup, Object, ObjectMetadata, Resource, NULLPTR,
};
use std::os::raw::c_char;
Expand Down Expand Up @@ -951,7 +951,7 @@ pub mod wl_callback {
Event::Done { callback_data } => Message {
sender_id: sender_id,
opcode: 0,
args: vec![Argument::Uint(callback_data)],
args: smallvec![Argument::Uint(callback_data),],
},
}
}
Expand Down
1 change: 1 addition & 0 deletions wayland-client/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ pub mod protocol {

pub(crate) use crate::{AnonymousObject, Attached, Main, Proxy, ProxyMap};
pub(crate) use wayland_commons::map::{Object, ObjectMetadata};
pub(crate) use wayland_commons::smallvec;
pub(crate) use wayland_commons::wire::{Argument, ArgumentType, Message, MessageDesc};
pub(crate) use wayland_commons::{Interface, MessageGroup};
pub(crate) use wayland_sys as sys;
Expand Down
1 change: 1 addition & 0 deletions wayland-client/src/rust_imp/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ impl ProxyMap {
/*
* Dispatching logic
*/
#[allow(clippy::large_enum_variant)]
pub(crate) enum Dispatched {
Yes,
NoDispatch(Message, ProxyInner),
Expand Down
2 changes: 1 addition & 1 deletion wayland-client/src/rust_imp/queues.rs
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ fn message_to_rawevent(msg: Message, proxy: &ProxyInner, map: &mut super::ProxyM
.map(|a| match a {
Argument::Int(i) => crate::Argument::Int(i),
Argument::Uint(u) => crate::Argument::Uint(u),
Argument::Array(v) => crate::Argument::Array(if v.is_empty() { None } else { Some(v) }),
Argument::Array(v) => crate::Argument::Array(if v.is_empty() { None } else { Some(*v) }),
Argument::Fixed(f) => crate::Argument::Float((f as f32) / 256.),
Argument::Fd(f) => crate::Argument::Fd(f),
Argument::Str(cs) => crate::Argument::Str({
Expand Down
1 change: 1 addition & 0 deletions wayland-commons/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,4 @@ travis-ci = { repository = "Smithay/wayland-rs" }
wayland-sys = { version = "0.24.0-pre", path = "../wayland-sys" }
nix = "0.15"
spin = "0.5"
smallvec = "0.6"
3 changes: 2 additions & 1 deletion wayland-commons/examples/manual_global_list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use std::os::unix::io::{FromRawFd, IntoRawFd};
use std::os::unix::net::UnixStream;
use std::path::PathBuf;

use wc::smallvec;
use wc::socket::{BufferedSocket, Socket};
use wc::wire::{Argument, ArgumentType, Message, MessageDesc};

Expand All @@ -20,7 +21,7 @@ fn main() {
.write_message(&Message {
sender_id: 1, // wl_display
opcode: 1, // get registry
args: vec![
args: smallvec![
Argument::NewId(2), // id of the created registry
],
})
Expand Down
2 changes: 2 additions & 0 deletions wayland-commons/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ pub mod socket;
pub mod user_data;
pub mod wire;

pub use smallvec::smallvec;

/// A group of messages
///
/// This represents a group of message that can be serialized on the protocol wire.
Expand Down
22 changes: 12 additions & 10 deletions wayland-commons/src/socket.rs
Original file line number Diff line number Diff line change
Expand Up @@ -435,6 +435,8 @@ mod tests {

use std::ffi::CString;

use smallvec::smallvec;

fn same_file(a: RawFd, b: RawFd) -> bool {
let stat1 = ::nix::sys::stat::fstat(a).unwrap();
let stat2 = ::nix::sys::stat::fstat(b).unwrap();
Expand Down Expand Up @@ -463,11 +465,11 @@ mod tests {
let msg = Message {
sender_id: 42,
opcode: 7,
args: vec![
args: smallvec![
Argument::Uint(3),
Argument::Fixed(-89),
Argument::Str(CString::new(&b"I like trains!"[..]).unwrap()),
Argument::Array(vec![1, 2, 3, 4, 5, 6, 7, 8, 9]),
Argument::Str(Box::new(CString::new(&b"I like trains!"[..]).unwrap())),
Argument::Array(vec![1, 2, 3, 4, 5, 6, 7, 8, 9].into()),
Argument::Object(88),
Argument::NewId(56),
Argument::Int(-25),
Expand Down Expand Up @@ -516,7 +518,7 @@ mod tests {
let msg = Message {
sender_id: 42,
opcode: 7,
args: vec![
args: smallvec![
Argument::Fd(1), // stdin
Argument::Fd(0), // stdout
],
Expand Down Expand Up @@ -557,23 +559,23 @@ mod tests {
Message {
sender_id: 42,
opcode: 0,
args: vec![
args: smallvec![
Argument::Int(42),
Argument::Str(CString::new(&b"I like trains"[..]).unwrap()),
Argument::Str(Box::new(CString::new(&b"I like trains"[..]).unwrap())),
],
},
Message {
sender_id: 42,
opcode: 1,
args: vec![
args: smallvec![
Argument::Fd(1), // stdin
Argument::Fd(0), // stdout
],
},
Message {
sender_id: 42,
opcode: 2,
args: vec![
args: smallvec![
Argument::Uint(3),
Argument::Fd(2), // stderr
],
Expand Down Expand Up @@ -625,9 +627,9 @@ mod tests {
let msg = Message {
sender_id: 2,
opcode: 0,
args: vec![
args: smallvec![
Argument::Uint(18),
Argument::Str(CString::new(&b"wl_shell"[..]).unwrap()),
Argument::Str(Box::new(CString::new(&b"wl_shell"[..]).unwrap())),
Argument::Uint(1),
],
};
Expand Down
Loading

0 comments on commit b933030

Please sign in to comment.