Skip to content
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

chunk_mut should not allocate #518

Closed
TwoClocks opened this issue Dec 12, 2021 · 5 comments
Closed

chunk_mut should not allocate #518

TwoClocks opened this issue Dec 12, 2021 · 5 comments

Comments

@TwoClocks
Copy link

I was surprised to discover chunk_mut doing an allocation if the buffer is full. I would think it would return a 0 length slice.

Is this intended behaviour, or an oversight? If intended why only allocate 64 ? why not os.page_size, or 64K. It seems kinda arbitrary.

@Darksonn
Copy link
Contributor

This is intended behavior. As for the size of the allocation, the new capacity will actually be something like max(old_capacity + 64, 2*old_capacity) since reserve allocates more if the buffer is already large.

@TwoClocks
Copy link
Author

huh.
It's easy enough to avoid, so ok.

Coming from other languages I had assumed a "zero-copy" network buffer library would also imply "zero non-explicit allocations" as those are a few orders of magnitude more expensive. Or at least have it as an option. Working on embedded / latency-sensitive systems was a bit of a shock to find allocations happening "under the hood". It's also doesn't seem to be documented any place that this happens. Should I be concerned about other places doing allocations that aren't generally known? Am I going to discover them via valgrind?
Or perhaps this is the wrong library to use for that kind of work and not your intended use case?

@Darksonn
Copy link
Contributor

Darksonn commented Dec 12, 2021

All of the uses of BytesMut that I know of would need to insert an explicit call to reallocate-if-out-of-capacity before calling chunk_mut if we didn't do it automatically. That's why it's done in this way. It should be considered like an Vec<u8>.

It is documented on the doc page for BytesMut that it will reallocate if it runs out of space.

Growth

BytesMut’s BufMut implementation will implicitly grow its buffer as necessary. However, explicitly reserving the required space up-front before a series of inserts will be more efficient.

Of course, we can add it to the BufMut trait method too.

@Darksonn
Copy link
Contributor

To elaborate a bit more: The chunk_mut method is something you're only going to call if you need to append to the buffer. Would it surprise you if Vec::push reallocates if you have used up the capacity?

@TwoClocks
Copy link
Author

TwoClocks commented Dec 12, 2021

Would it surprise you if Vec::push reallocates if you have used up the capacity?

No, because that's the common way they work. It's also very well documented. Same as calling reserve on a BufMut. no supprises. Calling the "get a slice to the un-used potion at the end of the buffer" API and having it allocated is a surprise.

Would you be surprised if this relocated?

    let mut buf: Vec<u8> = Vec::with_capacity(16);
    let l = buf.len();
    let s = &mut buf[l..];

Yes. That would be very surprising.

Edit : I think the way it is is fine, just surprising, I'm not trying to advocate for something else. However, every network/buffering library I've ever used in every language has a "no grow" option (or it's the only option). Where put can fail. If you are going to change anything, it'd be nice to have a version of the API that never allocates. Handy for embedded or latency work. Better ergonomics than tiptoeing around a "growable" api.

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

No branches or pull requests

2 participants