Skip to content

Commit

Permalink
Replace CopyOnto by Push
Browse files Browse the repository at this point in the history 
Swap the parameters for CopyOnto. Previously, we'd implement CopyOnto
for a specific type, specifying the region as a parameter to CopyOnto. This
has the undesired side-effect that downstream crates cannot implement the
trait for their own regions due to Rust's orphan rules. Switching the
parameters around (Push<T> for Region) moves a local type into the T0
position, which is compatible with the orphan rules.

Similarly updates ReserveItems.

Signed-off-by: Moritz Hoffmann <[email protected]>
  • Loading branch information
@antiguru
antiguru committed May 24, 2024
1 parent ce8ca6e commit e34b073
Show file tree
Hide file tree
Showing 14 changed files with 474 additions and 499 deletions.
2 changes: 1 addition & 1 deletion README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ flatcontainer = "0.1"
## Example

```rust
use flatcontainer::{FlatStack, CopyOnto};
use flatcontainer::FlatStack;
fn main() {
let r: Result<_, u16> = Ok("abc");
let mut c = FlatStack::default_impl::<Result<&str, u16>>();
Expand Down
13 changes: 6 additions & 7 deletions benches/bench.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ use flatcontainer::impls::deduplicate::{CollapseSequence, ConsecutiveOffsetPairs
use flatcontainer::impls::offsets::OffsetOptimized;
use flatcontainer::impls::tuple::{TupleABCRegion, TupleABRegion};
use flatcontainer::{
ColumnsRegion, Containerized, CopyOnto, FlatStack, MirrorRegion, OwnedRegion, Region,
ReserveItems, SliceRegion, StringRegion,
ColumnsRegion, Containerized, FlatStack, MirrorRegion, OwnedRegion, Push, Region, ReserveItems,
SliceRegion, StringRegion,
};
use test::Bencher;

Expand Down Expand Up @@ -265,7 +265,7 @@ fn vec_u_vn_s_prealloc(bencher: &mut Bencher) {

fn _bench_copy<T: Containerized + Eq>(bencher: &mut Bencher, record: T)
where
for<'a> &'a T: CopyOnto<<T as Containerized>::Region>,
for<'a> <T as Containerized>::Region: Push<&'a T>,
{
// prepare encoded data for bencher.bytes
let mut arena = FlatStack::default_impl::<T>();
Expand All @@ -286,7 +286,7 @@ where

fn _bench_copy_region<R: Region, T>(bencher: &mut Bencher, record: T)
where
for<'a> &'a T: CopyOnto<R>,
for<'a> R: Push<&'a T>,
{
// prepare encoded data for bencher.bytes
let mut arena = FlatStack::<R>::default();
Expand Down Expand Up @@ -319,7 +319,7 @@ fn _bench_clone<T: Containerized + Eq + Clone>(bencher: &mut Bencher, record: T)

fn _bench_realloc<T: Containerized + Eq>(bencher: &mut Bencher, record: T)
where
for<'a> &'a T: CopyOnto<<T as Containerized>::Region>,
for<'a> <T as Containerized>::Region: Push<&'a T>,
{
bencher.iter(|| {
// prepare encoded data for bencher.bytes
Expand All @@ -332,8 +332,7 @@ where

fn _bench_prealloc<T: Containerized + Eq>(bencher: &mut Bencher, record: T)
where
for<'a> &'a T:
ReserveItems<<T as Containerized>::Region> + CopyOnto<<T as Containerized>::Region>,
for<'a> <T as Containerized>::Region: ReserveItems<&'a T> + Push<&'a T>,
{
bencher.iter(|| {
// prepare encoded data for bencher.bytes
Expand Down
50 changes: 25 additions & 25 deletions src/impls/codec.rs
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//! A region that encodes its contents.
use crate::{CopyOnto, OwnedRegion, Region};
use crate::{OwnedRegion, Push, Region};

pub use self::misra_gries::MisraGries;
pub use dictionary::DictionaryCodec;
Expand Down Expand Up @@ -121,13 +121,13 @@ where
}
}

impl<C: Codec, R> CopyOnto<CodecRegion<C, R>> for &[u8]
impl<C: Codec, R> Push<&[u8]> for CodecRegion<C, R>
where
for<'a> R: Region<ReadItem<'a> = &'a [u8]> + 'a,
for<'a> &'a [u8]: CopyOnto<R>,
for<'a> R: Push<&'a [u8]>,
{
fn copy_onto(self, target: &mut CodecRegion<C, R>) -> <CodecRegion<C, R> as Region>::Index {
target.codec.encode(self, &mut target.inner)
fn push(&mut self, item: &[u8]) -> <CodecRegion<C, R> as Region>::Index {
self.codec.encode(item, &mut self.inner)
}
}

Expand All @@ -136,9 +136,9 @@ pub trait Codec: Default + 'static {
/// Decodes an input byte slice into a sequence of byte slices.
fn decode<'a>(&'a self, bytes: &'a [u8]) -> &'a [u8];
/// Encodes a sequence of byte slices into an output byte slice.
fn encode<R: Region>(&mut self, bytes: &[u8], output: &mut R) -> R::Index
fn encode<R>(&mut self, bytes: &[u8], output: &mut R) -> R::Index
where
for<'a> &'a [u8]: CopyOnto<R>;
for<'a> R: Region + Push<&'a [u8]>;
/// Constructs a new instance of `Self` from accumulated statistics.
/// These statistics should cover the data the output expects to see.
fn new_from<'a, I: Iterator<Item = &'a Self> + Clone>(stats: I) -> Self;
Expand All @@ -151,7 +151,7 @@ pub trait Codec: Default + 'static {

mod dictionary {

use crate::{CopyOnto, Region};
use crate::{Push, Region};
use std::collections::BTreeMap;

pub use super::{BytesMap, Codec, MisraGries};
Expand Down Expand Up @@ -179,18 +179,18 @@ mod dictionary {
/// Encode a sequence of byte slices.
///
/// Encoding also records statistics about the structure of the input.
fn encode<R: Region>(&mut self, bytes: &[u8], output: &mut R) -> R::Index
fn encode<R>(&mut self, bytes: &[u8], output: &mut R) -> R::Index
where
for<'a> &'a [u8]: CopyOnto<R>,
for<'a> R: Region + Push<&'a [u8]>,
{
self.total += bytes.len();
// If we have an index referencing `bytes`, use the index key.
let index = if let Some(b) = self.encode.get(bytes) {
self.bytes += 1;
[*b].as_slice().copy_onto(output)
output.push([*b].as_slice())
} else {
self.bytes += bytes.len();
bytes.copy_onto(output)
output.push(bytes)
};
// Stats stuff.
self.stats.0.insert(bytes.to_owned());
Expand Down Expand Up @@ -388,21 +388,21 @@ mod tests {
let mut r = CodecRegion::<DictionaryCodec>::default();

for _ in 0..1000 {
let index = "abc".as_bytes().copy_onto(&mut r);
let index = r.push("abc".as_bytes());
assert_eq!("abc".as_bytes(), r.index(index));
}

let mut r2 = CodecRegion::default();

for _ in 0..1000 {
let index = "abc".as_bytes().copy_onto(&mut r2);
let index = r2.push("abc".as_bytes());
assert_eq!("abc".as_bytes(), r2.index(index));
}

let mut r3 = CodecRegion::merge_regions([&r, &r2].into_iter());

for _ in 0..2000 {
let index = "abc".as_bytes().copy_onto(&mut r3);
let index = r3.push("abc".as_bytes());
assert_eq!("abc".as_bytes(), r3.index(index));
}

Expand All @@ -422,17 +422,17 @@ mod tests {
}
for _ in 0..1000 {
for r in &mut regions {
"abcdef".as_bytes().copy_onto(r);
"defghi".as_bytes().copy_onto(r);
r.push("abcdef".as_bytes());
r.push("defghi".as_bytes());
}
}

let mut merged = CodecRegion::merge_regions(regions.iter());

for _ in 0..2000 {
let index = "abcdef".as_bytes().copy_onto(&mut merged);
let index = merged.push("abcdef".as_bytes());
assert_eq!("abcdef".as_bytes(), merged.index(index));
let index = "defghi".as_bytes().copy_onto(&mut merged);
let index = merged.push("defghi".as_bytes());
assert_eq!("defghi".as_bytes(), merged.index(index));
}

Expand All @@ -448,17 +448,17 @@ mod tests {
}
for _ in 0..1000 {
for r in &mut regions {
"abcdef".as_bytes().copy_onto(r);
"defghi".as_bytes().copy_onto(r);
r.push("abcdef".as_bytes());
r.push("defghi".as_bytes());
}
}

let mut merged = CodecRegion::merge_regions(regions.iter());

for _ in 0..2000 {
let index = "abcdef".as_bytes().copy_onto(&mut merged);
let index = merged.push("abcdef".as_bytes());
assert_eq!("abcdef".as_bytes(), merged.index(index));
let index = "defghi".as_bytes().copy_onto(&mut merged);
let index = merged.push("defghi".as_bytes());
assert_eq!("defghi".as_bytes(), merged.index(index));
}

Expand All @@ -474,9 +474,9 @@ mod tests {
let mut merged2 = CodecRegion::merge_regions(std::iter::once(&merged));

for _ in 0..2000 {
let index = "abcdef".as_bytes().copy_onto(&mut merged2);
let index = merged2.push("abcdef".as_bytes());
assert_eq!("abcdef".as_bytes(), merged2.index(index));
let index = "defghi".as_bytes().copy_onto(&mut merged2);
let index = merged2.push("defghi".as_bytes());
assert_eq!("defghi".as_bytes(), merged2.index(index));
}

Expand Down
Loading

0 comments on commit e34b073

Please sign in to comment.