ayrat555 / frankenstein

Telegram bot API client for Rust
Do What The F*ck You Want To Public License
268 stars 28 forks source link

make some fields an enum instead of a struct #62

Closed johannesvollmer closed 2 years ago

johannesvollmer commented 2 years ago

mem::size_of::<Update>() went from 30,384 to 5,384 bytes

Size of related objects

mem::size_of::<Update>() = 5,384
mem::size_of::<UpdateContent>() = 5,376
mem::size_of::<CallbackQuery>() = 5,368
mem::size_of::<Message>() = 5,136
mem::size_of::<ChatMemberUpdated>() = 1,064
mem::size_of::<ChatMemberUpdated>() = 1,064
mem::size_of::<ChatJoinRequest>() = 768
mem::size_of::<PreCheckoutQuery>() = 440
mem::size_of::<Chat>() = 424
mem::size_of::<ShippingQuery>() = 304
mem::size_of::<InlineQuery>() = 264
mem::size_of::<ChosenInlineResult>() = 240
mem::size_of::<Poll>() = 160
mem::size_of::<PollAnswer>() = 160
mem::size_of::<User>() = 112

It is therefore advisable to reduce the size of the biggest structs that can occur inside an UpdateContent object, most importantly CallbackQuery and Message. The CallbackQuery contains a few small fields, except for one large Message. Therefore, we only need to optimize Message to reduce the total size of every Update.

johannesvollmer commented 2 years ago

What are the situations where using a builder is better than simply saying Update { id: 232323, .. Update::default() }? Only when default cannot have a nice default value for all the fields?

EdJoPaTo commented 2 years ago

The builder is useful for stuff like SendMessageParams. The Update is never user generated. (Maybe in tests?)

Also Update::default() wouldn't work as there is no derive(Default) and it wouldn't even make sense as the id is unique and the update contains exactly one content thingy? The builder does handle fields with or without defaults and can check for required fields.

So yeah… I don't see a usecase for the Builder for the Update as every field is required anyway and the user wont be interested in constructing Updates.

johannesvollmer commented 2 years ago

Yeah, makes sense. For the sake of compilation speed, we should remove it.

ayrat555 commented 2 years ago

released in 0.14.0