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

BytesMut::freeze is not zero-cost #401

Open
Tracked by #2
Jasper-Bekkers opened this issue Jun 28, 2020 · 6 comments · May be fixed by #435
Open
Tracked by #2

BytesMut::freeze is not zero-cost #401

Jasper-Bekkers opened this issue Jun 28, 2020 · 6 comments · May be fixed by #435

Comments

@Jasper-Bekkers
Copy link

If self.kind() == KIND_VEC is true, freeze ends up calling vec.into() which appears to not be zero-cost at all, and ends up doing a huge memcpy somewhere in the bowels of Vec

image

@Jasper-Bekkers
Copy link
Author

It looks like the original from_vec implementation didn't used to call into_boxed_slice before the refactoring in #298 https://github.com/tokio-rs/bytes/pull/298/files#diff-d272e47b868896a26bf5ddcfa9be0ea2L1836

@fanatid
Copy link
Contributor

fanatid commented Jul 12, 2020

@Jasper-Bekkers out of interest, how you get this trace? Is there a way do nearly same on Linux?

@Jasper-Bekkers
Copy link
Author

It's profiled with Superluminal Profiler; https://superluminal.eu I don't think it supports Linux yet.

@Matthias247
Copy link
Contributor

By looking at it this comes from calling Vec::into_boxed_slice(), which calls Vec::shrink_to_fit().

That again means freeze is not zero cost if the BytesMut wasn't written up to it's capacity. If you reserve the exact amount upfront, it might not be an issue. If you split off anything upfront (which promotes to an Arc), then it also wouldn't be an issue.

This is all fixable. However a tricky question is whether people are all ok with unused memory in the BytesMut being "lost" while Bytes elements refer to a subset of it. For myself - I would be - given that I don't see Bytes as an ideal long term storage anyway and everyone should strive for releasing the chunks as soon as no longer required to avoid references keeping bigger buffers alive for longer than necessary.

@Matthias247
Copy link
Contributor

After thinking a bit more about it: I think we can solve this easily by adding an additional shrink_and_freeze() method for people which care about potential waste of memory, and make sure freeze() is always allocation free.

@Jasper-Bekkers
Copy link
Author

After thinking a bit more about it: I think we can solve this easily by adding an additional shrink_and_freeze() method for people which care about potential waste of memory, and make sure freeze() is always allocation free.

Did this ever end up getting implemented, and if so, should we close this issue? ☺️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants