From 5fe4cc4dc741647ec6d0fec6ef6366ca50b38028 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Sun, 6 Oct 2024 17:32:12 +0200 Subject: [PATCH] When running GC on the blueprint, protect the latest 100 undo points --- crates/store/re_entity_db/src/entity_db.rs | 17 ++-------- crates/store/re_entity_db/src/lib.rs | 2 +- crates/viewer/re_viewer/src/app.rs | 4 +-- .../viewer/re_viewer_context/src/store_hub.rs | 32 ++++++++++++++----- crates/viewer/re_viewer_context/src/undo.rs | 19 +++++++++++ 5 files changed, 48 insertions(+), 26 deletions(-) diff --git a/crates/store/re_entity_db/src/entity_db.rs b/crates/store/re_entity_db/src/entity_db.rs index 963284e0fa74..c35f1243b2c6 100644 --- a/crates/store/re_entity_db/src/entity_db.rs +++ b/crates/store/re_entity_db/src/entity_db.rs @@ -18,7 +18,7 @@ use crate::{Error, TimesPerTimeline}; // ---------------------------------------------------------------------------- /// See [`GarbageCollectionOptions::time_budget`]. -const DEFAULT_GC_TIME_BUDGET: std::time::Duration = std::time::Duration::from_micros(3500); // empirical +pub const DEFAULT_GC_TIME_BUDGET: std::time::Duration = std::time::Duration::from_micros(3500); // empirical // ---------------------------------------------------------------------------- @@ -387,19 +387,6 @@ impl EntityDb { self.set_store_info = Some(store_info); } - pub fn gc_everything_but_the_latest_row_on_non_default_timelines( - &mut self, - ) -> Vec { - re_tracing::profile_function!(); - - self.gc(&GarbageCollectionOptions { - target: GarbageCollectionTarget::Everything, - protect_latest: 1, - time_budget: DEFAULT_GC_TIME_BUDGET, - protected_time_ranges: Default::default(), // TODO(#3135): Use this for undo buffer - }) - } - /// Free up some RAM by forgetting the older parts of all timelines. pub fn purge_fraction_of_ram(&mut self, fraction_to_purge: f32) -> Vec { re_tracing::profile_function!(); @@ -430,7 +417,7 @@ impl EntityDb { store_events } - fn gc(&mut self, gc_options: &GarbageCollectionOptions) -> Vec { + pub fn gc(&mut self, gc_options: &GarbageCollectionOptions) -> Vec { re_tracing::profile_function!(); let (store_events, stats_diff) = self.data_store.gc(gc_options); diff --git a/crates/store/re_entity_db/src/lib.rs b/crates/store/re_entity_db/src/lib.rs index 2ef69e2b24c2..c9fa97882faf 100644 --- a/crates/store/re_entity_db/src/lib.rs +++ b/crates/store/re_entity_db/src/lib.rs @@ -13,7 +13,7 @@ mod times_per_timeline; mod versioned_instance_path; pub use self::{ - entity_db::EntityDb, + entity_db::{EntityDb, DEFAULT_GC_TIME_BUDGET}, entity_tree::EntityTree, instance_path::{InstancePath, InstancePathHash}, store_bundle::{StoreBundle, StoreLoadError}, diff --git a/crates/viewer/re_viewer/src/app.rs b/crates/viewer/re_viewer/src/app.rs index 372734361294..619085bdd242 100644 --- a/crates/viewer/re_viewer/src/app.rs +++ b/crates/viewer/re_viewer/src/app.rs @@ -1555,7 +1555,7 @@ impl eframe::App for App { // TODO(#2579): implement web-storage for blueprints as well if let Some(hub) = &mut self.store_hub { if self.state.app_options.blueprint_gc { - hub.gc_blueprints(); + hub.gc_blueprints(&self.state.blueprint_undo_state); } if let Err(err) = hub.save_app_blueprints() { @@ -1673,7 +1673,7 @@ impl eframe::App for App { self.receive_messages(&mut store_hub, egui_ctx); if self.app_options().blueprint_gc { - store_hub.gc_blueprints(); + store_hub.gc_blueprints(&self.state.blueprint_undo_state); } store_hub.purge_empty(); diff --git a/crates/viewer/re_viewer_context/src/store_hub.rs b/crates/viewer/re_viewer_context/src/store_hub.rs index 00079d04f5df..2f89a8ff9661 100644 --- a/crates/viewer/re_viewer_context/src/store_hub.rs +++ b/crates/viewer/re_viewer_context/src/store_hub.rs @@ -3,12 +3,15 @@ use ahash::{HashMap, HashMapExt, HashSet}; use anyhow::Context as _; use itertools::Itertools as _; -use re_chunk_store::{ChunkStoreConfig, ChunkStoreGeneration, ChunkStoreStats}; +use re_chunk_store::{ + ChunkStoreConfig, ChunkStoreGeneration, ChunkStoreStats, GarbageCollectionOptions, + GarbageCollectionTarget, +}; use re_entity_db::{EntityDb, StoreBundle}; -use re_log_types::{ApplicationId, StoreId, StoreKind}; +use re_log_types::{ApplicationId, ResolvedTimeRange, StoreId, StoreKind}; use re_query::CachesStats; -use crate::{Caches, StoreContext}; +use crate::{BlueprintUndoState, Caches, StoreContext}; /// Interface for accessing all blueprints and recordings /// @@ -673,7 +676,7 @@ impl StoreHub { }); } - pub fn gc_blueprints(&mut self) { + pub fn gc_blueprints(&mut self, undo_state: &HashMap) { re_tracing::profile_function!(); for blueprint_id in self @@ -686,10 +689,23 @@ impl StoreHub { continue; // no change since last gc } - // TODO(jleibs): Decide a better tuning for this. Would like to save a - // reasonable amount of history, or incremental snapshots. - let store_events = - blueprint.gc_everything_but_the_latest_row_on_non_default_timelines(); + let mut protected_time_ranges = ahash::HashMap::default(); + if let Some(undo) = undo_state.get(blueprint_id) { + if let Some(time) = undo.oldest_undo_point() { + // Save everything that we could want to undo to: + protected_time_ranges.insert( + crate::blueprint_timeline(), + ResolvedTimeRange::new(time, re_chunk::TimeInt::MAX), + ); + } + } + + let store_events = blueprint.gc(&GarbageCollectionOptions { + target: GarbageCollectionTarget::Everything, + protect_latest: 0, + time_budget: re_entity_db::DEFAULT_GC_TIME_BUDGET, + protected_time_ranges, + }); if let Some(caches) = self.caches_per_recording.get_mut(blueprint_id) { caches.on_store_events(&store_events); } diff --git a/crates/viewer/re_viewer_context/src/undo.rs b/crates/viewer/re_viewer_context/src/undo.rs index d9bad8edf2c1..836e52976725 100644 --- a/crates/viewer/re_viewer_context/src/undo.rs +++ b/crates/viewer/re_viewer_context/src/undo.rs @@ -6,6 +6,11 @@ use re_log_types::ResolvedTimeRange; use crate::blueprint_timeline; +/// Max number of undo points. +/// +/// TODO(emilk): decide based on how much memory the blueprint uses instead. +const MAX_UNDOS: usize = 100; + /// We store the entire edit history of a blueprint in its store. /// /// When undoing, we move back time, and redoing move it forward. @@ -38,6 +43,11 @@ impl BlueprintUndoState { LatestAtQuery::latest(blueprint_timeline()) } + /// How far back in time can we undo? + pub fn oldest_undo_point(&self) -> Option { + self.inflection_points.first().copied() + } + pub fn blueprint_query(&self) -> LatestAtQuery { if let Some(time) = self.current_time { LatestAtQuery::new(blueprint_timeline(), time) @@ -98,6 +108,15 @@ impl BlueprintUndoState { // and disregard those, in case we miss something in `is_interacting`. // Note that this on its own won't enough though - if you drag a slider, // then you don't want an undo-point each time you pause the mouse - only on mouse-up! + + // Don't store too many undo-points: + while let Some(first) = self.inflection_points.first().copied() { + if MAX_UNDOS < self.inflection_points.len() { + self.inflection_points.remove(&first); + } else { + break; + } + } } }