From 0574e6a58c24a95d6f0bf9dc7cea88cbf944d15c Mon Sep 17 00:00:00 2001 From: victor koenders Date: Sat, 23 Sep 2023 15:55:14 +0200 Subject: [PATCH 1/4] Added unty dependency and added type checks --- Cargo.toml | 139 +++++++++++++++++++------------------ src/de/impls.rs | 84 +++++++++++----------- src/enc/impls.rs | 5 +- src/features/impl_alloc.rs | 63 ++++++++--------- 4 files changed, 145 insertions(+), 146 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 52448d7f..369f579a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,69 +1,70 @@ -[workspace] -members = ["derive", "compatibility"] - -[package] -name = "bincode" -version = "2.0.0-rc.3" # remember to update html_root_url and bincode_derive -authors = [ - "Ty Overby ", - "Zoey Riordan ", - "Victor Koenders ", -] -exclude = ["logo.svg", "examples/*", ".gitignore", ".github/"] - -publish = true - -repository = "https://github.com/bincode-org/bincode" -documentation = "https://docs.rs/bincode" -readme = "./readme.md" -categories = ["encoding", "network-programming"] -keywords = ["binary", "encode", "decode", "serialize", "deserialize"] - -license = "MIT" -description = "A binary serialization / deserialization strategy for transforming structs into bytes and vice versa!" - -edition = "2021" - -[features] -default = ["std", "derive"] -std = ["alloc", "serde?/std"] -alloc = ["serde?/alloc"] -derive = ["bincode_derive"] - -[dependencies] -bincode_derive = { path = "derive", version = "2.0.0-rc.3", optional = true } -serde = { version = "1.0", default-features = false, optional = true } - -# Used for tests -[dev-dependencies] -serde_derive = "1.0" -serde_json = { version = "1.0", default-features = false } -tempfile = "3.2" -criterion = "0.5" -rand = "0.8" -uuid = { version = "1.1", features = ["serde"] } -chrono = { version = "0.4", features = ["serde"] } -glam = { version = "0.24", features = ["serde"] } -bincode_1 = { version = "1.3", package = "bincode" } -serde = { version = "1.0", features = ["derive"] } - -[[bench]] -name = "varint" -harness = false - -[[bench]] -name = "inline" -harness = false - -[[bench]] -name = "string" -harness = false - -[profile.bench] -codegen-units = 1 -debug = 1 -lto = true - -[package.metadata.docs.rs] -all-features = true -rustdoc-args = ["--cfg", "docsrs"] +[workspace] +members = ["derive", "compatibility"] + +[package] +name = "bincode" +version = "2.0.0-rc.3" # remember to update html_root_url and bincode_derive +authors = [ + "Ty Overby ", + "Zoey Riordan ", + "Victor Koenders ", +] +exclude = ["logo.svg", "examples/*", ".gitignore", ".github/"] + +publish = true + +repository = "https://github.com/bincode-org/bincode" +documentation = "https://docs.rs/bincode" +readme = "./readme.md" +categories = ["encoding", "network-programming"] +keywords = ["binary", "encode", "decode", "serialize", "deserialize"] + +license = "MIT" +description = "A binary serialization / deserialization strategy for transforming structs into bytes and vice versa!" + +edition = "2021" + +[features] +default = ["std", "derive"] +std = ["alloc", "serde?/std"] +alloc = ["serde?/alloc"] +derive = ["bincode_derive"] + +[dependencies] +bincode_derive = { path = "derive", version = "2.0.0-rc.3", optional = true } +serde = { version = "1.0", default-features = false, optional = true } +unty = "0.0.1" + +# Used for tests +[dev-dependencies] +serde_derive = "1.0" +serde_json = { version = "1.0", default-features = false } +tempfile = "3.2" +criterion = "0.5" +rand = "0.8" +uuid = { version = "1.1", features = ["serde"] } +chrono = { version = "0.4", features = ["serde"] } +glam = { version = "0.24", features = ["serde"] } +bincode_1 = { version = "1.3", package = "bincode" } +serde = { version = "1.0", features = ["derive"] } + +[[bench]] +name = "varint" +harness = false + +[[bench]] +name = "inline" +harness = false + +[[bench]] +name = "string" +harness = false + +[profile.bench] +codegen-units = 1 +debug = 1 +lto = true + +[package.metadata.docs.rs] +all-features = true +rustdoc-args = ["--cfg", "docsrs"] diff --git a/src/de/impls.rs b/src/de/impls.rs index 542f5ded..0846e8c2 100644 --- a/src/de/impls.rs +++ b/src/de/impls.rs @@ -446,28 +446,26 @@ where fn decode(decoder: &mut D) -> Result { decoder.claim_bytes_read(core::mem::size_of::<[T; N]>())?; - // TODO: we can't limit `T: 'static` because that would break other things - // but we want to have this optimization - // This will be another contendor for specialization implementation - // if TypeId::of::() == TypeId::of::() { - // let mut buf = [0u8; N]; - // decoder.reader().read(&mut buf)?; - // let ptr = &mut buf as *mut _ as *mut [T; N]; - - // // Safety: we know that T is a u8, so it is perfectly safe to - // // translate an array of u8 into an array of T - // let res = unsafe { ptr.read() }; - // Ok(res) - // } - let result = super::impl_core::collect_into_array(&mut (0..N).map(|_| { - // See the documentation on `unclaim_bytes_read` as to why we're doing this here - decoder.unclaim_bytes_read(core::mem::size_of::()); - T::decode(decoder) - })); - - // result is only None if N does not match the values of `(0..N)`, which it always should - // So this unwrap should never occur - result.unwrap() + if unty::type_equal::() { + let mut buf = [0u8; N]; + decoder.reader().read(&mut buf)?; + let ptr = &mut buf as *mut _ as *mut [T; N]; + + // Safety: we know that T is a u8, so it is perfectly safe to + // translate an array of u8 into an array of T + let res = unsafe { ptr.read() }; + Ok(res) + } else { + let result = super::impl_core::collect_into_array(&mut (0..N).map(|_| { + // See the documentation on `unclaim_bytes_read` as to why we're doing this here + decoder.unclaim_bytes_read(core::mem::size_of::()); + T::decode(decoder) + })); + + // result is only None if N does not match the values of `(0..N)`, which it always should + // So this unwrap should never occur + result.unwrap() + } } } @@ -478,28 +476,26 @@ where fn borrow_decode>(decoder: &mut D) -> Result { decoder.claim_bytes_read(core::mem::size_of::<[T; N]>())?; - // TODO: we can't limit `T: 'static` because that would break other things - // but we want to have this optimization - // This will be another contendor for specialization implementation - // if TypeId::of::() == TypeId::of::() { - // let mut buf = [0u8; N]; - // decoder.reader().read(&mut buf)?; - // let ptr = &mut buf as *mut _ as *mut [T; N]; - - // // Safety: we know that T is a u8, so it is perfectly safe to - // // translate an array of u8 into an array of T - // let res = unsafe { ptr.read() }; - // Ok(res) - // } - let result = super::impl_core::collect_into_array(&mut (0..N).map(|_| { - // See the documentation on `unclaim_bytes_read` as to why we're doing this here - decoder.unclaim_bytes_read(core::mem::size_of::()); - T::borrow_decode(decoder) - })); - - // result is only None if N does not match the values of `(0..N)`, which it always should - // So this unwrap should never occur - result.unwrap() + if unty::type_equal::() { + let mut buf = [0u8; N]; + decoder.reader().read(&mut buf)?; + let ptr = &mut buf as *mut _ as *mut [T; N]; + + // Safety: we know that T is a u8, so it is perfectly safe to + // translate an array of u8 into an array of T + let res = unsafe { ptr.read() }; + Ok(res) + } else { + let result = super::impl_core::collect_into_array(&mut (0..N).map(|_| { + // See the documentation on `unclaim_bytes_read` as to why we're doing this here + decoder.unclaim_bytes_read(core::mem::size_of::()); + T::borrow_decode(decoder) + })); + + // result is only None if N does not match the values of `(0..N)`, which it always should + // So this unwrap should never occur + result.unwrap() + } } } diff --git a/src/enc/impls.rs b/src/enc/impls.rs index 2609eb62..c0b00ae2 100644 --- a/src/enc/impls.rs +++ b/src/enc/impls.rs @@ -292,12 +292,13 @@ impl Encode for char { impl Encode for [T] where - T: Encode + 'static, + T: Encode, { fn encode(&self, encoder: &mut E) -> Result<(), EncodeError> { super::encode_slice_len(encoder, self.len())?; - if core::any::TypeId::of::() == core::any::TypeId::of::() { + if unty::type_equal::() { + // Safety: T = u8 let t: &[u8] = unsafe { core::mem::transmute(self) }; encoder.writer().write(t)?; return Ok(()); diff --git a/src/features/impl_alloc.rs b/src/features/impl_alloc.rs index 2bec6c4c..71225d31 100644 --- a/src/features/impl_alloc.rs +++ b/src/features/impl_alloc.rs @@ -1,5 +1,5 @@ use crate::{ - de::{BorrowDecoder, Decode, Decoder}, + de::{read::Reader, BorrowDecoder, Decode, Decoder}, enc::{ self, write::{SizeWriter, Writer}, @@ -8,8 +8,6 @@ use crate::{ error::{DecodeError, EncodeError}, impl_borrow_decode, BorrowDecode, Config, }; -#[cfg(target_has_atomic = "ptr")] -use alloc::sync::Arc; use alloc::{ borrow::{Cow, ToOwned}, boxed::Box, @@ -19,6 +17,9 @@ use alloc::{ vec::Vec, }; +#[cfg(target_has_atomic = "ptr")] +use alloc::sync::Arc; + #[derive(Default)] pub(crate) struct VecWriter { inner: Vec, @@ -283,28 +284,26 @@ where fn decode(decoder: &mut D) -> Result { let len = crate::de::decode_slice_len(decoder)?; - // TODO: we can't limit `T: 'static` because that would break other things - // but we want to have this optimization - // This will be another contendor for specialization implementation - // if core::any::TypeId::of::() == core::any::TypeId::of::() { - // decoder.claim_container_read::(len)?; - // // optimize for reading u8 vecs - // let mut vec = Vec::new(); - // vec.resize(len, 0u8); - // decoder.reader().read(&mut vec)?; - // // Safety: Vec is Vec - // return Ok(unsafe { core::mem::transmute(vec) }); - // } - decoder.claim_container_read::(len)?; - - let mut vec = Vec::with_capacity(len); - for _ in 0..len { - // See the documentation on `unclaim_bytes_read` as to why we're doing this here - decoder.unclaim_bytes_read(core::mem::size_of::()); - - vec.push(T::decode(decoder)?); + if unty::type_equal::() { + decoder.claim_container_read::(len)?; + // optimize for reading u8 vecs + let mut vec = Vec::new(); + vec.resize(len, 0u8); + decoder.reader().read(&mut vec)?; + // Safety: Vec is Vec + Ok(unsafe { core::mem::transmute(vec) }) + } else { + decoder.claim_container_read::(len)?; + + let mut vec = Vec::with_capacity(len); + for _ in 0..len { + // See the documentation on `unclaim_bytes_read` as to why we're doing this here + decoder.unclaim_bytes_read(core::mem::size_of::()); + + vec.push(T::decode(decoder)?); + } + Ok(vec) } - Ok(vec) } } @@ -329,19 +328,21 @@ where impl Encode for Vec where - T: Encode + 'static, + T: Encode, { fn encode(&self, encoder: &mut E) -> Result<(), EncodeError> { crate::enc::encode_slice_len(encoder, self.len())?; - if core::any::TypeId::of::() == core::any::TypeId::of::() { + if unty::type_equal::() { + // Safety: T == u8 let slice: &[u8] = unsafe { core::mem::transmute(self.as_slice()) }; encoder.writer().write(slice)?; - return Ok(()); + Ok(()) + } else { + for item in self.iter() { + item.encode(encoder)?; + } + Ok(()) } - for item in self.iter() { - item.encode(encoder)?; - } - Ok(()) } } From 37bd22d952aaf52e49e833096736960d624b5025 Mon Sep 17 00:00:00 2001 From: Victor Koenders Date: Sat, 23 Sep 2023 17:45:31 +0200 Subject: [PATCH 2/4] Bumped unty 0.0.2 --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 369f579a..066ec78b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -33,7 +33,7 @@ derive = ["bincode_derive"] [dependencies] bincode_derive = { path = "derive", version = "2.0.0-rc.3", optional = true } serde = { version = "1.0", default-features = false, optional = true } -unty = "0.0.1" +unty = "0.0.2" # Used for tests [dev-dependencies] From 22c5954760300aa99ab87277571290f7ea749da0 Mon Sep 17 00:00:00 2001 From: Victor Koenders Date: Sun, 24 Sep 2023 11:21:48 +0200 Subject: [PATCH 3/4] Bump unty to 0.0.3 --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 066ec78b..256e2c9a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -33,7 +33,7 @@ derive = ["bincode_derive"] [dependencies] bincode_derive = { path = "derive", version = "2.0.0-rc.3", optional = true } serde = { version = "1.0", default-features = false, optional = true } -unty = "0.0.2" +unty = "0.0.3" # Used for tests [dev-dependencies] From fcf6224a66c76937b49fa935f8549a982f4013bc Mon Sep 17 00:00:00 2001 From: Victor Koenders Date: Sun, 24 Sep 2023 14:21:10 +0200 Subject: [PATCH 4/4] Removed unneeded + Sized requirements Optimize encode for [T; N] Made BinaryHeap proxy to Vec Made VecDeque decode/borrowdecode proxy to Vec Optimize VecDeque::::Encode to write 2 slices directly Optimize Vec borrowdecode implementation --- src/de/impls.rs | 20 +------- src/enc/impls.rs | 23 ++++----- src/features/impl_alloc.rs | 96 ++++++++++++++++---------------------- 3 files changed, 51 insertions(+), 88 deletions(-) diff --git a/src/de/impls.rs b/src/de/impls.rs index 0846e8c2..202be036 100644 --- a/src/de/impls.rs +++ b/src/de/impls.rs @@ -441,7 +441,7 @@ impl<'a, 'de: 'a> BorrowDecode<'de> for &'a str { impl Decode for [T; N] where - T: Decode + Sized, + T: Decode, { fn decode(decoder: &mut D) -> Result { decoder.claim_bytes_read(core::mem::size_of::<[T; N]>())?; @@ -471,7 +471,7 @@ where impl<'de, T, const N: usize> BorrowDecode<'de> for [T; N] where - T: BorrowDecode<'de> + Sized, + T: BorrowDecode<'de>, { fn borrow_decode>(decoder: &mut D) -> Result { decoder.claim_bytes_read(core::mem::size_of::<[T; N]>())?; @@ -543,22 +543,6 @@ where } } -// BlockedTODO: https://github.com/rust-lang/rust/issues/37653 -// -// We'll want to implement BorrowDecode for both Option<&[u8]> and Option<&[T: Encode]>, -// but those implementations overlap because &'a [u8] also implements BorrowDecode -// impl<'a, 'de: 'a> BorrowDecode<'de> for Option<&'a [u8]> { -// fn borrow_decode>(decoder: &mut D) -> Result { -// match super::decode_option_variant(decoder, core::any::type_name::>())? { -// Some(_) => { -// let val = BorrowDecode::borrow_decode(decoder)?; -// Ok(Some(val)) -// } -// None => Ok(None), -// } -// } -// } - impl Decode for Result where T: Decode, diff --git a/src/enc/impls.rs b/src/enc/impls.rs index c0b00ae2..a6d904fe 100644 --- a/src/enc/impls.rs +++ b/src/enc/impls.rs @@ -280,16 +280,6 @@ impl Encode for char { } } -// BlockedTODO: https://github.com/rust-lang/rust/issues/37653 -// -// We'll want to implement encoding for both &[u8] and &[T: Encode], -// but those implementations overlap because u8 also implements Encode -// impl Encode for &'_ [u8] { -// fn encode(&self, encoder: &mut E) -> Result<(), EncodeError> { -// encoder.writer().write(*self) -// } -// } - impl Encode for [T] where T: Encode, @@ -356,10 +346,17 @@ where T: Encode, { fn encode(&self, encoder: &mut E) -> Result<(), EncodeError> { - for item in self.iter() { - item.encode(encoder)?; + if unty::type_equal::() { + // Safety: this is &[u8; N] + let array_slice: &[u8] = + unsafe { core::slice::from_raw_parts(self.as_ptr().cast(), N) }; + encoder.writer().write(array_slice) + } else { + for item in self.iter() { + item.encode(encoder)?; + } + Ok(()) } - Ok(()) } } diff --git a/src/features/impl_alloc.rs b/src/features/impl_alloc.rs index 71225d31..03cfc8ca 100644 --- a/src/features/impl_alloc.rs +++ b/src/features/impl_alloc.rs @@ -68,18 +68,7 @@ where T: Decode + Ord, { fn decode(decoder: &mut D) -> Result { - let len = crate::de::decode_slice_len(decoder)?; - decoder.claim_container_read::(len)?; - - let mut map = BinaryHeap::with_capacity(len); - for _ in 0..len { - // See the documentation on `unclaim_bytes_read` as to why we're doing this here - decoder.unclaim_bytes_read(core::mem::size_of::()); - - let key = T::decode(decoder)?; - map.push(key); - } - Ok(map) + Ok(Vec::::decode(decoder)?.into()) } } impl<'de, T> BorrowDecode<'de> for BinaryHeap @@ -87,18 +76,7 @@ where T: BorrowDecode<'de> + Ord, { fn borrow_decode>(decoder: &mut D) -> Result { - let len = crate::de::decode_slice_len(decoder)?; - decoder.claim_container_read::(len)?; - - let mut map = BinaryHeap::with_capacity(len); - for _ in 0..len { - // See the documentation on `unclaim_bytes_read` as to why we're doing this here - decoder.unclaim_bytes_read(core::mem::size_of::()); - - let key = T::borrow_decode(decoder)?; - map.push(key); - } - Ok(map) + Ok(Vec::::borrow_decode(decoder)?.into()) } } @@ -107,6 +85,7 @@ where T: Encode + Ord, { fn encode(&self, encoder: &mut E) -> Result<(), EncodeError> { + // BLOCKEDTODO(https://github.com/rust-lang/rust/issues/83659): we can u8 optimize this with `.as_slice()` crate::enc::encode_slice_len(encoder, self.len())?; for val in self.iter() { val.encode(encoder)?; @@ -230,18 +209,7 @@ where T: Decode, { fn decode(decoder: &mut D) -> Result { - let len = crate::de::decode_slice_len(decoder)?; - decoder.claim_container_read::(len)?; - - let mut map = VecDeque::with_capacity(len); - for _ in 0..len { - // See the documentation on `unclaim_bytes_read` as to why we're doing this here - decoder.unclaim_bytes_read(core::mem::size_of::()); - - let key = T::decode(decoder)?; - map.push_back(key); - } - Ok(map) + Ok(Vec::::decode(decoder)?.into()) } } impl<'de, T> BorrowDecode<'de> for VecDeque @@ -249,18 +217,7 @@ where T: BorrowDecode<'de>, { fn borrow_decode>(decoder: &mut D) -> Result { - let len = crate::de::decode_slice_len(decoder)?; - decoder.claim_container_read::(len)?; - - let mut map = VecDeque::with_capacity(len); - for _ in 0..len { - // See the documentation on `unclaim_bytes_read` as to why we're doing this here - decoder.unclaim_bytes_read(core::mem::size_of::()); - - let key = T::borrow_decode(decoder)?; - map.push_back(key); - } - Ok(map) + Ok(Vec::::borrow_decode(decoder)?.into()) } } @@ -270,8 +227,22 @@ where { fn encode(&self, encoder: &mut E) -> Result<(), EncodeError> { crate::enc::encode_slice_len(encoder, self.len())?; - for item in self.iter() { - item.encode(encoder)?; + if unty::type_equal::() { + let slices: (&[T], &[T]) = self.as_slices(); + // Safety: T is u8 so turning this into `&[u8]` is okay + let slices: (&[u8], &[u8]) = unsafe { + ( + core::slice::from_raw_parts(slices.0.as_ptr().cast(), slices.0.len()), + core::slice::from_raw_parts(slices.1.as_ptr().cast(), slices.1.len()), + ) + }; + + encoder.writer().write(slices.0)?; + encoder.writer().write(slices.1)?; + } else { + for item in self.iter() { + item.encode(encoder)?; + } } Ok(()) } @@ -313,16 +284,27 @@ where { fn borrow_decode>(decoder: &mut D) -> Result { let len = crate::de::decode_slice_len(decoder)?; - decoder.claim_container_read::(len)?; - let mut vec = Vec::with_capacity(len); - for _ in 0..len { - // See the documentation on `unclaim_bytes_read` as to why we're doing this here - decoder.unclaim_bytes_read(core::mem::size_of::()); + if unty::type_equal::() { + decoder.claim_container_read::(len)?; + // optimize for reading u8 vecs + let mut vec = Vec::new(); + vec.resize(len, 0u8); + decoder.reader().read(&mut vec)?; + // Safety: Vec is Vec + Ok(unsafe { core::mem::transmute(vec) }) + } else { + decoder.claim_container_read::(len)?; + + let mut vec = Vec::with_capacity(len); + for _ in 0..len { + // See the documentation on `unclaim_bytes_read` as to why we're doing this here + decoder.unclaim_bytes_read(core::mem::size_of::()); - vec.push(T::borrow_decode(decoder)?); + vec.push(T::borrow_decode(decoder)?); + } + Ok(vec) } - Ok(vec) } }