From 389c772764512ceec33eb849074fc1650bdb88b8 Mon Sep 17 00:00:00 2001 From: Collin Styles Date: Sun, 10 Sep 2023 10:26:40 -0700 Subject: [PATCH] Improve calculation for `SerializedValues::with_capacity` 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` 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. --- scylla-cql/src/frame/value.rs | 46 +++++++++++++++++++---------------- 1 file changed, 25 insertions(+), 21 deletions(-) diff --git a/scylla-cql/src/frame/value.rs b/scylla-cql/src/frame/value.rs index e62469f4ef..e9164f2531 100644 --- a/scylla-cql/src/frame/value.rs +++ b/scylla-cql/src/frame/value.rs @@ -830,7 +830,8 @@ impl ValueList for [u8; 0] { // Implement ValueList for slices of Value types impl 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)?; } @@ -842,7 +843,9 @@ impl ValueList for &[T] { // Implement ValueList for Vec impl ValueList for Vec { 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)?; } @@ -894,20 +897,22 @@ impl_value_list_for_btree_map!(&str); // Further variants are done using a macro impl 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) ?; )* @@ -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 ValueList for &T {