From 46feabdd59d201556a398ccfa9ffccb73b776aab Mon Sep 17 00:00:00 2001 From: Ivan Pleshkov Date: Thu, 10 Aug 2023 14:01:38 +0200 Subject: [PATCH] Optimize strings ram usage for map index (#2388) * optimize strings ram usage for map index * conversion tests * are you happy fmt * better unicode test * better unicode test * are you happy clippy * better comment * are you happy fmt * Use SmolStr instead of custom string --- Cargo.lock | 10 +++ lib/segment/Cargo.toml | 1 + .../src/index/field_index/field_index_base.rs | 3 +- .../src/index/field_index/map_index.rs | 70 +++++++++++++------ .../query_optimization/condition_converter.rs | 16 +++-- lib/segment/src/types.rs | 9 +++ 6 files changed, 82 insertions(+), 27 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7a023fee66..b13c16b06d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4351,6 +4351,7 @@ dependencies = [ "serde-value", "serde_cbor", "serde_json", + "smol_str", "sysinfo", "tar", "tempfile", @@ -4564,6 +4565,15 @@ version = "1.10.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a507befe795404456341dfab10cef66ead4c041f62b8b11bbb92bffe5d0953e0" +[[package]] +name = "smol_str" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "74212e6bbe9a4352329b2f68ba3130c15a3f26fe88ff22dbdc6cdd58fa85e99c" +dependencies = [ + "serde", +] + [[package]] name = "socket2" version = "0.4.9" diff --git a/lib/segment/Cargo.toml b/lib/segment/Cargo.toml index 8590e87ede..d25c70a2a4 100644 --- a/lib/segment/Cargo.toml +++ b/lib/segment/Cargo.toml @@ -59,6 +59,7 @@ tinyvec = { version = "1.6.0", features = ["alloc"] } quantization = { git = "https://github.com/qdrant/quantization.git" } validator = { version = "0.16", features = ["derive"] } chrono = { version = "0.4.26", features = ["serde"] } +smol_str = "0.2.0" sysinfo = "0.29" futures = "0.3.28" diff --git a/lib/segment/src/index/field_index/field_index_base.rs b/lib/segment/src/index/field_index/field_index_base.rs index 2c45ab5592..f3774b8941 100644 --- a/lib/segment/src/index/field_index/field_index_base.rs +++ b/lib/segment/src/index/field_index/field_index_base.rs @@ -1,4 +1,5 @@ use serde_json::Value; +use smol_str::SmolStr; use crate::common::utils::MultiValue; use crate::common::Flusher; @@ -115,7 +116,7 @@ pub trait ValueIndexer { pub enum FieldIndex { IntIndex(NumericIndex), IntMapIndex(MapIndex), - KeywordIndex(MapIndex), + KeywordIndex(MapIndex), FloatIndex(NumericIndex), GeoIndex(GeoMapIndex), FullTextIndex(FullTextIndex), diff --git a/lib/segment/src/index/field_index/map_index.rs b/lib/segment/src/index/field_index/map_index.rs index 4f737da082..5997b0e361 100644 --- a/lib/segment/src/index/field_index/map_index.rs +++ b/lib/segment/src/index/field_index/map_index.rs @@ -9,6 +9,7 @@ use itertools::Itertools; use parking_lot::RwLock; use rocksdb::DB; use serde_json::Value; +use smol_str::SmolStr; use crate::common::rocksdb_wrapper::DatabaseColumnWrapper; use crate::common::Flusher; @@ -83,7 +84,12 @@ impl MapIndex { self.db_wrapper.flusher() } - pub fn match_cardinality(&self, value: &N) -> CardinalityEstimation { + pub fn match_cardinality(&self, value: &Q) -> CardinalityEstimation + where + Q: ?Sized, + N: std::borrow::Borrow, + Q: Hash + Eq, + { let values_count = self.map.get(value).map(|p| p.len()).unwrap_or(0); CardinalityEstimation::exact(values_count) @@ -102,7 +108,10 @@ impl MapIndex { } } - fn add_many_to_map(&mut self, idx: PointOffsetType, values: Vec) -> OperationResult<()> { + fn add_many_to_map(&mut self, idx: PointOffsetType, values: Vec) -> OperationResult<()> + where + Q: Into, + { if values.is_empty() { return Ok(()); } @@ -111,7 +120,7 @@ impl MapIndex { if self.point_to_values.len() <= idx as usize { self.point_to_values.resize(idx as usize + 1, Vec::new()) } - self.point_to_values[idx as usize] = values.into_iter().collect(); + self.point_to_values[idx as usize] = values.into_iter().map(|v| v.into()).collect(); for value in &self.point_to_values[idx as usize] { let entry = self.map.entry(value.clone()).or_default(); entry.insert(idx); @@ -123,7 +132,12 @@ impl MapIndex { Ok(()) } - fn get_iterator(&self, value: &N) -> Box + '_> { + fn get_iterator(&self, value: &Q) -> Box + '_> + where + Q: ?Sized, + N: std::borrow::Borrow, + Q: Hash + Eq, + { self.map .get(value) .map(|ids| Box::new(ids.iter().copied()) as Box>) @@ -193,7 +207,13 @@ impl MapIndex { /// # Returns /// /// * `CardinalityEstimation` - estimation of cardinality - fn except_cardinality(&self, excluded: &[N]) -> CardinalityEstimation { + fn except_cardinality(&self, excluded: impl Iterator) -> CardinalityEstimation + where + I: std::borrow::Borrow, + Q: ?Sized, + N: std::borrow::Borrow, + Q: Hash + Eq, + { // Minimal case: we exclude as many points as possible. // In this case, excluded points do not have any other values except excluded ones. // So the first step - we estimate how many other points is needed to fit unused values. @@ -235,9 +255,14 @@ impl MapIndex { // exp = ... // max = min(60, 20) = 20 + // todo let excluded_value_counts: Vec<_> = excluded - .iter() - .map(|val| self.map.get(val).map(|points| points.len()).unwrap_or(0)) + .map(|val| { + self.map + .get(val.borrow()) + .map(|points| points.len()) + .unwrap_or(0) + }) .collect(); let total_excluded_value_count: usize = excluded_value_counts.iter().sum(); @@ -245,7 +270,7 @@ impl MapIndex { let non_excluded_values_count = self.values_count.saturating_sub(total_excluded_value_count); - let max_values_per_point = self.map.len().saturating_sub(excluded.len()); + let max_values_per_point = self.map.len().saturating_sub(excluded_value_counts.len()); if max_values_per_point == 0 { // All points are excluded, so we can't select any point @@ -288,21 +313,24 @@ impl MapIndex { } } - fn except_iterator<'a>( + fn except_iterator<'a, Q>( &'a self, - excluded: &'a [N], - ) -> Box + 'a> { + excluded: &'a [Q], + ) -> Box + 'a> + where + Q: PartialEq, + { let iter = self .map .keys() - .filter(|key| !excluded.contains(*key)) + .filter(|key| !excluded.iter().any(|e| e.eq(*key))) .flat_map(|key| self.get_iterator(key)) .unique(); Box::new(iter) } } -impl PayloadFieldIndex for MapIndex { +impl PayloadFieldIndex for MapIndex { fn count_indexed_points(&self) -> usize { self.indexed_points } @@ -326,13 +354,13 @@ impl PayloadFieldIndex for MapIndex { match &condition.r#match { Some(Match::Value(MatchValue { value: ValueVariants::Keyword(keyword), - })) => Some(self.get_iterator(keyword)), + })) => Some(self.get_iterator(keyword.as_str())), Some(Match::Any(MatchAny { any: AnyVariants::Keywords(keywords), })) => Some(Box::new( keywords .iter() - .flat_map(|keyword| self.get_iterator(keyword)) + .flat_map(|keyword| self.get_iterator(keyword.as_str())) .unique(), )), Some(Match::Except(MatchExcept { @@ -347,7 +375,7 @@ impl PayloadFieldIndex for MapIndex { Some(Match::Value(MatchValue { value: ValueVariants::Keyword(keyword), })) => { - let mut estimation = self.match_cardinality(keyword); + let mut estimation = self.match_cardinality(keyword.as_str()); estimation .primary_clauses .push(PrimaryCondition::Condition(condition.clone())); @@ -358,7 +386,7 @@ impl PayloadFieldIndex for MapIndex { })) => { let estimations = keywords .iter() - .map(|keyword| self.match_cardinality(keyword)) + .map(|keyword| self.match_cardinality(keyword.as_str())) .collect::>(); Some(combine_should_estimations( &estimations, @@ -367,7 +395,7 @@ impl PayloadFieldIndex for MapIndex { } Some(Match::Except(MatchExcept { except: AnyVariants::Keywords(keywords), - })) => Some(self.except_cardinality(keywords)), + })) => Some(self.except_cardinality::(keywords.iter().map(|k| k.as_str()))), _ => None, } } @@ -454,7 +482,9 @@ impl PayloadFieldIndex for MapIndex { } Some(Match::Except(MatchExcept { except: AnyVariants::Integers(integers), - })) => Some(self.except_cardinality(integers)), + })) => Some( + self.except_cardinality::(integers.iter().cloned()), + ), _ => None, } } @@ -476,7 +506,7 @@ impl PayloadFieldIndex for MapIndex { } } -impl ValueIndexer for MapIndex { +impl ValueIndexer for MapIndex { fn add_many(&mut self, id: PointOffsetType, values: Vec) -> OperationResult<()> { self.add_many_to_map(id, values) } diff --git a/lib/segment/src/index/query_optimization/condition_converter.rs b/lib/segment/src/index/query_optimization/condition_converter.rs index 31c3ff8a10..1dc7db0985 100644 --- a/lib/segment/src/index/query_optimization/condition_converter.rs +++ b/lib/segment/src/index/query_optimization/condition_converter.rs @@ -249,9 +249,11 @@ pub fn get_match_checkers(index: &FieldIndex, cond_match: Match) -> Option match (any, index) { (AnyVariants::Keywords(list), FieldIndex::KeywordIndex(index)) => { Some(Box::new(move |point_id: PointOffsetType| { - index - .get_values(point_id) - .map_or(false, |values| values.iter().any(|k| list.contains(k))) + index.get_values(point_id).map_or(false, |values| { + values + .iter() + .any(|k| list.iter().any(|s| s.as_str() == k.as_ref())) + }) })) } (AnyVariants::Integers(list), FieldIndex::IntMapIndex(index)) => { @@ -266,9 +268,11 @@ pub fn get_match_checkers(index: &FieldIndex, cond_match: Match) -> Option match (except, index) { (AnyVariants::Keywords(list), FieldIndex::KeywordIndex(index)) => { Some(Box::new(move |point_id: PointOffsetType| { - index - .get_values(point_id) - .map_or(false, |values| values.iter().any(|k| !list.contains(k))) + index.get_values(point_id).map_or(false, |values| { + values + .iter() + .any(|k| !list.iter().any(|s| s.as_str() == k.as_ref())) + }) })) } (AnyVariants::Integers(list), FieldIndex::IntMapIndex(index)) => { diff --git a/lib/segment/src/types.rs b/lib/segment/src/types.rs index 7dccdc5303..4e21425e33 100644 --- a/lib/segment/src/types.rs +++ b/lib/segment/src/types.rs @@ -14,6 +14,7 @@ use ordered_float::OrderedFloat; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use serde_json::{Map, Value}; +use smol_str::SmolStr; use uuid::Uuid; use validator::{Validate, ValidationErrors}; @@ -1115,6 +1116,14 @@ impl From for Match { } } +impl From for Match { + fn from(keyword: SmolStr) -> Self { + Self::Value(MatchValue { + value: ValueVariants::Keyword(keyword.into()), + }) + } +} + impl From for Match { fn from(integer: IntPayloadType) -> Self { Self::Value(MatchValue {