From a576e6d522754f00301e5e3e08e4155508909467 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Thu, 12 Oct 2023 11:00:06 +0200 Subject: [PATCH 1/7] Don't tuch `StoreDb::entity_db` directly as often --- crates/re_data_store/src/store_db.rs | 35 +++++++++++++++---- crates/re_data_ui/src/annotation_context.rs | 3 +- crates/re_data_ui/src/component_path.rs | 4 +-- crates/re_data_ui/src/image.rs | 3 +- crates/re_data_ui/src/image_meaning.rs | 2 +- crates/re_data_ui/src/instance_path.rs | 4 +-- crates/re_data_ui/src/item_ui.rs | 2 +- .../src/view_part_system.rs | 2 +- .../src/contexts/depth_offsets.rs | 2 +- .../src/contexts/transform_context.rs | 2 +- .../re_space_view_spatial/src/heuristics.rs | 4 +-- .../src/parts/entity_iterator.rs | 4 +-- .../src/parts/transform3d_arrows.rs | 2 +- crates/re_space_view_spatial/src/ui.rs | 2 +- .../src/view_part_system.rs | 2 +- .../src/view_part_system.rs | 2 +- .../src/space_view_class.rs | 7 +--- .../src/view_part_system.rs | 2 +- .../src/view_part_system.rs | 2 +- crates/re_time_panel/src/lib.rs | 8 ++--- crates/re_viewer/src/saving.rs | 3 +- crates/re_viewer/src/store_hub.rs | 18 ++++------ crates/re_viewer/src/ui/selection_panel.rs | 4 +-- crates/re_viewer_context/src/annotations.rs | 2 +- .../src/space_view_entity_picker.rs | 4 +-- .../re_viewport/src/space_view_heuristics.rs | 4 +-- crates/re_viewport/src/viewport_blueprint.rs | 2 +- .../src/color_coordinates_view_part_system.rs | 2 +- examples/rust/extend_viewer_ui/src/main.rs | 10 ++---- 29 files changed, 73 insertions(+), 70 deletions(-) diff --git a/crates/re_data_store/src/store_db.rs b/crates/re_data_store/src/store_db.rs index 4d47551abd6a..e22e8fdf8432 100644 --- a/crates/re_data_store/src/store_db.rs +++ b/crates/re_data_store/src/store_db.rs @@ -15,6 +15,8 @@ use crate::{Error, TimesPerTimeline}; // ---------------------------------------------------------------------------- /// Stored entities with easy indexing of the paths. +/// +/// NOTE: don't go mutating the contents of this. Use the public functions instead. pub struct EntityDb { /// In many places we just store the hashes, so we need a way to translate back. pub entity_path_from_hash: IntMap, @@ -226,6 +228,8 @@ impl EntityDb { // ---------------------------------------------------------------------------- /// A in-memory database built from a stream of [`LogMsg`]es. +/// +/// NOTE: all mutation is to be done via public functions! pub struct StoreDb { /// The [`StoreId`] for this log. store_id: StoreId, @@ -254,6 +258,11 @@ impl StoreDb { } } + #[inline] + pub fn entity_db(&self) -> &EntityDb { + &self.entity_db + } + pub fn recording_msg(&self) -> Option<&SetStoreInfo> { self.recording_msg.as_ref() } @@ -266,11 +275,6 @@ impl StoreDb { self.store_info().map(|ri| &ri.application_id) } - #[inline] - pub fn store_mut(&mut self) -> &mut re_arrow_store::DataStore { - &mut self.entity_db.data_store - } - #[inline] pub fn store(&self) -> &re_arrow_store::DataStore { &self.entity_db.data_store @@ -346,12 +350,23 @@ impl StoreDb { self.entity_op_msgs.get(row_id) } + pub fn gc_everything_but_the_latest_row(&mut self) { + re_tracing::profile_function!(); + + self.gc(GarbageCollectionOptions { + target: re_arrow_store::GarbageCollectionTarget::Everything, + gc_timeless: true, + protect_latest: 1, // TODO(jleibs): Bump this after we have an undo buffer + purge_empty_tables: true, + }); + } + /// Free up some RAM by forgetting the older parts of all timelines. pub fn purge_fraction_of_ram(&mut self, fraction_to_purge: f32) { re_tracing::profile_function!(); - assert!((0.0..=1.0).contains(&fraction_to_purge)); - let (deleted, stats_diff) = self.entity_db.data_store.gc(GarbageCollectionOptions { + assert!((0.0..=1.0).contains(&fraction_to_purge)); + self.gc(GarbageCollectionOptions { target: re_arrow_store::GarbageCollectionTarget::DropAtLeastFraction( fraction_to_purge as _, ), @@ -359,6 +374,12 @@ impl StoreDb { protect_latest: 1, purge_empty_tables: false, }); + } + + pub fn gc(&mut self, gc_options: GarbageCollectionOptions) { + re_tracing::profile_function!(); + + let (deleted, stats_diff) = self.entity_db.data_store.gc(gc_options); re_log::trace!( num_row_ids_dropped = deleted.row_ids.len(), size_bytes_dropped = re_format::format_bytes(stats_diff.total.num_bytes as _), diff --git a/crates/re_data_ui/src/annotation_context.rs b/crates/re_data_ui/src/annotation_context.rs index 0a2192c53730..ac8c9e5427bf 100644 --- a/crates/re_data_ui/src/annotation_context.rs +++ b/crates/re_data_ui/src/annotation_context.rs @@ -87,8 +87,7 @@ fn annotation_info( ) -> Option { let class_id = ctx .store_db - .entity_db - .data_store + .store() .query_latest_component::(entity_path, query)?; let annotations = crate::annotations(ctx, query, entity_path); let class = annotations.resolved_class_description(Some(*class_id)); diff --git a/crates/re_data_ui/src/component_path.rs b/crates/re_data_ui/src/component_path.rs index f9f123bce699..518a8fc69777 100644 --- a/crates/re_data_ui/src/component_path.rs +++ b/crates/re_data_ui/src/component_path.rs @@ -16,7 +16,7 @@ impl DataUi for ComponentPath { component_name, } = self; - let store = &ctx.store_db.entity_db.data_store; + let store = ctx.store_db.store(); if let Some(archetype_name) = component_name.indicator_component_archetype() { ui.label(format!( @@ -30,7 +30,7 @@ impl DataUi for ComponentPath { component_data, } .data_ui(ctx, ui, verbosity, query); - } else if let Some(entity_tree) = ctx.store_db.entity_db.tree.subtree(entity_path) { + } else if let Some(entity_tree) = ctx.store_db.entity_db().tree.subtree(entity_path) { if entity_tree.components.contains_key(component_name) { ui.label(""); } else { diff --git a/crates/re_data_ui/src/image.rs b/crates/re_data_ui/src/image.rs index e645c8a9ef58..dbffece6e09d 100644 --- a/crates/re_data_ui/src/image.rs +++ b/crates/re_data_ui/src/image.rs @@ -33,8 +33,7 @@ impl EntityDataUi for re_types::components::TensorData { let tensor_data_row_id = ctx .store_db - .entity_db - .data_store + .store() .query_latest_component::(entity_path, query) .map_or(RowId::ZERO, |tensor| tensor.row_id); diff --git a/crates/re_data_ui/src/image_meaning.rs b/crates/re_data_ui/src/image_meaning.rs index 4623f1befd90..3e4afe445683 100644 --- a/crates/re_data_ui/src/image_meaning.rs +++ b/crates/re_data_ui/src/image_meaning.rs @@ -10,7 +10,7 @@ pub fn image_meaning_for_entity( entity_path: &EntityPath, ctx: &ViewerContext<'_>, ) -> TensorDataMeaning { - let store = &ctx.store_db.entity_db.data_store; + let store = ctx.store_db.store(); let timeline = &ctx.current_query().timeline; if store.entity_has_component(timeline, entity_path, &DepthImage::indicator().name()) { TensorDataMeaning::Depth diff --git a/crates/re_data_ui/src/instance_path.rs b/crates/re_data_ui/src/instance_path.rs index 85bc0950074a..8b92e1b2a6a7 100644 --- a/crates/re_data_ui/src/instance_path.rs +++ b/crates/re_data_ui/src/instance_path.rs @@ -19,10 +19,10 @@ impl DataUi for InstancePath { instance_key, } = self; - let store = &ctx.store_db.entity_db.data_store; + let store = ctx.store_db.store(); let Some(components) = store.all_components(&query.timeline, entity_path) else { - if ctx.store_db.entity_db.is_known_entity(entity_path) { + if ctx.store_db.entity_db().is_known_entity(entity_path) { ui.label(ctx.re_ui.warning_text(format!( "No components logged on timeline {:?}", query.timeline.name() diff --git a/crates/re_data_ui/src/item_ui.rs b/crates/re_data_ui/src/item_ui.rs index 961531860089..347e00c71d08 100644 --- a/crates/re_data_ui/src/item_ui.rs +++ b/crates/re_data_ui/src/item_ui.rs @@ -342,7 +342,7 @@ pub fn instance_hover_card_ui( let query = ctx.current_query(); if instance_path.instance_key.is_splat() { - let store = &ctx.store_db.entity_db.data_store; + let store = ctx.store_db.store(); let stats = store.entity_stats(query.timeline, instance_path.entity_path.hash()); entity_stats_ui(ui, &query.timeline, &stats); } else { diff --git a/crates/re_space_view_bar_chart/src/view_part_system.rs b/crates/re_space_view_bar_chart/src/view_part_system.rs index 82773f98b467..f689ecd99836 100644 --- a/crates/re_space_view_bar_chart/src/view_part_system.rs +++ b/crates/re_space_view_bar_chart/src/view_part_system.rs @@ -72,7 +72,7 @@ impl ViewPartSystem for BarChartViewPartSystem { ) -> Result, SpaceViewSystemExecutionError> { re_tracing::profile_function!(); - let store = &ctx.store_db.entity_db.data_store; + let store = ctx.store_db.store(); for (ent_path, _props) in query.iter_entities_for_system(Self::name()) { let query = LatestAtQuery::new(query.timeline, query.latest_at); diff --git a/crates/re_space_view_spatial/src/contexts/depth_offsets.rs b/crates/re_space_view_spatial/src/contexts/depth_offsets.rs index 689f1588d8f6..4f1e2f323bf3 100644 --- a/crates/re_space_view_spatial/src/contexts/depth_offsets.rs +++ b/crates/re_space_view_spatial/src/contexts/depth_offsets.rs @@ -44,7 +44,7 @@ impl ViewContextSystem for EntityDepthOffsets { DefaultPoints, } - let store = &ctx.store_db.entity_db.data_store; + let store = ctx.store_db.store(); // Use a BTreeSet for entity hashes to get a stable order. let mut entities_per_draw_order = BTreeMap::>::new(); diff --git a/crates/re_space_view_spatial/src/contexts/transform_context.rs b/crates/re_space_view_spatial/src/contexts/transform_context.rs index db766978492d..a050da0dc8f4 100644 --- a/crates/re_space_view_spatial/src/contexts/transform_context.rs +++ b/crates/re_space_view_spatial/src/contexts/transform_context.rs @@ -83,7 +83,7 @@ impl ViewContextSystem for TransformContext { ) { re_tracing::profile_function!(); - let entity_db = &ctx.store_db.entity_db; + let entity_db = ctx.store_db.entity_db(); let time_ctrl = &ctx.rec_cfg.time_ctrl; let entity_prop_map = query.entity_props_map; diff --git a/crates/re_space_view_spatial/src/heuristics.rs b/crates/re_space_view_spatial/src/heuristics.rs index 136d1b98c0de..7db85eff9e52 100644 --- a/crates/re_space_view_spatial/src/heuristics.rs +++ b/crates/re_space_view_spatial/src/heuristics.rs @@ -153,7 +153,7 @@ fn update_depth_cloud_property_heuristics( .get(&ImagesPart::name()) .unwrap_or(&BTreeSet::new()) { - let store = &ctx.store_db.entity_db.data_store; + let store = ctx.store_db.store(); let Some(tensor) = store.query_latest_component::(ent_path, &ctx.current_query()) else { @@ -213,7 +213,7 @@ fn update_transform3d_lines_heuristics( return Some(ent_path); } else { // Any direct child has a pinhole camera? - if let Some(child_tree) = ctx.store_db.entity_db.tree.subtree(ent_path) { + if let Some(child_tree) = ctx.store_db.entity_db().tree.subtree(ent_path) { for child in child_tree.children.values() { if query_pinhole(store, &ctx.current_query(), &child.path).is_some() { return Some(&child.path); diff --git a/crates/re_space_view_spatial/src/parts/entity_iterator.rs b/crates/re_space_view_spatial/src/parts/entity_iterator.rs index 6879ee7e1c77..848e3c262dd1 100644 --- a/crates/re_space_view_spatial/src/parts/entity_iterator.rs +++ b/crates/re_space_view_spatial/src/parts/entity_iterator.rs @@ -49,7 +49,7 @@ where } else { transforms.reference_from_entity_ignoring_pinhole( ent_path, - &ctx.store_db.entity_db.data_store, + ctx.store_db.store(), &query.latest_at_query(), ) }; @@ -70,7 +70,7 @@ where }; match query_archetype_with_history::( - &ctx.store_db.entity_db.data_store, + ctx.store_db.store(), &query.timeline, &query.latest_at, &props.visible_history, diff --git a/crates/re_space_view_spatial/src/parts/transform3d_arrows.rs b/crates/re_space_view_spatial/src/parts/transform3d_arrows.rs index 183ce6861229..ef57b7a26311 100644 --- a/crates/re_space_view_spatial/src/parts/transform3d_arrows.rs +++ b/crates/re_space_view_spatial/src/parts/transform3d_arrows.rs @@ -57,7 +57,7 @@ impl ViewPartSystem for Transform3DArrowsPart { let mut line_builder = view_ctx.get::()?.lines(); let transforms = view_ctx.get::()?; - let store = &ctx.store_db.entity_db.data_store; + let store = ctx.store_db.store(); let latest_at_query = re_arrow_store::LatestAtQuery::new(query.timeline, query.latest_at); for (ent_path, props) in query.iter_entities_for_system(Self::name()) { if store diff --git a/crates/re_space_view_spatial/src/ui.rs b/crates/re_space_view_spatial/src/ui.rs index 33d47b78b201..c582e1717e33 100644 --- a/crates/re_space_view_spatial/src/ui.rs +++ b/crates/re_space_view_spatial/src/ui.rs @@ -518,7 +518,7 @@ pub fn picking( // TODO(#1818): Depth at pointer only works for depth images so far. let mut depth_at_pointer = None; for hit in &picking_result.hits { - let Some(mut instance_path) = hit.instance_path_hash.resolve(&ctx.store_db.entity_db) + let Some(mut instance_path) = hit.instance_path_hash.resolve(ctx.store_db.entity_db()) else { continue; }; diff --git a/crates/re_space_view_tensor/src/view_part_system.rs b/crates/re_space_view_tensor/src/view_part_system.rs index dfe83ea8244c..975198cee632 100644 --- a/crates/re_space_view_tensor/src/view_part_system.rs +++ b/crates/re_space_view_tensor/src/view_part_system.rs @@ -62,7 +62,7 @@ impl ViewPartSystem for TensorSystem { ) -> Result, SpaceViewSystemExecutionError> { re_tracing::profile_function!(); - let store = &ctx.store_db.entity_db.data_store; + let store = ctx.store_db.store(); for (ent_path, props) in query.iter_entities_for_system(Self::name()) { let timeline_query = LatestAtQuery::new(query.timeline, query.latest_at); diff --git a/crates/re_space_view_text_document/src/view_part_system.rs b/crates/re_space_view_text_document/src/view_part_system.rs index f3df825aa1d6..43a09e82b444 100644 --- a/crates/re_space_view_text_document/src/view_part_system.rs +++ b/crates/re_space_view_text_document/src/view_part_system.rs @@ -47,7 +47,7 @@ impl ViewPartSystem for TextDocumentSystem { query: &ViewQuery<'_>, _view_ctx: &ViewContextCollection, ) -> Result, SpaceViewSystemExecutionError> { - let store = &ctx.store_db.entity_db.data_store; + let store = ctx.store_db.store(); let timeline_query = LatestAtQuery::new(query.timeline, query.latest_at); diff --git a/crates/re_space_view_text_log/src/space_view_class.rs b/crates/re_space_view_text_log/src/space_view_class.rs index f96b67652f6b..84866e61723f 100644 --- a/crates/re_space_view_text_log/src/space_view_class.rs +++ b/crates/re_space_view_text_log/src/space_view_class.rs @@ -248,12 +248,7 @@ impl ViewTextFilters { // --- fn get_time_point(ctx: &ViewerContext<'_>, entry: &Entry) -> Option { - if let Some(time_point) = ctx - .store_db - .entity_db - .data_store - .get_msg_metadata(&entry.row_id) - { + if let Some(time_point) = ctx.store_db.store().get_msg_metadata(&entry.row_id) { Some(time_point.clone()) } else { re_log::warn_once!("Missing meta-data for {:?}", entry.entity_path); diff --git a/crates/re_space_view_text_log/src/view_part_system.rs b/crates/re_space_view_text_log/src/view_part_system.rs index f111451b4ea1..8a587890e42e 100644 --- a/crates/re_space_view_text_log/src/view_part_system.rs +++ b/crates/re_space_view_text_log/src/view_part_system.rs @@ -59,7 +59,7 @@ impl ViewPartSystem for TextLogSystem { query: &ViewQuery<'_>, _view_ctx: &ViewContextCollection, ) -> Result, SpaceViewSystemExecutionError> { - let store = &ctx.store_db.entity_db.data_store; + let store = ctx.store_db.store(); for (ent_path, _) in query.iter_entities_for_system(Self::name()) { // We want everything, for all times: diff --git a/crates/re_space_view_time_series/src/view_part_system.rs b/crates/re_space_view_time_series/src/view_part_system.rs index 46d8976034c4..eb4fce58629f 100644 --- a/crates/re_space_view_time_series/src/view_part_system.rs +++ b/crates/re_space_view_time_series/src/view_part_system.rs @@ -115,7 +115,7 @@ impl TimeSeriesSystem { ) -> Result<(), QueryError> { re_tracing::profile_function!(); - let store = &ctx.store_db.entity_db.data_store; + let store = ctx.store_db.store(); for (ent_path, _ent_props) in query.iter_entities_for_system(Self::name()) { let mut points = Vec::new(); diff --git a/crates/re_time_panel/src/lib.rs b/crates/re_time_panel/src/lib.rs index 92d35e3b4ec5..1f1f61c05a26 100644 --- a/crates/re_time_panel/src/lib.rs +++ b/crates/re_time_panel/src/lib.rs @@ -390,7 +390,7 @@ impl TimePanel { time_area_painter, tree_max_y, None, - &ctx.store_db.entity_db.tree, + &ctx.store_db.entity_db().tree, ui, ); } else { @@ -399,7 +399,7 @@ impl TimePanel { time_area_response, time_area_painter, tree_max_y, - &ctx.store_db.entity_db.tree, + &ctx.store_db.entity_db().tree, ui, ); } @@ -777,7 +777,7 @@ fn is_time_safe_to_show( return true; // no timeless messages, no problem } - if let Some(times) = store_db.entity_db.tree.prefix_times.get(timeline) { + if let Some(times) = store_db.entity_db().tree.prefix_times.get(timeline) { if let Some(first_time) = times.min_key() { let margin = match timeline.typ() { re_arrow_store::TimeType::Time => TimeInt::from_seconds(10_000), @@ -822,7 +822,7 @@ fn initialize_time_ranges_ui( if let Some(times) = ctx .store_db - .entity_db + .entity_db() .tree .prefix_times .get(ctx.rec_cfg.time_ctrl.timeline()) diff --git a/crates/re_viewer/src/saving.rs b/crates/re_viewer/src/saving.rs index 6af32a17c85f..137b4a97ffd5 100644 --- a/crates/re_viewer/src/saving.rs +++ b/crates/re_viewer/src/saving.rs @@ -102,8 +102,7 @@ pub fn save_database_to_file( ) }); let data_msgs: Result, _> = store_db - .entity_db - .data_store + .store() .to_data_tables(time_filter) .map(|table| { table diff --git a/crates/re_viewer/src/store_hub.rs b/crates/re_viewer/src/store_hub.rs index acd64a7d4b89..52996debe0d2 100644 --- a/crates/re_viewer/src/store_hub.rs +++ b/crates/re_viewer/src/store_hub.rs @@ -228,17 +228,11 @@ impl StoreHub { // save the blueprints referenced by `blueprint_by_app_id`, even though // there may be other Blueprints in the Hub. - use re_arrow_store::GarbageCollectionOptions; for (app_id, blueprint_id) in &self.blueprint_by_app_id { if let Some(blueprint) = self.store_dbs.blueprint_mut(blueprint_id) { if self.blueprint_last_save.get(blueprint_id) != Some(&blueprint.generation()) { - // GC everything but the latest row. - blueprint.store_mut().gc(GarbageCollectionOptions { - target: re_arrow_store::GarbageCollectionTarget::Everything, - gc_timeless: true, - protect_latest: 1, // TODO(jleibs): Bump this after we have an undo buffer - purge_empty_tables: true, - }); + blueprint.gc_everything_but_the_latest_row(); + #[cfg(not(target_arch = "wasm32"))] { let blueprint_path = default_blueprint_path(app_id)?; @@ -314,11 +308,11 @@ impl StoreHub { .and_then(|blueprint_id| self.store_dbs.blueprint(blueprint_id)); let blueprint_stats = blueprint - .map(|store_db| DataStoreStats::from_store(&store_db.entity_db.data_store)) + .map(|store_db| DataStoreStats::from_store(store_db.store())) .unwrap_or_default(); let blueprint_config = blueprint - .map(|store_db| store_db.entity_db.data_store.config().clone()) + .map(|store_db| store_db.store().config().clone()) .unwrap_or_default(); let recording = self @@ -327,11 +321,11 @@ impl StoreHub { .and_then(|rec_id| self.store_dbs.recording(rec_id)); let recording_stats = recording - .map(|store_db| DataStoreStats::from_store(&store_db.entity_db.data_store)) + .map(|store_db| DataStoreStats::from_store(store_db.store())) .unwrap_or_default(); let recording_config = recording - .map(|store_db| store_db.entity_db.data_store.config().clone()) + .map(|store_db| store_db.store().config().clone()) .unwrap_or_default(); StoreHubStats { diff --git a/crates/re_viewer/src/ui/selection_panel.rs b/crates/re_viewer/src/ui/selection_panel.rs index e0cff0df72fe..6351ce4f4cd4 100644 --- a/crates/re_viewer/src/ui/selection_panel.rs +++ b/crates/re_viewer/src/ui/selection_panel.rs @@ -450,7 +450,7 @@ fn pinhole_props_ui( entity_props: &mut EntityProperties, ) { let query = ctx.current_query(); - let store = &ctx.store_db.entity_db.data_store; + let store = ctx.store_db.store(); if store .query_latest_component::(entity_path, &query) .is_some() @@ -482,7 +482,7 @@ fn depth_props_ui( re_tracing::profile_function!(); let query = ctx.current_query(); - let store = &ctx.store_db.entity_db.data_store; + let store = ctx.store_db.store(); let meaning = image_meaning_for_entity(entity_path, ctx); diff --git a/crates/re_viewer_context/src/annotations.rs b/crates/re_viewer_context/src/annotations.rs index ba4e28cedace..d9c7c1610615 100644 --- a/crates/re_viewer_context/src/annotations.rs +++ b/crates/re_viewer_context/src/annotations.rs @@ -235,7 +235,7 @@ impl AnnotationMap { let mut visited = IntSet::::default(); - let data_store = &ctx.store_db.entity_db.data_store; + let data_store = ctx.store_db.store(); // This logic is borrowed from `iter_ancestor_meta_field`, but using the arrow-store instead // not made generic as `AnnotationContext` was the only user of that function diff --git a/crates/re_viewport/src/space_view_entity_picker.rs b/crates/re_viewport/src/space_view_entity_picker.rs index 839e3dbfbf5d..c2f1775ac433 100644 --- a/crates/re_viewport/src/space_view_entity_picker.rs +++ b/crates/re_viewport/src/space_view_entity_picker.rs @@ -82,8 +82,8 @@ fn add_entities_ui( ui: &mut egui::Ui, space_view: &mut SpaceViewBlueprint, ) { - let spaces_info = SpaceInfoCollection::new(&ctx.store_db.entity_db); - let tree = &ctx.store_db.entity_db.tree; + let spaces_info = SpaceInfoCollection::new(ctx.store_db.entity_db()); + let tree = &ctx.store_db.entity_db().tree; let entities_add_info = create_entity_add_info(ctx, tree, space_view, &spaces_info); add_entities_tree_ui( diff --git a/crates/re_viewport/src/space_view_heuristics.rs b/crates/re_viewport/src/space_view_heuristics.rs index 2ed322988919..ad0dcf973958 100644 --- a/crates/re_viewport/src/space_view_heuristics.rs +++ b/crates/re_viewport/src/space_view_heuristics.rs @@ -44,7 +44,7 @@ fn candidate_space_view_paths<'a>( ) -> impl Iterator { // Everything with a SpaceInfo is a candidate (that is root + whenever there is a transform), // as well as all direct descendants of the root. - let root_children = &ctx.store_db.entity_db.tree.children; + let root_children = &ctx.store_db.entity_db().tree.children; spaces_info .iter() .map(|info| &info.path) @@ -499,7 +499,7 @@ pub fn identify_entities_per_system_per_class( let mut entities_per_system_per_class = EntitiesPerSystemPerClass::default(); let store = ctx.store_db.store(); - for ent_path in ctx.store_db.entity_db.entity_paths() { + for ent_path in ctx.store_db.entity_db().entity_paths() { let Some(components) = store.all_components(&re_log_types::Timeline::log_time(), ent_path) else { continue; diff --git a/crates/re_viewport/src/viewport_blueprint.rs b/crates/re_viewport/src/viewport_blueprint.rs index 1674dc86bf0f..d4d7139b32b6 100644 --- a/crates/re_viewport/src/viewport_blueprint.rs +++ b/crates/re_viewport/src/viewport_blueprint.rs @@ -361,7 +361,7 @@ pub fn load_viewport_blueprint(blueprint_db: &re_data_store::StoreDb) -> Viewpor let space_views: HashMap = if let Some(space_views) = blueprint_db - .entity_db + .entity_db() .tree .children .get(&re_data_store::EntityPathPart::Name( diff --git a/examples/rust/custom_space_view/src/color_coordinates_view_part_system.rs b/examples/rust/custom_space_view/src/color_coordinates_view_part_system.rs index 42b08d2fbdc9..89039d3b2055 100644 --- a/examples/rust/custom_space_view/src/color_coordinates_view_part_system.rs +++ b/examples/rust/custom_space_view/src/color_coordinates_view_part_system.rs @@ -62,7 +62,7 @@ impl ViewPartSystem for InstanceColorSystem { for (ent_path, _props) in query.iter_entities_for_system(InstanceColorSystem::name()) { // ...gather all colors and their instance ids. if let Ok(arch_view) = query_archetype::( - &ctx.store_db.entity_db.data_store, + ctx.store_db.store(), &ctx.current_query(), ent_path, ) { diff --git a/examples/rust/extend_viewer_ui/src/main.rs b/examples/rust/extend_viewer_ui/src/main.rs index ed36956af853..4e1863aab20c 100644 --- a/examples/rust/extend_viewer_ui/src/main.rs +++ b/examples/rust/extend_viewer_ui/src/main.rs @@ -124,7 +124,7 @@ fn store_db_ui(ui: &mut egui::Ui, store_db: &re_data_store::StoreDb) { egui::ScrollArea::vertical() .auto_shrink([false, true]) .show(ui, |ui| { - for entity_path in store_db.entity_db.entity_paths() { + for entity_path in store_db.entity_db().entity_paths() { ui.collapsing(entity_path.to_string(), |ui| { entity_ui(ui, store_db, timeline, entity_path); }); @@ -139,11 +139,7 @@ fn entity_ui( entity_path: &re_log_types::EntityPath, ) { // Each entity can have many components (e.g. position, color, radius, …): - if let Some(mut components) = store_db - .entity_db - .data_store - .all_components(&timeline, entity_path) - { + if let Some(mut components) = store_db.store().all_components(&timeline, entity_path) { components.sort(); // Make the order predicatable for component in components { ui.collapsing(component.to_string(), |ui| { @@ -165,7 +161,7 @@ fn component_ui( let query = re_arrow_store::LatestAtQuery::latest(timeline); if let Some((_, component)) = re_query::get_component_with_instances( - &store_db.entity_db.data_store, + store_db.store(), &query, entity_path, component_name, From c969c4ce6eed15c2c0e2f8ae0855704f5e30a4cb Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Thu, 12 Oct 2023 11:01:35 +0200 Subject: [PATCH 2/7] Add TODO --- crates/re_data_store/src/store_db.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/re_data_store/src/store_db.rs b/crates/re_data_store/src/store_db.rs index e22e8fdf8432..7e1ff2e1bcf0 100644 --- a/crates/re_data_store/src/store_db.rs +++ b/crates/re_data_store/src/store_db.rs @@ -244,7 +244,7 @@ pub struct StoreDb { recording_msg: Option, /// Where we store the entities. - pub entity_db: EntityDb, + pub entity_db: EntityDb, // TODO(emilk): remove `pub` } impl StoreDb { From f2eb8bcc751558a242ae569fc042ad55014ee7fe Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Thu, 12 Oct 2023 11:14:29 +0200 Subject: [PATCH 3/7] All data rows must now be added via StoreDb public functions --- crates/re_data_store/src/store_db.rs | 88 ++++++++++++++------------- crates/re_viewer/src/app.rs | 2 +- crates/re_viewer/src/app_blueprint.rs | 5 +- 3 files changed, 49 insertions(+), 46 deletions(-) diff --git a/crates/re_data_store/src/store_db.rs b/crates/re_data_store/src/store_db.rs index 7e1ff2e1bcf0..7c56c53eaaf5 100644 --- a/crates/re_data_store/src/store_db.rs +++ b/crates/re_data_store/src/store_db.rs @@ -4,9 +4,9 @@ use nohash_hasher::IntMap; use re_arrow_store::{DataStoreConfig, GarbageCollectionOptions}; use re_log_types::{ - ApplicationId, ArrowMsg, ComponentPath, DataCell, DataRow, DataTable, EntityPath, - EntityPathHash, EntityPathOpMsg, LogMsg, PathOp, RowId, SetStoreInfo, StoreId, StoreInfo, - StoreKind, TimePoint, Timeline, + ApplicationId, ComponentPath, DataCell, DataRow, DataTable, EntityPath, EntityPathHash, + EntityPathOpMsg, LogMsg, PathOp, RowId, SetStoreInfo, StoreId, StoreInfo, StoreKind, TimePoint, + Timeline, }; use re_types::{components::InstanceKey, Loggable as _}; @@ -76,45 +76,12 @@ impl EntityDb { .or_insert_with(|| entity_path.clone()); } - fn try_add_arrow_msg(&mut self, msg: &ArrowMsg) -> Result<(), Error> { - re_tracing::profile_function!(); - - // TODO(#1760): Compute the size of the datacells in the batching threads on the clients. - let mut table = DataTable::from_arrow_msg(msg)?; - table.compute_all_size_bytes(); - - // TODO(cmc): batch all of this - for row in table.to_rows() { - let row = row?; - - self.register_entity_path(&row.entity_path); - - self.try_add_data_row(&row)?; - - // Look for a `ClearIsRecursive` component, and if it's there, go through the clear path - // instead. - use re_types::components::ClearIsRecursive; - if let Some(idx) = row.find_cell(&ClearIsRecursive::name()) { - let cell = &row.cells()[idx]; - let settings = cell.try_to_native_mono::().unwrap(); - let path_op = if settings.map_or(false, |s| s.0) { - PathOp::ClearRecursive(row.entity_path.clone()) - } else { - PathOp::ClearComponents(row.entity_path.clone()) - }; - // NOTE: We've just added the row itself, so make sure to bump the row ID already! - self.add_path_op(row.row_id().next(), row.timepoint(), &path_op); - } - } - - Ok(()) - } - - // TODO(jleibs): If this shouldn't be public, chain together other setters // TODO(cmc): Updates of secondary datastructures should be the result of subscribing to the // datastore's changelog and reacting to these changes appropriately. We shouldn't be creating // many sources of truth. - pub fn try_add_data_row(&mut self, row: &DataRow) -> Result<(), Error> { + fn add_data_row(&mut self, row: &DataRow) -> Result<(), Error> { + self.register_entity_path(&row.entity_path); + for (&timeline, &time_int) in row.timepoint().iter() { self.times_per_timeline.insert(timeline, time_int); } @@ -146,7 +113,24 @@ impl EntityDb { } } - self.data_store.insert_row(row).map_err(Into::into) + self.data_store.insert_row(row)?; + + // Look for a `ClearIsRecursive` component, and if it's there, go through the clear path + // instead. + use re_types::components::ClearIsRecursive; + if let Some(idx) = row.find_cell(&ClearIsRecursive::name()) { + let cell = &row.cells()[idx]; + let settings = cell.try_to_native_mono::().unwrap(); + let path_op = if settings.map_or(false, |s| s.0) { + PathOp::ClearRecursive(row.entity_path.clone()) + } else { + PathOp::ClearComponents(row.entity_path.clone()) + }; + // NOTE: We've just added the row itself, so make sure to bump the row ID already! + self.add_path_op(row.row_id().next(), row.timepoint(), &path_op); + } + + Ok(()) } fn add_path_op(&mut self, row_id: RowId, time_point: &TimePoint, path_op: &PathOp) { @@ -322,6 +306,7 @@ impl StoreDb { match &msg { LogMsg::SetStoreInfo(msg) => self.add_begin_recording_msg(msg), + LogMsg::EntityPathOpMsg(_, msg) => { let EntityPathOpMsg { row_id, @@ -331,12 +316,33 @@ impl StoreDb { self.entity_op_msgs.insert(*row_id, msg.clone()); self.entity_db.add_path_op(*row_id, time_point, path_op); } - LogMsg::ArrowMsg(_, inner) => self.entity_db.try_add_arrow_msg(inner)?, + + LogMsg::ArrowMsg(_, arrow_msg) => { + let table = DataTable::from_arrow_msg(arrow_msg)?; + self.add_data_table(table)?; + } } Ok(()) } + pub fn add_data_table(&mut self, mut table: DataTable) -> Result<(), Error> { + // TODO(#1760): Compute the size of the datacells in the batching threads on the clients. + table.compute_all_size_bytes(); + + // TODO(cmc): batch all of this + for row in table.to_rows() { + let row = row?; + self.add_data_row(&row)?; + } + + Ok(()) + } + + pub fn add_data_row(&mut self, row: &DataRow) -> Result<(), Error> { + self.entity_db.add_data_row(row) + } + pub fn add_begin_recording_msg(&mut self, msg: &SetStoreInfo) { self.recording_msg = Some(msg.clone()); } diff --git a/crates/re_viewer/src/app.rs b/crates/re_viewer/src/app.rs index 9f6b084dff76..f079e4b4f122 100644 --- a/crates/re_viewer/src/app.rs +++ b/crates/re_viewer/src/app.rs @@ -353,7 +353,7 @@ impl App { SystemCommand::UpdateBlueprint(blueprint_id, updates) => { let blueprint_db = store_hub.store_db_mut(&blueprint_id); for row in updates { - match blueprint_db.entity_db.try_add_data_row(&row) { + match blueprint_db.add_data_row(&row) { Ok(()) => {} Err(err) => { re_log::warn_once!("Failed to store blueprint delta: {err}"); diff --git a/crates/re_viewer/src/app_blueprint.rs b/crates/re_viewer/src/app_blueprint.rs index fe1d2f743685..89833defc0d9 100644 --- a/crates/re_viewer/src/app_blueprint.rs +++ b/crates/re_viewer/src/app_blueprint.rs @@ -95,10 +95,7 @@ pub fn setup_welcome_screen_blueprint(welcome_screen_blueprint: &mut StoreDb) { DataRow::from_cells1_sized(RowId::random(), entity_path, timepoint, 1, [component]) .unwrap(); // Can only fail if we have the wrong number of instances for the component, and we don't - welcome_screen_blueprint - .entity_db - .try_add_data_row(&row) - .unwrap(); // Can only fail if we have the wrong number of instances for the component, and we don't + welcome_screen_blueprint.add_data_row(&row).unwrap(); // Can only fail if we have the wrong number of instances for the component, and we don't } } From f491decd097a910297afe463417f2896fbdc01c6 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Thu, 12 Oct 2023 11:16:05 +0200 Subject: [PATCH 4/7] Make `SoreDb::entity_db` private --- crates/re_data_store/src/store_db.rs | 2 +- crates/re_viewer/src/app.rs | 2 +- crates/re_viewer/src/app_state.rs | 2 +- crates/rerun/src/run.rs | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/re_data_store/src/store_db.rs b/crates/re_data_store/src/store_db.rs index 7c56c53eaaf5..cb66ad004857 100644 --- a/crates/re_data_store/src/store_db.rs +++ b/crates/re_data_store/src/store_db.rs @@ -228,7 +228,7 @@ pub struct StoreDb { recording_msg: Option, /// Where we store the entities. - pub entity_db: EntityDb, // TODO(emilk): remove `pub` + entity_db: EntityDb, } impl StoreDb { diff --git a/crates/re_viewer/src/app.rs b/crates/re_viewer/src/app.rs index f079e4b4f122..05812cd1a194 100644 --- a/crates/re_viewer/src/app.rs +++ b/crates/re_viewer/src/app.rs @@ -506,7 +506,7 @@ impl App { UICommand::PrintDatastore => { if let Some(ctx) = store_context { if let Some(recording) = ctx.recording { - let table = recording.entity_db.data_store.to_data_table(); + let table = recording.store().to_data_table(); match table { Ok(table) => { println!("{table}"); diff --git a/crates/re_viewer/src/app_state.rs b/crates/re_viewer/src/app_state.rs index 6f8b2c7b63bf..b0cb658cae81 100644 --- a/crates/re_viewer/src/app_state.rs +++ b/crates/re_viewer/src/app_state.rs @@ -122,7 +122,7 @@ impl AppState { // First update the viewport and thus all active space views. // This may update their heuristics, so that all panels that are shown in this frame, // have the latest information. - let spaces_info = SpaceInfoCollection::new(&ctx.store_db.entity_db); + let spaces_info = SpaceInfoCollection::new(ctx.store_db.entity_db()); // If the blueprint is invalid, reset it. if viewport.blueprint.is_invalid() { diff --git a/crates/rerun/src/run.rs b/crates/rerun/src/run.rs index 8ec909a75825..cb9e54adb514 100644 --- a/crates/rerun/src/run.rs +++ b/crates/rerun/src/run.rs @@ -601,7 +601,7 @@ fn assert_receive_into_store_db(rx: &ReceiveSet) -> anyhow::Result Date: Thu, 12 Oct 2023 11:28:57 +0200 Subject: [PATCH 5/7] Rename `add_begin_recording_msg` to `set_store_info` --- crates/re_data_store/src/store_db.rs | 20 ++++++++++---------- crates/re_viewer/src/saving.rs | 6 +++--- crates/re_viewer/src/store_hub.rs | 2 +- crates/re_viewer/src/ui/recordings_panel.rs | 4 ++-- 4 files changed, 16 insertions(+), 16 deletions(-) diff --git a/crates/re_data_store/src/store_db.rs b/crates/re_data_store/src/store_db.rs index cb66ad004857..70a7bb13a58a 100644 --- a/crates/re_data_store/src/store_db.rs +++ b/crates/re_data_store/src/store_db.rs @@ -225,7 +225,7 @@ pub struct StoreDb { pub data_source: Option, /// Comes in a special message, [`LogMsg::SetStoreInfo`]. - recording_msg: Option, + set_store_info: Option, /// Where we store the entities. entity_db: EntityDb, @@ -237,7 +237,7 @@ impl StoreDb { store_id, entity_op_msgs: Default::default(), data_source: None, - recording_msg: None, + set_store_info: None, entity_db: Default::default(), } } @@ -247,12 +247,12 @@ impl StoreDb { &self.entity_db } - pub fn recording_msg(&self) -> Option<&SetStoreInfo> { - self.recording_msg.as_ref() + pub fn store_info_msg(&self) -> Option<&SetStoreInfo> { + self.set_store_info.as_ref() } pub fn store_info(&self) -> Option<&StoreInfo> { - self.recording_msg().map(|msg| &msg.info) + self.store_info_msg().map(|msg| &msg.info) } pub fn app_id(&self) -> Option<&ApplicationId> { @@ -296,7 +296,7 @@ impl StoreDb { } pub fn is_empty(&self) -> bool { - self.recording_msg.is_none() && self.num_rows() == 0 + self.set_store_info.is_none() && self.num_rows() == 0 } pub fn add(&mut self, msg: &LogMsg) -> Result<(), Error> { @@ -305,7 +305,7 @@ impl StoreDb { debug_assert_eq!(msg.store_id(), self.store_id()); match &msg { - LogMsg::SetStoreInfo(msg) => self.add_begin_recording_msg(msg), + LogMsg::SetStoreInfo(msg) => self.set_store_info(msg.clone()), LogMsg::EntityPathOpMsg(_, msg) => { let EntityPathOpMsg { @@ -343,8 +343,8 @@ impl StoreDb { self.entity_db.add_data_row(row) } - pub fn add_begin_recording_msg(&mut self, msg: &SetStoreInfo) { - self.recording_msg = Some(msg.clone()); + pub fn set_store_info(&mut self, store_info: SetStoreInfo) { + self.set_store_info = Some(store_info); } /// Returns an iterator over all [`EntityPathOpMsg`]s that have been written to this `StoreDb`. @@ -396,7 +396,7 @@ impl StoreDb { store_id: _, entity_op_msgs, data_source: _, - recording_msg: _, + set_store_info: _, entity_db, } = self; diff --git a/crates/re_viewer/src/saving.rs b/crates/re_viewer/src/saving.rs index 137b4a97ffd5..08bbd79d062d 100644 --- a/crates/re_viewer/src/saving.rs +++ b/crates/re_viewer/src/saving.rs @@ -86,8 +86,8 @@ pub fn save_database_to_file( re_tracing::profile_function!(); - let begin_rec_msg = store_db - .recording_msg() + let set_store_info_msg = store_db + .store_info_msg() .map(|msg| LogMsg::SetStoreInfo(msg.clone())); let ent_op_msgs = store_db @@ -115,7 +115,7 @@ pub fn save_database_to_file( use re_log_types::LogMsg; let data_msgs = data_msgs.with_context(|| "Failed to export to data tables")?; - let msgs = std::iter::once(begin_rec_msg) + let msgs = std::iter::once(set_store_info_msg) .flatten() // option .chain(ent_op_msgs) .chain(data_msgs); diff --git a/crates/re_viewer/src/store_hub.rs b/crates/re_viewer/src/store_hub.rs index 52996debe0d2..7479886fd67c 100644 --- a/crates/re_viewer/src/store_hub.rs +++ b/crates/re_viewer/src/store_hub.rs @@ -500,7 +500,7 @@ impl StoreBundle { let mut blueprint_db = StoreDb::new(id.clone()); - blueprint_db.add_begin_recording_msg(&re_log_types::SetStoreInfo { + blueprint_db.set_store_info(re_log_types::SetStoreInfo { row_id: re_log_types::RowId::random(), info: re_log_types::StoreInfo { application_id: id.as_str().into(), diff --git a/crates/re_viewer/src/ui/recordings_panel.rs b/crates/re_viewer/src/ui/recordings_panel.rs index 3de0b660b906..54982688e82f 100644 --- a/crates/re_viewer/src/ui/recordings_panel.rs +++ b/crates/re_viewer/src/ui/recordings_panel.rs @@ -254,7 +254,7 @@ fn recording_hover_ui( ui.end_row(); } - if let Some(set_store_info) = store_db.recording_msg() { + if let Some(store_info) = store_db.store_info() { let re_log_types::StoreInfo { application_id, store_id: _, @@ -262,7 +262,7 @@ fn recording_hover_ui( started, store_source, store_kind, - } = &set_store_info.info; + } = store_info; re_ui.grid_left_hand_label(ui, "Application ID"); ui.label(application_id.to_string()); From 13251a2929f4e6c22f3476a54b38462094d12afd Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Thu, 12 Oct 2023 15:49:55 +0200 Subject: [PATCH 6/7] Fixed merge issues --- crates/re_viewport/src/space_view_entity_picker.rs | 2 +- crates/re_viewport/src/space_view_heuristics.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/re_viewport/src/space_view_entity_picker.rs b/crates/re_viewport/src/space_view_entity_picker.rs index 4d0cf9fdf12d..8c5684825413 100644 --- a/crates/re_viewport/src/space_view_entity_picker.rs +++ b/crates/re_viewport/src/space_view_entity_picker.rs @@ -86,7 +86,7 @@ fn add_entities_ui( ui: &mut egui::Ui, space_view: &mut SpaceViewBlueprint, ) { - let spaces_info = SpaceInfoCollection::new(&ctx.store_db.entity_db()); + let spaces_info = SpaceInfoCollection::new(ctx.store_db.entity_db()); let tree = &ctx.store_db.entity_db().tree; let heuristic_context_per_entity = compute_heuristic_context_for_entities(ctx); let entities_add_info = create_entity_add_info( diff --git a/crates/re_viewport/src/space_view_heuristics.rs b/crates/re_viewport/src/space_view_heuristics.rs index 10b63217b933..3d3a5c47ba3d 100644 --- a/crates/re_viewport/src/space_view_heuristics.rs +++ b/crates/re_viewport/src/space_view_heuristics.rs @@ -465,7 +465,7 @@ pub fn compute_heuristic_context_for_entities( let query_time = TimeInt::MAX; let query = LatestAtQuery::new(timeline, query_time); - let tree = &ctx.store_db.entity_db.tree; + let tree = &ctx.store_db.entity_db().tree; fn visit_children_recursively( has_parent_pinhole: bool, @@ -493,7 +493,7 @@ pub fn compute_heuristic_context_for_entities( visit_children_recursively( false, tree, - &ctx.store_db.entity_db.data_store, + &ctx.store_db.entity_db().data_store, &query, &mut heuristic_context, ); From 244b0e9a041c2b9c56e335c6fc98cd220cc022d7 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Thu, 12 Oct 2023 15:52:07 +0200 Subject: [PATCH 7/7] Remove unsued `EntityPathOpMsg` (#3833) ### What We now have a `Clear` component. * Blocked on https://github.com/rerun-io/rerun/pull/3832 ### Checklist * [x] I have read and agree to [Contributor Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and the [Code of Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md) * [x] I've included a screenshot or gif (if applicable) * [x] I have tested [demo.rerun.io](https://demo.rerun.io/pr/3833) (if applicable) * [x] The PR title and labels are set such as to maximize their usefulness for the next release's CHANGELOG - [PR Build Summary](https://build.rerun.io/pr/3833) - [Docs preview](https://rerun.io/preview/a39e2fa7d5ea311739b060d294491d89a3329826/docs) - [Examples preview](https://rerun.io/preview/a39e2fa7d5ea311739b060d294491d89a3329826/examples) - [Recent benchmark results](https://ref.rerun.io/dev/bench/) - [Wasm size tracking](https://ref.rerun.io/dev/sizes/) --- 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 || {