facebook/sapling
1
1
Fork 0
mirror of https://github.com/facebook/sapling.git synced 2024-10-09 16:31:02 +03:00
Code Issues Projects Releases Wiki Activity

rust: add compact_protocol::deserialize Bytes thrift entry point

Browse Source 
Summary:
Although thrift compact_protocol had support for Bytes, the default deserializer entry point was forcing use of Cursor<&[u8]> which meant we weren't seeing the expected improvement in memory usage for code like Mononoke's packblob deserialization from Bytes.

This change updates compact_protocol::deserialize in terms of a new type DeserializeSource. DeserializeSource has conversions that it can take ownership of Bytes and items that are Into<Bytes>, whilst still being able to copy data from types that are AsRef<[u8]>.

Due to lack of negative bounds or specialization, this was a little verbose:

   - Bytes is AsRef<&[u8]>, hence the need to define the ref type conversions in bufext explicitly with impl_deser_as_ref_u8!()

   - From<&Bytes> is not defined upsteam, which results in needing to declare Into<Bytes> types individually with impl_deser_into_bytes so that From<&Bytes> can be handled ( otherwise one gets the "upstream crates may add a new impl of trait error" )

As the above was a bit fiddly I initially had a new compact_protocol::deserialize_from_bytes() entry point, but that meant callsites already passing Bytes, &Bytes or Vec<u8> needed updating to benefit from the copy removal, which seemed sub-optimal.

There are of course some callsites where they have a Vec<u8> and could pass ownership but don't. They will need updating to see a benefit. (e.g. compact_protocol::deserialize(&v) to compact_protocol::deserialize(v) ).  That is not included in this change, I'll take a look at Mononoke's in a separate diff.

Reviewed By: markbt

Differential Revision: D26663804

fbshipit-source-id: 9a81f3c1dbef9d1b245492212afe7c2a4baff942
This commit is contained in:
Alex Hornby 2021-02-27 04:22:01 -08:00 committed by Facebook GitHub Bot
parent 400f8ecbff
commit 3e0c5df020
1 changed files with 2 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
eden/mononoke/blobstore/packblob/src/envelope.rs
View File

@ -76,7 +76,7 @@ impl TryFrom<BlobstoreBytes> for PackEnvelope {
let mut bytes = bytes.into_bytes();
let header: HeaderType = HeaderType::try_from(bytes.get_u32())?;
let t: StorageEnvelope = match header {
HeaderType::PackBlobCompactFormat => compact_protocol::deserialize(bytes)?,
HeaderType::PackBlobCompactFormat => compact_protocol::deserialize(&bytes)?,
};
Ok(PackEnvelope(t))
}
@ -117,7 +117,7 @@ mod tests {
let mut bytes = compact_serialize_with_header(magic, value.clone());
let header = bytes.get_u32();
assert_eq!(magic, header);
let roundtripped: String = compact_protocol::deserialize(&bytes)?;
let roundtripped: String = compact_protocol::deserialize(bytes)?;
assert_eq!(value, roundtripped);
Ok(())
}