less allocate and memcpy in serialization #6
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: qupeng <[email protected]>
Signed-off-by: qupeng <[email protected]>
nrc
reviewed
Oct 20, 2020
protobuf/src/core.rs
Outdated
| /// Write the message to bytes vec. | ||
| fn write_to_vec(&self, v: &mut Vec<u8>) -> ProtobufResult<()> { | ||
| let size = self.compute_size() as usize; | ||
| if v.capacity() - v.len() < size { | ||
| v.reserve(size - v.len()); |
There was a problem hiding this comment.
I think something is off here. Does this function end up writing to the start of v or to v.len() - 1? It seems that line 115 assumes the latter and line 116 assumes the former.
| @@ -56,12 +56,7 @@ pub trait Message: fmt::Debug + Clear + Any + Send + Sync { | |||
| /// Results in error if message is not fully initialized. | |||
| fn write_to(&self, os: &mut CodedOutputStream) -> ProtobufResult<()> { | |||
| self.check_initialized()?; | |||
|
|
|||
| // cache sizes | |||
| self.compute_size(); | |||
There was a problem hiding this comment.
No indeed. the cached internal size is not used later.
Signed-off-by: qupeng <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
By default there is an internal buffer in
CodedOutputStream, which is always used when serializing aMessage. It's useful when the output stream is a disk file or pipe. However if the output is an another memory buffer, redundantmemcpycalls are involved.This PR add a new method
write_to_writer_without_buffer, which can avoid such buffer mechanism.