From 100a501f0faf098cc64f254d55ba6fde5a413874 Mon Sep 17 00:00:00 2001 From: Vilgot Fredenberg Date: Fri, 11 Feb 2022 17:37:15 +0100 Subject: [PATCH] perf(gateway,model)!: optimize `Event` size (#1436) This PR chooses an arbitrary size for the `Event` enum, and adds and removes `Box`es around variant types respectively. Instead of fixing a clippy lint each time it appears, this codifies a system to ensure the `Event` enum stays consistent. Co-authored-by: Cassandra McCarthy Co-authored-by: Zeyla Hellyer --- cache/in-memory/src/event/channel.rs | 4 +- cache/in-memory/src/lib.rs | 14 ++-- model/src/gateway/event/dispatch.rs | 42 +++++----- model/src/gateway/event/mod.rs | 111 ++++++++++++++++++++++++--- standby/src/lib.rs | 16 ++-- 5 files changed, 136 insertions(+), 51 deletions(-) diff --git a/cache/in-memory/src/event/channel.rs b/cache/in-memory/src/event/channel.rs index 1ca79b8782e..1abbabb2d12 100644 --- a/cache/in-memory/src/event/channel.rs +++ b/cache/in-memory/src/event/channel.rs @@ -185,8 +185,8 @@ mod tests { .unwrap() .contains(&channel_id)); - cache.update(&Event::ChannelDelete(ChannelDelete(Channel::Guild( - channel, + cache.update(&Event::ChannelDelete(Box::new(ChannelDelete( + Channel::Guild(channel), )))); assert!(cache.channels_guild.is_empty()); assert!(cache.guild_channels.get(&guild_id).unwrap().is_empty()); diff --git a/cache/in-memory/src/lib.rs b/cache/in-memory/src/lib.rs index 9ef102ad5e8..0b049310aa1 100644 --- a/cache/in-memory/src/lib.rs +++ b/cache/in-memory/src/lib.rs @@ -930,10 +930,10 @@ impl UpdateCache for Event { match self { BanAdd(_) => {} BanRemove(_) => {} - ChannelCreate(v) => c.update(v), - ChannelDelete(v) => c.update(v), + ChannelCreate(v) => c.update(v.deref()), + ChannelDelete(v) => c.update(v.deref()), ChannelPinsUpdate(v) => c.update(v), - ChannelUpdate(v) => c.update(v), + ChannelUpdate(v) => c.update(v.deref()), GatewayHeartbeat(_) => {} GatewayHeartbeatAck => {} GatewayHello(_) => {} @@ -941,7 +941,7 @@ impl UpdateCache for Event { GatewayReconnect => {} GiftCodeUpdate => {} GuildCreate(v) => c.update(v.deref()), - GuildDelete(v) => c.update(v.deref()), + GuildDelete(v) => c.update(v), GuildEmojisUpdate(v) => c.update(v), GuildStickersUpdate(v) => c.update(v), GuildIntegrationsUpdate(_) => {} @@ -949,7 +949,7 @@ impl UpdateCache for Event { IntegrationCreate(v) => c.update(v.deref()), IntegrationDelete(v) => c.update(v.deref()), IntegrationUpdate(v) => c.update(v.deref()), - InteractionCreate(v) => c.update(v.deref()), + InteractionCreate(v) => c.update(v), InviteCreate(_) => {} InviteDelete(_) => {} MemberAdd(v) => c.update(v.deref()), @@ -981,8 +981,8 @@ impl UpdateCache for Event { StageInstanceCreate(v) => c.update(v), StageInstanceDelete(v) => c.update(v), StageInstanceUpdate(v) => c.update(v), - ThreadCreate(v) => c.update(v), - ThreadUpdate(v) => c.update(v), + ThreadCreate(v) => c.update(v.deref()), + ThreadUpdate(v) => c.update(v.deref()), ThreadDelete(v) => c.update(v), ThreadListSync(v) => c.update(v), ThreadMemberUpdate(_) => {} diff --git a/model/src/gateway/event/dispatch.rs b/model/src/gateway/event/dispatch.rs index d640f865266..39e24292df3 100644 --- a/model/src/gateway/event/dispatch.rs +++ b/model/src/gateway/event/dispatch.rs @@ -16,20 +16,20 @@ use serde::{ pub enum DispatchEvent { BanAdd(BanAdd), BanRemove(BanRemove), - ChannelCreate(ChannelCreate), - ChannelDelete(ChannelDelete), + ChannelCreate(Box), + ChannelDelete(Box), ChannelPinsUpdate(ChannelPinsUpdate), - ChannelUpdate(ChannelUpdate), + ChannelUpdate(Box), GiftCodeUpdate, GuildCreate(Box), - GuildDelete(Box), + GuildDelete(GuildDelete), GuildEmojisUpdate(GuildEmojisUpdate), GuildIntegrationsUpdate(GuildIntegrationsUpdate), GuildUpdate(Box), IntegrationCreate(Box), IntegrationDelete(IntegrationDelete), IntegrationUpdate(Box), - InteractionCreate(Box), + InteractionCreate(InteractionCreate), InviteCreate(Box), InviteDelete(InviteDelete), MemberAdd(Box), @@ -54,12 +54,12 @@ pub enum DispatchEvent { StageInstanceCreate(StageInstanceCreate), StageInstanceDelete(StageInstanceDelete), StageInstanceUpdate(StageInstanceUpdate), - ThreadCreate(ThreadCreate), + ThreadCreate(Box), ThreadDelete(ThreadDelete), ThreadListSync(ThreadListSync), - ThreadMemberUpdate(ThreadMemberUpdate), + ThreadMemberUpdate(Box), ThreadMembersUpdate(ThreadMembersUpdate), - ThreadUpdate(ThreadUpdate), + ThreadUpdate(Box), TypingStart(Box), UnavailableGuild(UnavailableGuild), UserUpdate(UserUpdate), @@ -211,16 +211,16 @@ impl<'de, 'a> DeserializeSeed<'de> for DispatchEventWithTypeDeserializer<'a> { fn deserialize>(self, deserializer: D) -> Result { Ok(match self.0 { "CHANNEL_CREATE" => { - DispatchEvent::ChannelCreate(ChannelCreate::deserialize(deserializer)?) + DispatchEvent::ChannelCreate(Box::new(ChannelCreate::deserialize(deserializer)?)) } "CHANNEL_DELETE" => { - DispatchEvent::ChannelDelete(ChannelDelete::deserialize(deserializer)?) + DispatchEvent::ChannelDelete(Box::new(ChannelDelete::deserialize(deserializer)?)) } "CHANNEL_PINS_UPDATE" => { DispatchEvent::ChannelPinsUpdate(ChannelPinsUpdate::deserialize(deserializer)?) } "CHANNEL_UPDATE" => { - DispatchEvent::ChannelUpdate(ChannelUpdate::deserialize(deserializer)?) + DispatchEvent::ChannelUpdate(Box::new(ChannelUpdate::deserialize(deserializer)?)) } "GIFT_CODE_UPDATE" => { deserializer.deserialize_ignored_any(IgnoredAny)?; @@ -232,9 +232,7 @@ impl<'de, 'a> DeserializeSeed<'de> for DispatchEventWithTypeDeserializer<'a> { "GUILD_CREATE" => { DispatchEvent::GuildCreate(Box::new(GuildCreate::deserialize(deserializer)?)) } - "GUILD_DELETE" => { - DispatchEvent::GuildDelete(Box::new(GuildDelete::deserialize(deserializer)?)) - } + "GUILD_DELETE" => DispatchEvent::GuildDelete(GuildDelete::deserialize(deserializer)?), "GUILD_EMOJIS_UPDATE" => { DispatchEvent::GuildEmojisUpdate(GuildEmojisUpdate::deserialize(deserializer)?) } @@ -274,9 +272,9 @@ impl<'de, 'a> DeserializeSeed<'de> for DispatchEventWithTypeDeserializer<'a> { "INTEGRATION_UPDATE" => DispatchEvent::IntegrationUpdate(Box::new( IntegrationUpdate::deserialize(deserializer)?, )), - "INTERACTION_CREATE" => DispatchEvent::InteractionCreate(Box::new( - InteractionCreate::deserialize(deserializer)?, - )), + "INTERACTION_CREATE" => { + DispatchEvent::InteractionCreate(InteractionCreate::deserialize(deserializer)?) + } "INVITE_CREATE" => { DispatchEvent::InviteCreate(Box::new(InviteCreate::deserialize(deserializer)?)) } @@ -331,7 +329,7 @@ impl<'de, 'a> DeserializeSeed<'de> for DispatchEventWithTypeDeserializer<'a> { DispatchEvent::StageInstanceUpdate(StageInstanceUpdate::deserialize(deserializer)?) } "THREAD_CREATE" => { - DispatchEvent::ThreadCreate(ThreadCreate::deserialize(deserializer)?) + DispatchEvent::ThreadCreate(Box::new(ThreadCreate::deserialize(deserializer)?)) } "THREAD_DELETE" => { DispatchEvent::ThreadDelete(ThreadDelete::deserialize(deserializer)?) @@ -339,14 +337,14 @@ impl<'de, 'a> DeserializeSeed<'de> for DispatchEventWithTypeDeserializer<'a> { "THREAD_LIST_SYNC" => { DispatchEvent::ThreadListSync(ThreadListSync::deserialize(deserializer)?) } - "THREAD_MEMBER_UPDATE" => { - DispatchEvent::ThreadMemberUpdate(ThreadMemberUpdate::deserialize(deserializer)?) - } + "THREAD_MEMBER_UPDATE" => DispatchEvent::ThreadMemberUpdate(Box::new( + ThreadMemberUpdate::deserialize(deserializer)?, + )), "THREAD_MEMBERS_UPDATE" => { DispatchEvent::ThreadMembersUpdate(ThreadMembersUpdate::deserialize(deserializer)?) } "THREAD_UPDATE" => { - DispatchEvent::ThreadUpdate(ThreadUpdate::deserialize(deserializer)?) + DispatchEvent::ThreadUpdate(Box::new(ThreadUpdate::deserialize(deserializer)?)) } "TYPING_START" => { DispatchEvent::TypingStart(Box::new(TypingStart::deserialize(deserializer)?)) diff --git a/model/src/gateway/event/mod.rs b/model/src/gateway/event/mod.rs index e2ff84888be..6b12a74e0e2 100644 --- a/model/src/gateway/event/mod.rs +++ b/model/src/gateway/event/mod.rs @@ -28,13 +28,13 @@ pub enum Event { /// A user's ban from a guild was removed. BanRemove(BanRemove), /// A channel was created. - ChannelCreate(ChannelCreate), + ChannelCreate(Box), /// A channel was deleted. - ChannelDelete(ChannelDelete), + ChannelDelete(Box), /// A channel's pins were updated. ChannelPinsUpdate(ChannelPinsUpdate), /// A channel was updated. - ChannelUpdate(ChannelUpdate), + ChannelUpdate(Box), /// A heartbeat was sent to or received from the gateway. GatewayHeartbeat(u64), /// A heartbeat acknowledgement was received from the gateway. @@ -52,7 +52,7 @@ pub enum Event { /// A guild was created. GuildCreate(Box), /// A guild was deleted or the current user was removed from a guild. - GuildDelete(Box), + GuildDelete(GuildDelete), /// A guild's emojis were updated. GuildEmojisUpdate(GuildEmojisUpdate), /// A guild's integrations were updated. @@ -68,7 +68,7 @@ pub enum Event { /// A guild integration was deleted. IntegrationUpdate(Box), /// An interaction was invoked by a user. - InteractionCreate(Box), + InteractionCreate(InteractionCreate), /// A invite was made. InviteCreate(Box), /// A invite was deleted. @@ -139,17 +139,17 @@ pub enum Event { StageInstanceUpdate(StageInstanceUpdate), /// A thread has been created, relevant to the current user, /// or the current user has been added to a thread. - ThreadCreate(ThreadCreate), + ThreadCreate(Box), /// A thread, relevant to the current user, has been deleted. ThreadDelete(ThreadDelete), /// The current user has gained access to a thread. ThreadListSync(ThreadListSync), /// The thread member object for the current user has been updated. - ThreadMemberUpdate(ThreadMemberUpdate), + ThreadMemberUpdate(Box), /// A user has been added to or removed from a thread. ThreadMembersUpdate(ThreadMembersUpdate), /// A thread has been updated. - ThreadUpdate(ThreadUpdate), + ThreadUpdate(Box), /// A user started typing in a channel. TypingStart(Box), /// A guild is now unavailable. @@ -236,9 +236,9 @@ impl Event { } } -impl From> for Event { - fn from(event: Box) -> Self { - match *event { +impl From for Event { + fn from(event: DispatchEvent) -> Self { + match event { DispatchEvent::BanAdd(v) => Self::BanAdd(v), DispatchEvent::BanRemove(v) => Self::BanRemove(v), DispatchEvent::ChannelCreate(v) => Self::ChannelCreate(v), @@ -298,7 +298,7 @@ impl From> for Event { impl From for Event { fn from(event: GatewayEvent) -> Self { match event { - GatewayEvent::Dispatch(_, e) => Self::from(e), + GatewayEvent::Dispatch(_, e) => Self::from(*e), GatewayEvent::Heartbeat(interval) => Self::GatewayHeartbeat(interval), GatewayEvent::HeartbeatAck => Self::GatewayHeartbeatAck, GatewayEvent::Hello(interval) => Self::GatewayHello(interval), @@ -352,3 +352,90 @@ impl Display for EventConversionError { } impl Error for EventConversionError {} + +#[cfg(test)] +mod tests { + //! `EVENT_THRESHOLD` is equivalent to 192 bytes. This was decided based on + //! the size of `Event` at the time of writing. The assertions here are to + //! ensure that in the case the events themselves grow or shrink past the + //! threshold, they are properly boxed or unboxed respectively. + //! + //! If a field has been added to an event in the "unboxed" section and its + //! assertion now fails, then you will need to wrap the event in a box in + //! the `Event` type and move the assertion to the "boxed" section. + //! + //! Likewise, if a field has been removed from an event in the "boxed" + //! section and the assertion now fails, you will need to remove the box + //! wrapping the event in the `Event` type and move the assertion to the + //! "unboxed" section. + + use super::{super::payload::incoming::*, shard::*, Event}; + use static_assertions::const_assert; + use std::mem; + + // `dead_code`: `const_assert` operates at the compiler level, and the lint + // requires a variable to be used in a function, so this is a false + // positive. + #[allow(dead_code)] + const EVENT_THRESHOLD: usize = 192; + + const_assert!(mem::size_of::() == EVENT_THRESHOLD); + + // Boxed events. + const_assert!(mem::size_of::() > EVENT_THRESHOLD); + const_assert!(mem::size_of::() > EVENT_THRESHOLD); + const_assert!(mem::size_of::() > EVENT_THRESHOLD); + const_assert!(mem::size_of::() > EVENT_THRESHOLD); + const_assert!(mem::size_of::() > EVENT_THRESHOLD); + const_assert!(mem::size_of::() > EVENT_THRESHOLD); + const_assert!(mem::size_of::() > EVENT_THRESHOLD); + const_assert!(mem::size_of::() > EVENT_THRESHOLD); + const_assert!(mem::size_of::() > EVENT_THRESHOLD); + const_assert!(mem::size_of::() > EVENT_THRESHOLD); + const_assert!(mem::size_of::() > EVENT_THRESHOLD); + const_assert!(mem::size_of::() > EVENT_THRESHOLD); + const_assert!(mem::size_of::() > EVENT_THRESHOLD); + const_assert!(mem::size_of::() > EVENT_THRESHOLD); + const_assert!(mem::size_of::() > EVENT_THRESHOLD); + const_assert!(mem::size_of::() > EVENT_THRESHOLD); + const_assert!(mem::size_of::() > EVENT_THRESHOLD); + const_assert!(mem::size_of::() > EVENT_THRESHOLD); + const_assert!(mem::size_of::() > EVENT_THRESHOLD); + const_assert!(mem::size_of::() > EVENT_THRESHOLD); + + // Unboxed. + const_assert!(mem::size_of::() <= EVENT_THRESHOLD); + const_assert!(mem::size_of::() <= EVENT_THRESHOLD); + const_assert!(mem::size_of::() <= EVENT_THRESHOLD); + const_assert!(mem::size_of::() <= EVENT_THRESHOLD); + const_assert!(mem::size_of::() <= EVENT_THRESHOLD); + const_assert!(mem::size_of::() <= EVENT_THRESHOLD); + const_assert!(mem::size_of::() <= EVENT_THRESHOLD); + const_assert!(mem::size_of::() <= EVENT_THRESHOLD); + const_assert!(mem::size_of::() <= EVENT_THRESHOLD); + const_assert!(mem::size_of::() <= EVENT_THRESHOLD); + const_assert!(mem::size_of::() <= EVENT_THRESHOLD); + const_assert!(mem::size_of::() <= EVENT_THRESHOLD); + const_assert!(mem::size_of::() <= EVENT_THRESHOLD); + const_assert!(mem::size_of::() <= EVENT_THRESHOLD); + const_assert!(mem::size_of::() <= EVENT_THRESHOLD); + const_assert!(mem::size_of::() <= EVENT_THRESHOLD); + const_assert!(mem::size_of::() <= EVENT_THRESHOLD); + const_assert!(mem::size_of::() <= EVENT_THRESHOLD); + const_assert!(mem::size_of::() <= EVENT_THRESHOLD); + const_assert!(mem::size_of::() <= EVENT_THRESHOLD); + const_assert!(mem::size_of::() <= EVENT_THRESHOLD); + const_assert!(mem::size_of::() <= EVENT_THRESHOLD); + const_assert!(mem::size_of::() <= EVENT_THRESHOLD); + const_assert!(mem::size_of::() <= EVENT_THRESHOLD); + const_assert!(mem::size_of::() <= EVENT_THRESHOLD); + const_assert!(mem::size_of::() <= EVENT_THRESHOLD); + const_assert!(mem::size_of::() <= EVENT_THRESHOLD); + const_assert!(mem::size_of::() <= EVENT_THRESHOLD); + const_assert!(mem::size_of::() <= EVENT_THRESHOLD); + const_assert!(mem::size_of::() <= EVENT_THRESHOLD); + const_assert!(mem::size_of::() <= EVENT_THRESHOLD); + const_assert!(mem::size_of::() <= EVENT_THRESHOLD); + const_assert!(mem::size_of::() <= EVENT_THRESHOLD); + const_assert!(mem::size_of::() <= EVENT_THRESHOLD); +} diff --git a/standby/src/lib.rs b/standby/src/lib.rs index 32d1c604279..beb65da6cf3 100644 --- a/standby/src/lib.rs +++ b/standby/src/lib.rs @@ -1606,8 +1606,8 @@ mod tests { /// the matching of a later event. #[tokio::test] async fn test_wait_for_component() { - let event = Event::InteractionCreate(Box::new(InteractionCreate( - Interaction::MessageComponent(Box::new(button())), + let event = Event::InteractionCreate(InteractionCreate(Interaction::MessageComponent( + Box::new(button()), ))); let standby = Standby::new(); @@ -1630,20 +1630,20 @@ mod tests { let standby = Standby::new(); let mut stream = standby.wait_for_component_stream(Id::new(3), |_: &MessageComponentInteraction| true); - standby.process(&Event::InteractionCreate(Box::new(InteractionCreate( + standby.process(&Event::InteractionCreate(InteractionCreate( Interaction::MessageComponent(Box::new(button())), - )))); - standby.process(&Event::InteractionCreate(Box::new(InteractionCreate( + ))); + standby.process(&Event::InteractionCreate(InteractionCreate( Interaction::MessageComponent(Box::new(button())), - )))); + ))); assert!(stream.next().await.is_some()); assert!(stream.next().await.is_some()); drop(stream); assert_eq!(1, standby.components.len()); - standby.process(&Event::InteractionCreate(Box::new(InteractionCreate( + standby.process(&Event::InteractionCreate(InteractionCreate( Interaction::MessageComponent(Box::new(button())), - )))); + ))); assert!(standby.components.is_empty()); }