Skip to content

Commit

Permalink
Improve calculation for SerializedValues::with_capacity
Browse files Browse the repository at this point in the history
It seems that currently the argument provided to this function is being
treated as the _number_ of items that we expect to serialize. Normally
that would make sense because the argument is passed along to
`Vec::with_capacity` which provisions space for the inner
`serialized_values` field. However, that field is a `Vec<u8>` so really
the argument corresponds to the total _size_ of the serialized values in
bytes.

This commit tries to be smarter about reserving space by taking into
account the actual size in memory of the values to be serialized. It's
not perfect (e.g., `String`s will always reserve the same amount of
space, regardless of how long their actual contents are) but hopefully
it will still result in fewer re-allocations.
  • Loading branch information
cstyles committed Sep 20, 2023
1 parent ceba60a commit 389c772
Showing 1 changed file with 25 additions and 21 deletions.
46 changes: 25 additions & 21 deletions scylla-cql/src/frame/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -830,7 +830,8 @@ impl ValueList for [u8; 0] {
// Implement ValueList for slices of Value types
impl<T: Value> ValueList for &[T] {
fn serialized(&self) -> SerializedResult<'_> {
let mut result = SerializedValues::with_capacity(self.len());
let size = std::mem::size_of_val(*self);
let mut result = SerializedValues::with_capacity(size);
for val in *self {
result.add_value(val)?;
}
Expand All @@ -842,7 +843,9 @@ impl<T: Value> ValueList for &[T] {
// Implement ValueList for Vec<Value>
impl<T: Value> ValueList for Vec<T> {
fn serialized(&self) -> SerializedResult<'_> {
let mut result = SerializedValues::with_capacity(self.len());
let slice = self.as_slice();
let size = std::mem::size_of_val(slice);
let mut result = SerializedValues::with_capacity(size);
for val in self {
result.add_value(val)?;
}
Expand Down Expand Up @@ -894,20 +897,22 @@ impl_value_list_for_btree_map!(&str);
// Further variants are done using a macro
impl<T0: Value> ValueList for (T0,) {
fn serialized(&self) -> SerializedResult<'_> {
let mut result = SerializedValues::with_capacity(1);
let size = std::mem::size_of_val(self);
let mut result = SerializedValues::with_capacity(size);
result.add_value(&self.0)?;
Ok(Cow::Owned(result))
}
}

macro_rules! impl_value_list_for_tuple {
( $($Ti:ident),* ; $($FieldI:tt),* ; $size: expr) => {
( $($Ti:ident),* ; $($FieldI:tt),*) => {
impl<$($Ti),+> ValueList for ($($Ti,)+)
where
$($Ti: Value),+
{
fn serialized(&self) -> SerializedResult<'_> {
let mut result = SerializedValues::with_capacity($size);
let size = std::mem::size_of_val(self);
let mut result = SerializedValues::with_capacity(size);
$(
result.add_value(&self.$FieldI) ?;
)*
Expand All @@ -917,28 +922,27 @@ macro_rules! impl_value_list_for_tuple {
}
}

impl_value_list_for_tuple!(T0, T1; 0, 1; 2);
impl_value_list_for_tuple!(T0, T1, T2; 0, 1, 2; 3);
impl_value_list_for_tuple!(T0, T1, T2, T3; 0, 1, 2, 3; 4);
impl_value_list_for_tuple!(T0, T1, T2, T3, T4; 0, 1, 2, 3, 4; 5);
impl_value_list_for_tuple!(T0, T1, T2, T3, T4, T5; 0, 1, 2, 3, 4, 5; 6);
impl_value_list_for_tuple!(T0, T1, T2, T3, T4, T5, T6; 0, 1, 2, 3, 4, 5, 6; 7);
impl_value_list_for_tuple!(T0, T1, T2, T3, T4, T5, T6, T7; 0, 1, 2, 3, 4, 5, 6, 7; 8);
impl_value_list_for_tuple!(T0, T1, T2, T3, T4, T5, T6, T7, T8; 0, 1, 2, 3, 4, 5, 6, 7, 8; 9);
impl_value_list_for_tuple!(T0, T1, T2, T3, T4, T5, T6, T7, T8, T9;
0, 1, 2, 3, 4, 5, 6, 7, 8, 9; 10);
impl_value_list_for_tuple!(T0, T1; 0, 1);
impl_value_list_for_tuple!(T0, T1, T2; 0, 1, 2);
impl_value_list_for_tuple!(T0, T1, T2, T3; 0, 1, 2, 3);
impl_value_list_for_tuple!(T0, T1, T2, T3, T4; 0, 1, 2, 3, 4);
impl_value_list_for_tuple!(T0, T1, T2, T3, T4, T5; 0, 1, 2, 3, 4, 5);
impl_value_list_for_tuple!(T0, T1, T2, T3, T4, T5, T6; 0, 1, 2, 3, 4, 5, 6);
impl_value_list_for_tuple!(T0, T1, T2, T3, T4, T5, T6, T7; 0, 1, 2, 3, 4, 5, 6, 7);
impl_value_list_for_tuple!(T0, T1, T2, T3, T4, T5, T6, T7, T8; 0, 1, 2, 3, 4, 5, 6, 7, 8);
impl_value_list_for_tuple!(T0, T1, T2, T3, T4, T5, T6, T7, T8, T9; 0, 1, 2, 3, 4, 5, 6, 7, 8, 9);
impl_value_list_for_tuple!(T0, T1, T2, T3, T4, T5, T6, T7, T8, T9, T10;
0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10; 11);
0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10);
impl_value_list_for_tuple!(T0, T1, T2, T3, T4, T5, T6, T7, T8, T9, T10, T11;
0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11; 12);
0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11);
impl_value_list_for_tuple!(T0, T1, T2, T3, T4, T5, T6, T7, T8, T9, T10, T11, T12;
0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12; 13);
0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12);
impl_value_list_for_tuple!(T0, T1, T2, T3, T4, T5, T6, T7, T8, T9, T10, T11, T12, T13;
0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13; 14);
0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13);
impl_value_list_for_tuple!(T0, T1, T2, T3, T4, T5, T6, T7, T8, T9, T10, T11, T12, T13, T14;
0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14; 15);
0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14);
impl_value_list_for_tuple!(T0, T1, T2, T3, T4, T5, T6, T7, T8, T9, T10, T11, T12, T13, T14, T15;
0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15; 16);
0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15);

// Every &impl ValueList should also implement ValueList
impl<T: ValueList> ValueList for &T {
Expand Down

0 comments on commit 389c772

Please sign in to comment.