From a39e2fa7d5ea311739b060d294491d89a3329826 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Thu, 12 Oct 2023 11:37:59 +0200 Subject: [PATCH] Remove unsued `EntityPathOpMsg` --- crates/re_data_store/src/store_db.rs | 34 ++------------------------- crates/re_data_ui/src/log_msg.rs | 29 +---------------------- crates/re_log_types/src/lib.rs | 23 +----------------- crates/re_sdk/src/recording_stream.rs | 16 ++++++------- crates/re_sdk_comms/src/server.rs | 2 +- crates/re_viewer/src/saving.rs | 7 ------ 6 files changed, 13 insertions(+), 98 deletions(-) diff --git a/crates/re_data_store/src/store_db.rs b/crates/re_data_store/src/store_db.rs index 70a7bb13a58a..26b94a42b1e5 100644 --- a/crates/re_data_store/src/store_db.rs +++ b/crates/re_data_store/src/store_db.rs @@ -4,9 +4,8 @@ use nohash_hasher::IntMap; use re_arrow_store::{DataStoreConfig, GarbageCollectionOptions}; use re_log_types::{ - ApplicationId, ComponentPath, DataCell, DataRow, DataTable, EntityPath, EntityPathHash, - EntityPathOpMsg, LogMsg, PathOp, RowId, SetStoreInfo, StoreId, StoreInfo, StoreKind, TimePoint, - Timeline, + ApplicationId, ComponentPath, DataCell, DataRow, DataTable, EntityPath, EntityPathHash, LogMsg, + PathOp, RowId, SetStoreInfo, StoreId, StoreInfo, StoreKind, TimePoint, Timeline, }; use re_types::{components::InstanceKey, Loggable as _}; @@ -218,9 +217,6 @@ pub struct StoreDb { /// The [`StoreId`] for this log. store_id: StoreId, - /// All [`EntityPathOpMsg`]s ever received. - entity_op_msgs: BTreeMap, - /// Set by whomever created this [`StoreDb`]. pub data_source: Option, @@ -235,7 +231,6 @@ impl StoreDb { pub fn new(store_id: StoreId) -> Self { Self { store_id, - entity_op_msgs: Default::default(), data_source: None, set_store_info: None, entity_db: Default::default(), @@ -307,16 +302,6 @@ impl StoreDb { match &msg { LogMsg::SetStoreInfo(msg) => self.set_store_info(msg.clone()), - LogMsg::EntityPathOpMsg(_, msg) => { - let EntityPathOpMsg { - row_id, - time_point, - path_op, - } = msg; - self.entity_op_msgs.insert(*row_id, msg.clone()); - self.entity_db.add_path_op(*row_id, time_point, path_op); - } - LogMsg::ArrowMsg(_, arrow_msg) => { let table = DataTable::from_arrow_msg(arrow_msg)?; self.add_data_table(table)?; @@ -347,15 +332,6 @@ impl StoreDb { self.set_store_info = Some(store_info); } - /// Returns an iterator over all [`EntityPathOpMsg`]s that have been written to this `StoreDb`. - pub fn iter_entity_op_msgs(&self) -> impl Iterator { - self.entity_op_msgs.values() - } - - pub fn get_entity_op_msg(&self, row_id: &RowId) -> Option<&EntityPathOpMsg> { - self.entity_op_msgs.get(row_id) - } - pub fn gc_everything_but_the_latest_row(&mut self) { re_tracing::profile_function!(); @@ -394,17 +370,11 @@ impl StoreDb { let Self { store_id: _, - entity_op_msgs, data_source: _, set_store_info: _, entity_db, } = self; - { - re_tracing::profile_scope!("entity_op_msgs"); - entity_op_msgs.retain(|row_id, _| !deleted.row_ids.contains(row_id)); - } - entity_db.purge(&deleted); } diff --git a/crates/re_data_ui/src/log_msg.rs b/crates/re_data_ui/src/log_msg.rs index dcb6de45ad41..42a96141a6f0 100644 --- a/crates/re_data_ui/src/log_msg.rs +++ b/crates/re_data_ui/src/log_msg.rs @@ -1,4 +1,4 @@ -use re_log_types::{ArrowMsg, DataTable, EntityPathOpMsg, LogMsg, SetStoreInfo, StoreInfo}; +use re_log_types::{ArrowMsg, DataTable, LogMsg, SetStoreInfo, StoreInfo}; use re_viewer_context::{UiVerbosity, ViewerContext}; use super::DataUi; @@ -14,7 +14,6 @@ impl DataUi for LogMsg { ) { match self { LogMsg::SetStoreInfo(msg) => msg.data_ui(ctx, ui, verbosity, query), - LogMsg::EntityPathOpMsg(_, msg) => msg.data_ui(ctx, ui, verbosity, query), LogMsg::ArrowMsg(_, msg) => msg.data_ui(ctx, ui, verbosity, query), } } @@ -67,32 +66,6 @@ impl DataUi for SetStoreInfo { } } -impl DataUi for EntityPathOpMsg { - fn data_ui( - &self, - ctx: &mut ViewerContext<'_>, - ui: &mut egui::Ui, - verbosity: UiVerbosity, - query: &re_arrow_store::LatestAtQuery, - ) { - let EntityPathOpMsg { - row_id: _, - time_point, - path_op, - } = self; - - egui::Grid::new("fields").num_columns(2).show(ui, |ui| { - ui.monospace("time_point:"); - time_point.data_ui(ctx, ui, verbosity, query); - ui.end_row(); - - ui.monospace("path_op:"); - path_op.data_ui(ctx, ui, verbosity, query); - ui.end_row(); - }); - } -} - impl DataUi for ArrowMsg { fn data_ui( &self, diff --git a/crates/re_log_types/src/lib.rs b/crates/re_log_types/src/lib.rs index aac810469730..6f97bcb09aae 100644 --- a/crates/re_log_types/src/lib.rs +++ b/crates/re_log_types/src/lib.rs @@ -228,9 +228,6 @@ pub enum LogMsg { /// Should usually be the first message sent. SetStoreInfo(SetStoreInfo), - /// Server-backed operation on an [`EntityPath`]. - EntityPathOpMsg(StoreId, EntityPathOpMsg), - /// Log an entity using an [`ArrowMsg`]. ArrowMsg(StoreId, ArrowMsg), } @@ -239,7 +236,7 @@ impl LogMsg { pub fn store_id(&self) -> &StoreId { match self { Self::SetStoreInfo(msg) => &msg.info.store_id, - Self::EntityPathOpMsg(store_id, _) | Self::ArrowMsg(store_id, _) => store_id, + Self::ArrowMsg(store_id, _) => store_id, } } } @@ -372,24 +369,6 @@ impl std::fmt::Display for StoreSource { // ---------------------------------------------------------------------------- -/// An operation (like a 'clear') on an [`EntityPath`]. -#[must_use] -#[derive(Clone, Debug, PartialEq, Eq)] -#[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))] -pub struct EntityPathOpMsg { - /// A unique id per [`EntityPathOpMsg`]. - pub row_id: RowId, - - /// Time information (when it was logged, when it was received, …). - /// - /// If this is empty, no operation will be performed as we - /// cannot be timeless in a meaningful way. - pub time_point: TimePoint, - - /// What operation. - pub path_op: PathOp, -} - /// Operation to perform on an [`EntityPath`], e.g. clearing all components. #[derive(Clone, Debug, PartialEq, Eq)] #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))] diff --git a/crates/re_sdk/src/recording_stream.rs b/crates/re_sdk/src/recording_stream.rs index 5d43a727e490..4cd6ec7ec4e5 100644 --- a/crates/re_sdk/src/recording_stream.rs +++ b/crates/re_sdk/src/recording_stream.rs @@ -1423,7 +1423,7 @@ mod tests { assert!(msg.row_id != RowId::ZERO); similar_asserts::assert_eq!(store_info, msg.info); } - _ => panic!("expected SetStoreInfo"), + LogMsg::ArrowMsg { .. } => panic!("expected SetStoreInfo"), } // Second message should be a set_store_info resulting from the later sink swap from @@ -1434,7 +1434,7 @@ mod tests { assert!(msg.row_id != RowId::ZERO); similar_asserts::assert_eq!(store_info, msg.info); } - _ => panic!("expected SetStoreInfo"), + LogMsg::ArrowMsg { .. } => panic!("expected SetStoreInfo"), } // Third message is the batched table itself, which was sent as a result of the implicit @@ -1451,7 +1451,7 @@ mod tests { similar_asserts::assert_eq!(table, got); } - _ => panic!("expected ArrowMsg"), + LogMsg::SetStoreInfo { .. } => panic!("expected ArrowMsg"), } // That's all. @@ -1490,7 +1490,7 @@ mod tests { assert!(msg.row_id != RowId::ZERO); similar_asserts::assert_eq!(store_info, msg.info); } - _ => panic!("expected SetStoreInfo"), + LogMsg::ArrowMsg { .. } => panic!("expected SetStoreInfo"), } // Second message should be a set_store_info resulting from the later sink swap from @@ -1501,7 +1501,7 @@ mod tests { assert!(msg.row_id != RowId::ZERO); similar_asserts::assert_eq!(store_info, msg.info); } - _ => panic!("expected SetStoreInfo"), + LogMsg::ArrowMsg { .. } => panic!("expected SetStoreInfo"), } let mut rows = { @@ -1525,7 +1525,7 @@ mod tests { similar_asserts::assert_eq!(expected, got); } - _ => panic!("expected ArrowMsg"), + LogMsg::SetStoreInfo { .. } => panic!("expected ArrowMsg"), } }; @@ -1570,7 +1570,7 @@ mod tests { assert!(msg.row_id != RowId::ZERO); similar_asserts::assert_eq!(store_info, msg.info); } - _ => panic!("expected SetStoreInfo"), + LogMsg::ArrowMsg { .. } => panic!("expected SetStoreInfo"), } // MemorySinkStorage transparently handles flushing during `take()`! @@ -1588,7 +1588,7 @@ mod tests { similar_asserts::assert_eq!(table, got); } - _ => panic!("expected ArrowMsg"), + LogMsg::SetStoreInfo { .. } => panic!("expected ArrowMsg"), } // That's all. diff --git a/crates/re_sdk_comms/src/server.rs b/crates/re_sdk_comms/src/server.rs index 084e175f5e8b..7254f782a024 100644 --- a/crates/re_sdk_comms/src/server.rs +++ b/crates/re_sdk_comms/src/server.rs @@ -249,7 +249,7 @@ impl CongestionManager { #[allow(clippy::match_same_arms)] match msg { // we don't want to drop any of these - LogMsg::SetStoreInfo(_) | LogMsg::EntityPathOpMsg(_, _) => true, + LogMsg::SetStoreInfo(_) => true, LogMsg::ArrowMsg(_, arrow_msg) => self.should_send_time_point(&arrow_msg.timepoint_max), } diff --git a/crates/re_viewer/src/saving.rs b/crates/re_viewer/src/saving.rs index 08bbd79d062d..2a8a8510e69a 100644 --- a/crates/re_viewer/src/saving.rs +++ b/crates/re_viewer/src/saving.rs @@ -81,7 +81,6 @@ pub fn save_database_to_file( path: std::path::PathBuf, time_selection: Option<(re_data_store::Timeline, re_log_types::TimeRangeF)>, ) -> anyhow::Result anyhow::Result> { - use itertools::Itertools as _; use re_arrow_store::TimeRange; re_tracing::profile_function!(); @@ -90,11 +89,6 @@ pub fn save_database_to_file( .store_info_msg() .map(|msg| LogMsg::SetStoreInfo(msg.clone())); - let ent_op_msgs = store_db - .iter_entity_op_msgs() - .map(|msg| LogMsg::EntityPathOpMsg(store_db.store_id().clone(), msg.clone())) - .collect_vec(); - let time_filter = time_selection.map(|(timeline, range)| { ( timeline, @@ -117,7 +111,6 @@ pub fn save_database_to_file( let msgs = std::iter::once(set_store_info_msg) .flatten() // option - .chain(ent_op_msgs) .chain(data_msgs); Ok(move || {