Skip to content

Commit

Permalink
When running GC on the blueprint, protect the latest 100 undo points
Browse files Browse the repository at this point in the history
  • Loading branch information
emilk committed Oct 7, 2024
1 parent 719ddb5 commit 5fe4cc4
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 26 deletions.
17 changes: 2 additions & 15 deletions crates/store/re_entity_db/src/entity_db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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

// ----------------------------------------------------------------------------

Expand Down Expand Up @@ -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<ChunkStoreEvent> {
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<ChunkStoreEvent> {
re_tracing::profile_function!();
Expand Down Expand Up @@ -430,7 +417,7 @@ impl EntityDb {
store_events
}

fn gc(&mut self, gc_options: &GarbageCollectionOptions) -> Vec<ChunkStoreEvent> {
pub fn gc(&mut self, gc_options: &GarbageCollectionOptions) -> Vec<ChunkStoreEvent> {
re_tracing::profile_function!();

let (store_events, stats_diff) = self.data_store.gc(gc_options);
Expand Down
2 changes: 1 addition & 1 deletion crates/store/re_entity_db/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down
4 changes: 2 additions & 2 deletions crates/viewer/re_viewer/src/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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();
Expand Down
32 changes: 24 additions & 8 deletions crates/viewer/re_viewer_context/src/store_hub.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
///
Expand Down Expand Up @@ -673,7 +676,7 @@ impl StoreHub {
});
}

pub fn gc_blueprints(&mut self) {
pub fn gc_blueprints(&mut self, undo_state: &HashMap<StoreId, BlueprintUndoState>) {
re_tracing::profile_function!();

for blueprint_id in self
Expand All @@ -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);
}
Expand Down
19 changes: 19 additions & 0 deletions crates/viewer/re_viewer_context/src/undo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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<TimeInt> {
self.inflection_points.first().copied()
}

pub fn blueprint_query(&self) -> LatestAtQuery {
if let Some(time) = self.current_time {
LatestAtQuery::new(blueprint_timeline(), time)
Expand Down Expand Up @@ -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;
}
}
}
}

Expand Down

0 comments on commit 5fe4cc4

Please sign in to comment.