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.

Sign up for GitHub

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

Jump to bottom

Remove redundant reserve call #674

Merged
merged 4 commits into from
Mar 4, 2024
+22 −2
Merged

Remove redundant reserve call #674

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
869c3b4
Remove redundant reserve call.
braddunbar Mar 2, 2024
0211037
remove inline-able todo
braddunbar Mar 2, 2024
c040374
Merge branch 'master' of https://github.com/tokio-rs/bytes into redun…
braddunbar Mar 2, 2024
76dae7b
add a test for extending past lower limit
braddunbar Mar 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions src/bytes_mut.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1283,9 +1283,7 @@ impl Extend<u8> for BytesMut {

// TODO: optimize
// 1. If self.kind() == KIND_VEC, use Vec::extend
// 2. Make `reserve` inline-able
Copy link
Member

@taiki-e taiki-e Mar 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I'm not sure what this TODO comment originally meant, but if it means something like "reserve (in here or put_u8) should be able to omit if the compiler knows the actual length of iter" it may not have been resolved yet.

(In my experience, the compiler optimizations around the reserve have not worked that well.)

EDIT: but seanmonstar who wrote this TODO comment approved this PR, so this is probably not the meaning I have imagined here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @taiki-e! I think it refers to #313, which was just after the introduction of the comment in #305. I should've mentioned that in my commit though. Thanks for taking a look!

for b in iter {
self.reserve(1);
self.put_u8(b);
}
}
Expand Down
Loading