From 96183e804a10d3b1337f695da822623d5861e6f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Lecrenier?= Date: Thu, 6 Apr 2023 16:24:44 +0200 Subject: [PATCH] Simplify the logger --- .../search/new/graph_based_ranking_rule.rs | 12 --- milli/src/search/new/logger/detailed.rs | 55 +++----------- milli/src/search/new/logger/mod.rs | 73 ++++--------------- milli/src/search/new/mod.rs | 2 +- .../new/ranking_rule_graph/exactness/mod.rs | 29 +------- .../src/search/new/ranking_rule_graph/mod.rs | 15 ---- .../new/ranking_rule_graph/proximity/mod.rs | 31 +------- .../search/new/ranking_rule_graph/typo/mod.rs | 24 +----- milli/src/search/new/words.rs | 2 - 9 files changed, 38 insertions(+), 205 deletions(-) diff --git a/milli/src/search/new/graph_based_ranking_rule.rs b/milli/src/search/new/graph_based_ranking_rule.rs index 28b4ed1f4..154bcd3b2 100644 --- a/milli/src/search/new/graph_based_ranking_rule.rs +++ b/milli/src/search/new/graph_based_ranking_rule.rs @@ -190,10 +190,8 @@ impl<'ctx, G: RankingRuleGraphTrait> RankingRule<'ctx, QueryGraph> for GraphBase cur_distance_idx: _, } = &mut state; - let original_universe = universe; let mut universe = universe.clone(); - let original_graph = graph.clone(); let mut used_conditions = SmallBitmap::for_interned_values_in(&graph.conditions_interner); let mut good_paths = vec![]; let mut considered_paths = vec![]; @@ -272,16 +270,6 @@ impl<'ctx, G: RankingRuleGraphTrait> RankingRule<'ctx, QueryGraph> for GraphBase } })?; - G::log_state( - &original_graph, - &considered_paths, - dead_ends_cache, - original_universe, - all_costs, - cost, - logger, - ); - // We modify the next query graph so that it only contains the subgraph // that was used to compute this bucket // But we only do it in case the bucket length is >1, because otherwise diff --git a/milli/src/search/new/logger/detailed.rs b/milli/src/search/new/logger/detailed.rs index 86568d5d2..0efc457f6 100644 --- a/milli/src/search/new/logger/detailed.rs +++ b/milli/src/search/new/logger/detailed.rs @@ -1,3 +1,4 @@ +use std::any::Any; use std::fs::File; use std::io::Write; use std::path::PathBuf; @@ -6,6 +7,7 @@ use std::time::Instant; // use rand::random; use roaring::RoaringBitmap; +use crate::search::new::graph_based_ranking_rule::Typo; use crate::search::new::interner::{Interned, MappedInterner}; use crate::search::new::query_graph::QueryNodeData; use crate::search::new::query_term::LocatedQueryTermSubset; @@ -14,6 +16,8 @@ use crate::search::new::ranking_rule_graph::{ RankingRuleGraphTrait, TypoCondition, TypoGraph, }; use crate::search::new::ranking_rules::BoxRankingRule; +use crate::search::new::sort::Sort; +use crate::search::new::words::Words; use crate::search::new::{QueryGraph, QueryNode, RankingRule, SearchContext, SearchLogger}; pub enum SearchEvents { @@ -92,7 +96,7 @@ impl SearchLogger for DetailedSearchLogger { self.initial_query_time = Some(Instant::now()); } - fn query_for_universe(&mut self, query: &QueryGraph) { + fn query_for_initial_universe(&mut self, query: &QueryGraph) { self.query_for_universe = Some(query.clone()); } @@ -161,46 +165,12 @@ impl SearchLogger for DetailedSearchLogger { self.events.push(SearchEvents::ExtendResults { new: docids.to_vec() }); } - fn log_words_state(&mut self, query_graph: &QueryGraph) { - self.events.push(SearchEvents::WordsState { query_graph: query_graph.clone() }); - } - - fn log_proximity_state( - &mut self, - query_graph: &RankingRuleGraph, - paths_map: &[Vec>], - dead_ends_cache: &DeadEndsCache, - universe: &RoaringBitmap, - costs: &MappedInterner>, - cost: u64, - ) { - self.events.push(SearchEvents::ProximityState { - graph: query_graph.clone(), - paths: paths_map.to_vec(), - dead_ends_cache: dead_ends_cache.clone(), - universe: universe.clone(), - costs: costs.clone(), - cost, - }) - } - - fn log_typo_state( - &mut self, - query_graph: &RankingRuleGraph, - paths_map: &[Vec>], - dead_ends_cache: &DeadEndsCache, - universe: &RoaringBitmap, - costs: &MappedInterner>, - cost: u64, - ) { - self.events.push(SearchEvents::TypoState { - graph: query_graph.clone(), - paths: paths_map.to_vec(), - dead_ends_cache: dead_ends_cache.clone(), - universe: universe.clone(), - costs: costs.clone(), - cost, - }) + /// Logs the internal state of the ranking rule + fn log_ranking_rule_state<'ctx>(&mut self, state: &(dyn Any + 'ctx)) { + if let Some(_words) = state.downcast_ref::() { + } else if let Some(_sort) = state.downcast_ref::>() { + } else if let Some(_typo) = state.downcast_ref::() { + } } } @@ -567,9 +537,8 @@ results.{cur_ranking_rule}{cur_activated_id} {{ file, "{condition_id} {{ shape: class -{} +label }}", - R::label_for_condition(ctx, condition).unwrap() ) .unwrap(); } diff --git a/milli/src/search/new/logger/mod.rs b/milli/src/search/new/logger/mod.rs index 889e811ad..1ef048a8e 100644 --- a/milli/src/search/new/logger/mod.rs +++ b/milli/src/search/new/logger/mod.rs @@ -1,14 +1,14 @@ // #[cfg(test)] pub mod detailed; +use std::any::Any; + use roaring::RoaringBitmap; -use super::interner::{Interned, MappedInterner}; -use super::query_graph::QueryNode; -use super::ranking_rule_graph::{ - DeadEndsCache, ProximityCondition, ProximityGraph, RankingRuleGraph, TypoCondition, TypoGraph, -}; +use super::graph_based_ranking_rule::Typo; use super::ranking_rules::BoxRankingRule; +use super::sort::Sort; +use super::words::Words; use super::{RankingRule, RankingRuleQueryTrait}; /// Trait for structure logging the execution of a search query. @@ -16,12 +16,12 @@ pub trait SearchLogger { /// Logs the initial query fn initial_query(&mut self, query: &Q); - /// Logs the query that was used to compute the set of all candidates - fn query_for_universe(&mut self, query: &Q); - /// Logs the value of the initial set of all candidates fn initial_universe(&mut self, universe: &RoaringBitmap); + /// Logs the query that was used to compute the set of all candidates + fn query_for_initial_universe(&mut self, query: &Q); + /// Logs the ranking rules used to perform the search query fn ranking_rules(&mut self, rr: &[BoxRankingRule]); @@ -58,30 +58,13 @@ pub trait SearchLogger { /// Logs the addition of document ids to the final results fn add_to_results(&mut self, docids: &[u32]); - /// Logs the internal state of the words ranking rule - fn log_words_state(&mut self, query_graph: &Q); - - /// Logs the internal state of the proximity ranking rule - fn log_proximity_state( - &mut self, - query_graph: &RankingRuleGraph, - paths: &[Vec>], - dead_ends_cache: &DeadEndsCache, - universe: &RoaringBitmap, - distances: &MappedInterner>, - cost: u64, - ); - - /// Logs the internal state of the typo ranking rule - fn log_typo_state( - &mut self, - query_graph: &RankingRuleGraph, - paths: &[Vec>], - dead_ends_cache: &DeadEndsCache, - universe: &RoaringBitmap, - distances: &MappedInterner>, - cost: u64, - ); + /// Logs the internal state of the ranking rule + fn log_ranking_rule_state<'ctx>(&mut self, rr: &(dyn Any + 'ctx)) { + if let Some(_words) = rr.downcast_ref::() { + } else if let Some(_sort) = rr.downcast_ref::>() { + } else if let Some(_typo) = rr.downcast_ref::() { + } + } } /// A dummy [`SearchLogger`] which does nothing. @@ -90,7 +73,7 @@ pub struct DefaultSearchLogger; impl SearchLogger for DefaultSearchLogger { fn initial_query(&mut self, _query: &Q) {} - fn query_for_universe(&mut self, _query: &Q) {} + fn query_for_initial_universe(&mut self, _query: &Q) {} fn initial_universe(&mut self, _universe: &RoaringBitmap) {} @@ -130,28 +113,4 @@ impl SearchLogger for DefaultSearchLogger { } fn add_to_results(&mut self, _docids: &[u32]) {} - - fn log_words_state(&mut self, _query_graph: &Q) {} - - fn log_proximity_state( - &mut self, - _query_graph: &RankingRuleGraph, - _paths_map: &[Vec>], - _dead_ends_cache: &DeadEndsCache, - _universe: &RoaringBitmap, - _distances: &MappedInterner>, - _cost: u64, - ) { - } - - fn log_typo_state( - &mut self, - _query_graph: &RankingRuleGraph, - _paths: &[Vec>], - _dead_ends_cache: &DeadEndsCache, - _universe: &RoaringBitmap, - _distances: &MappedInterner>, - _cost: u64, - ) { - } } diff --git a/milli/src/search/new/mod.rs b/milli/src/search/new/mod.rs index e07a17029..5bebd3bff 100644 --- a/milli/src/search/new/mod.rs +++ b/milli/src/search/new/mod.rs @@ -96,7 +96,7 @@ fn resolve_maximally_reduced_query_graph( }; graph.remove_nodes_keep_edges(&nodes_to_remove); - logger.query_for_universe(&graph); + logger.query_for_initial_universe(&graph); let docids = compute_query_graph_docids(ctx, &graph, universe)?; Ok(docids) diff --git a/milli/src/search/new/ranking_rule_graph/exactness/mod.rs b/milli/src/search/new/ranking_rule_graph/exactness/mod.rs index 6639299a1..55c4497dd 100644 --- a/milli/src/search/new/ranking_rule_graph/exactness/mod.rs +++ b/milli/src/search/new/ranking_rule_graph/exactness/mod.rs @@ -1,11 +1,10 @@ use heed::BytesDecode; use roaring::RoaringBitmap; -use super::{ComputedCondition, DeadEndsCache, RankingRuleGraph, RankingRuleGraphTrait}; -use crate::search::new::interner::{DedupInterner, Interned, MappedInterner}; -use crate::search::new::query_graph::{QueryGraph, QueryNode}; +use super::{ComputedCondition, RankingRuleGraphTrait}; +use crate::search::new::interner::{DedupInterner, Interned}; use crate::search::new::query_term::{ExactTerm, LocatedQueryTermSubset}; -use crate::{Result, RoaringBitmapCodec, SearchContext, SearchLogger}; +use crate::{Result, RoaringBitmapCodec, SearchContext}; #[derive(Clone, PartialEq, Eq, Hash)] pub enum ExactnessCondition { @@ -77,26 +76,4 @@ impl RankingRuleGraphTrait for ExactnessGraph { Ok(vec![(0, exact_condition), (dest_node.term_ids.len() as u32, skip_condition)]) } - - fn log_state( - _graph: &RankingRuleGraph, - _paths: &[Vec>], - _dead_ends_cache: &DeadEndsCache, - _niverse: &RoaringBitmap, - _costs: &MappedInterner>, - _cost: u64, - _logger: &mut dyn SearchLogger, - ) { - } - - fn label_for_condition( - _ctx: &mut SearchContext, - condition: &Self::Condition, - ) -> Result { - Ok(match condition { - ExactnessCondition::ExactInAttribute(_) => "exact", - ExactnessCondition::Skip(_) => "skip", - } - .to_owned()) - } } diff --git a/milli/src/search/new/ranking_rule_graph/mod.rs b/milli/src/search/new/ranking_rule_graph/mod.rs index 936c3e942..00e759a28 100644 --- a/milli/src/search/new/ranking_rule_graph/mod.rs +++ b/milli/src/search/new/ranking_rule_graph/mod.rs @@ -28,7 +28,6 @@ use roaring::RoaringBitmap; pub use typo::{TypoCondition, TypoGraph}; use super::interner::{DedupInterner, FixedSizeInterner, Interned, MappedInterner}; -use super::logger::SearchLogger; use super::query_term::LocatedQueryTermSubset; use super::small_bitmap::SmallBitmap; use super::{QueryGraph, QueryNode, SearchContext}; @@ -86,10 +85,6 @@ impl PartialEq for Edge { pub trait RankingRuleGraphTrait: Sized { type Condition: Sized + Clone + PartialEq + Eq + Hash; - /// Return the label of the given edge condition, to be used when visualising - /// the ranking rule graph. - fn label_for_condition(ctx: &mut SearchContext, condition: &Self::Condition) -> Result; - /// Compute the document ids associated with the given edge condition, /// restricted to the given universe. fn resolve_condition( @@ -105,16 +100,6 @@ pub trait RankingRuleGraphTrait: Sized { source_node: Option<&LocatedQueryTermSubset>, dest_node: &LocatedQueryTermSubset, ) -> Result)>>; - - fn log_state( - graph: &RankingRuleGraph, - paths: &[Vec>], - dead_ends_cache: &DeadEndsCache, - universe: &RoaringBitmap, - costs: &MappedInterner>, - cost: u64, - logger: &mut dyn SearchLogger, - ); } /// The graph used by graph-based ranking rules. diff --git a/milli/src/search/new/ranking_rule_graph/proximity/mod.rs b/milli/src/search/new/ranking_rule_graph/proximity/mod.rs index cfd3f62bf..ead717a6f 100644 --- a/milli/src/search/new/ranking_rule_graph/proximity/mod.rs +++ b/milli/src/search/new/ranking_rule_graph/proximity/mod.rs @@ -3,11 +3,10 @@ pub mod compute_docids; use roaring::RoaringBitmap; -use super::{ComputedCondition, DeadEndsCache, RankingRuleGraph, RankingRuleGraphTrait}; -use crate::search::new::interner::{DedupInterner, Interned, MappedInterner}; -use crate::search::new::logger::SearchLogger; +use super::{ComputedCondition, RankingRuleGraphTrait}; +use crate::search::new::interner::{DedupInterner, Interned}; use crate::search::new::query_term::LocatedQueryTermSubset; -use crate::search::new::{QueryGraph, QueryNode, SearchContext}; +use crate::search::new::SearchContext; use crate::Result; #[derive(Clone, PartialEq, Eq, Hash)] @@ -37,28 +36,4 @@ impl RankingRuleGraphTrait for ProximityGraph { ) -> Result)>> { build::build_edges(ctx, conditions_interner, source_term, dest_term) } - - fn log_state( - graph: &RankingRuleGraph, - paths: &[Vec>], - dead_ends_cache: &DeadEndsCache, - universe: &RoaringBitmap, - distances: &MappedInterner>, - cost: u64, - logger: &mut dyn SearchLogger, - ) { - logger.log_proximity_state(graph, paths, dead_ends_cache, universe, distances, cost); - } - - fn label_for_condition(ctx: &mut SearchContext, condition: &Self::Condition) -> Result { - match condition { - ProximityCondition::Uninit { cost, .. } => { - // TODO - Ok(format!("{cost}: cost")) - } - ProximityCondition::Term { term } => { - Ok(format!("{} : exists", term.term_subset.description(ctx))) - } - } - } } diff --git a/milli/src/search/new/ranking_rule_graph/typo/mod.rs b/milli/src/search/new/ranking_rule_graph/typo/mod.rs index 5d7e0f874..da5198c23 100644 --- a/milli/src/search/new/ranking_rule_graph/typo/mod.rs +++ b/milli/src/search/new/ranking_rule_graph/typo/mod.rs @@ -1,11 +1,10 @@ use roaring::RoaringBitmap; -use super::{ComputedCondition, DeadEndsCache, RankingRuleGraph, RankingRuleGraphTrait}; -use crate::search::new::interner::{DedupInterner, Interned, MappedInterner}; -use crate::search::new::logger::SearchLogger; +use super::{ComputedCondition, RankingRuleGraphTrait}; +use crate::search::new::interner::{DedupInterner, Interned}; use crate::search::new::query_term::LocatedQueryTermSubset; use crate::search::new::resolve_query_graph::compute_query_term_subset_docids; -use crate::search::new::{QueryGraph, QueryNode, SearchContext}; +use crate::search::new::SearchContext; use crate::Result; #[derive(Clone, PartialEq, Eq, Hash)] @@ -76,21 +75,4 @@ impl RankingRuleGraphTrait for TypoGraph { } Ok(edges) } - - fn log_state( - graph: &RankingRuleGraph, - paths: &[Vec>], - dead_ends_cache: &DeadEndsCache, - universe: &RoaringBitmap, - distances: &MappedInterner>, - cost: u64, - logger: &mut dyn SearchLogger, - ) { - logger.log_typo_state(graph, paths, dead_ends_cache, universe, distances, cost); - } - - fn label_for_condition(ctx: &mut SearchContext, condition: &Self::Condition) -> Result { - let TypoCondition { term, nbr_typos } = condition; - Ok(format!("{}: {nbr_typos}", term.term_subset.description(ctx))) - } } diff --git a/milli/src/search/new/words.rs b/milli/src/search/new/words.rs index 0036694c3..c3ae07bcb 100644 --- a/milli/src/search/new/words.rs +++ b/milli/src/search/new/words.rs @@ -66,8 +66,6 @@ impl<'ctx> RankingRule<'ctx, QueryGraph> for Words { } let Some(query_graph) = &mut self.query_graph else { panic!() }; - logger.log_words_state(query_graph); - let this_bucket = compute_query_graph_docids(ctx, query_graph, universe)?; let child_query_graph = query_graph.clone();