-
Notifications
You must be signed in to change notification settings - Fork 19
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Stream/General improvements #1041
Conversation
914f2d9
to
beebf2f
Compare
let client = self.clone(); | ||
|
||
let group_id_to_info = group_id_to_info.clone(); | ||
let group_info = group_id_to_info.clone(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Previously, this would clone the entire HashMap<>
(list of groups) for every single group message, where the key is a Vec<u8>
. now we just clone the Arc
.
pub async fn stream<'a, ApiClient>( | ||
&'a self, | ||
client: &'a Client<ApiClient>, | ||
) -> Result<impl Stream<Item = StoredGroupMessage> + '_, GroupError> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This lifetime capture (+ '_
) becomes the default behavior in rust 2024 edition + allows us to attach more flexible lifetime captures wherever we use impl Trait
, which will allow eliding the 'a
lifetime here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a huge set of improvements
Non-wasm specific improvements pulled from work from wasm:
improve streams. Since Rust 1.75
async fn
in trait is supported, along with a much lighter replacement toasync_trait
.Pin<Box<dyn Stream>>
async-trait
from everywhere except a trait inxmtp_id
which requires dynamic dispatchasync-trait
should also generally improve compile timesclones()
and use references insteadremoves some unnecessary dependencies
flume
, used in one placesmart-default
used in once placeremoves an expensive clone we were doing in
stream_all_messages
preferring anArc<>
clone insteadreplaces
ethers_core
withethers::core
removes a random
unsafe
code block inxmtp_id
unifies
trait InboxOwner
(uses one in xmtp_id, deletes elsewhere)