Skip to content

Conversation

@frankmcsherry
Copy link
Member

This PR separates Bytes into Bytes and BytesMut. The former implements clone() and is unwriteable. The latter does not implement clone but is writeable. One goes from the latter to the former with the freeze() method.

The intent is to minimize the use of BytesMut, which is narrowed only to moments within the writing to a BytesSlab. As soon as the writing completes it yields a Bytes which is from that point on unwriteable.

One of the goals is to make serialized messages cloneable, so that broadcast channels can more efficiently be implemented.

@frankmcsherry
Copy link
Member Author

The changes to bytes/src/lib.rs are a bit silly to read as a diff. As the functionality was split between the two, the diff inserts a type definition halfway through an impl block. It can also be read as "copy Bytes to BytesMut; make Bytes.ptr a const ptr; delete methods that do not get used".

Copy link
Member

@antiguru antiguru left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this looks good. I left some comments inline, but the overall structure makes sense to me.

You might want to add #[inline] annotations to simple functions (freeze, deref*) so we don't have to go back in the future should we notice that they're not inlined. As they don't have generic parameters, without LTO, they wouldn't be inlined.

@frankmcsherry frankmcsherry merged commit 38ef688 into TimelyDataflow:master Feb 11, 2025
7 checks passed
@frankmcsherry frankmcsherry deleted the bytes_update branch February 11, 2025 09:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants