Don't needlessly call element destructors of CqlValue::Vector

scylladb · Aug 15, 2024 · 9a19bbd · 9a19bbd
1 parent faea1f9
commit 9a19bbd
Show file tree

Hide file tree

Showing 4 changed files with 63 additions and 9 deletions.
diff --git a/scylla-cql/src/frame/response/result.rs b/scylla-cql/src/frame/response/result.rs
@@ -12,7 +12,9 @@ use crate::types::deserialize::value::{
 use crate::types::deserialize::{DeserializationError, FrameSlice};
 use bytes::{Buf, Bytes};
 use std::borrow::Cow;
-use std::{convert::TryInto, net::IpAddr, result::Result as StdResult, str};
+use std::{convert::TryInto, mem, net::IpAddr, result::Result as StdResult, str};
+use std::mem::ManuallyDrop;
+use std::ops::Deref;
 use uuid::Uuid;
 
 #[derive(Debug)]
@@ -96,7 +98,7 @@ pub enum CqlValue {
     List(Vec<CqlValue>),
     Map(Vec<(CqlValue, CqlValue)>),
     Set(Vec<CqlValue>),
-    Vector(Vec<CqlValue>),
+    Vector(DropOptimizedVec<CqlValue>),
     UserDefinedType {
         keyspace: String,
         type_name: String,
@@ -115,6 +117,52 @@ pub enum CqlValue {
     Varint(CqlVarint),
 }
 
+
+#[derive(Clone, Debug, PartialEq)]
+pub struct DropOptimizedVec<T> {
+    data: Vec<T>,
+    drop_elements: bool
+}
+
+impl<T> DropOptimizedVec<T> {
+    pub fn new(data: Vec<T>, drop_elements: bool) -> DropOptimizedVec<T> {
+        DropOptimizedVec {
+            data,
+            drop_elements,
+        }
+    }
+
+    pub fn dropping(data: Vec<T>) -> DropOptimizedVec<T> {
+        Self::new(data, true)
+    }
+
+    pub fn non_dropping(data: Vec<T>) -> DropOptimizedVec<T> {
+        Self::new(data, false)
+    }
+
+    pub fn into_vec(mut self) -> Vec<T> {
+        let mut vec = vec![];
+        mem::swap(&mut self.data, &mut vec);
+        vec
+    }
+}
+
+impl<T> Deref for DropOptimizedVec<T> {
+    type Target = Vec<T>;
+
+    fn deref(&self) -> &Self::Target {
+        &self.data
+    }
+}
+
+impl<T> Drop for DropOptimizedVec<T> {
+    fn drop(&mut self) {
+        if !self.drop_elements {
+            unsafe { self.data.set_len(0); }        
+        }
+    }
+}
+
 impl<'a> TableSpec<'a> {
     pub const fn borrowed(ks: &'a str, table: &'a str) -> Self {
         Self {
@@ -355,7 +403,7 @@ impl CqlValue {
     pub fn as_list(&self) -> Option<&Vec<CqlValue>> {
         match self {
             Self::List(s) => Some(s),
-            Self::Vector(s) => Some(s),
+            Self::Vector(s) => Some(&s),
             _ => None,
         }
     }
@@ -385,7 +433,7 @@ impl CqlValue {
         match self {
             Self::List(s) => Some(s),
             Self::Set(s) => Some(s),
-            Self::Vector(s) => Some(s),
+            Self::Vector(s) => Some(s.into_vec()),
             _ => None,
         }
     }
@@ -836,11 +884,11 @@ pub fn deser_cql_value(
             let v = VectorIterator::<CqlValue>::deserialize_vector_of_float_to_vec_of_cql_value(
                 typ, v,
             )?;
-            CqlValue::Vector(v)
+            CqlValue::Vector(DropOptimizedVec::non_dropping(v))
         }
         Vector(_, _) => {
             let v = Vec::<CqlValue>::deserialize(typ, v)?;
-            CqlValue::Vector(v)
+            CqlValue::Vector(DropOptimizedVec::dropping(v))
         }
     })
 }

diff --git a/scylla-cql/src/lib.rs b/scylla-cql/src/lib.rs
@@ -29,7 +29,7 @@ pub mod _macro_internal {
     pub use crate::frame::response::cql_to_rust::{
         FromCqlVal, FromCqlValError, FromRow, FromRowError,
     };
-    pub use crate::frame::response::result::{ColumnSpec, ColumnType, CqlValue, Row};
+    pub use crate::frame::response::result::{ColumnSpec, ColumnType, CqlValue, DropOptimizedVec, Row};
     pub use crate::frame::value::{
         LegacySerializedValues, SerializedResult, Value, ValueList, ValueTooBig,
     };

diff --git a/scylla-cql/src/types/serialize/value.rs b/scylla-cql/src/types/serialize/value.rs
@@ -4,6 +4,7 @@ use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet};
 use std::fmt::Display;
 use std::hash::BuildHasher;
 use std::net::IpAddr;
+use std::ops::Deref;
 use std::sync::Arc;
 
 use thiserror::Error;
@@ -576,7 +577,7 @@ fn serialize_cql_value<'b>(
         }
         CqlValue::Uuid(u) => <_ as SerializeValue>::serialize(&u, typ, writer),
         CqlValue::Varint(v) => <_ as SerializeValue>::serialize(&v, typ, writer),
-        CqlValue::Vector(v) => <_ as SerializeValue>::serialize(&v, typ, writer),
+        CqlValue::Vector(v) => <_ as SerializeValue>::serialize(v.deref(), typ, writer),
     }
 }
 

diff --git a/scylla/src/utils/pretty.rs b/scylla/src/utils/pretty.rs
@@ -116,7 +116,12 @@ where
                 .fmt(f)?;
                 f.write_str(")")?;
             }
-            CqlValue::List(v) | CqlValue::Vector(v) => {
+            CqlValue::List(v) => {
+                f.write_str("[")?;
+                CommaSeparatedDisplayer(v.iter().map(CqlValueDisplayer)).fmt(f)?;
+                f.write_str("]")?;
+            }
+            CqlValue::Vector(v) => {
                 f.write_str("[")?;
                 CommaSeparatedDisplayer(v.iter().map(CqlValueDisplayer)).fmt(f)?;
                 f.write_str("]")?;