First round of PR cleanup

crates/re_arrow_store/src/lib.rs

@@ -36,7 +36,7 @@ pub mod polars_util;
 pub mod test_util;
 
 pub use self::arrow_util::ArrayExt;
-pub use self::store::{DataStore, DataStoreConfig};
+pub use self::store::{DataStore, DataStoreConfig, StoreGeneration};
 pub use self::store_gc::GarbageCollectionTarget;
 pub use self::store_read::{LatestAtQuery, RangeQuery};
 pub use self::store_stats::{DataStoreRowStats, DataStoreStats};

crates/re_arrow_store/src/store.rs

@@ -155,6 +155,10 @@ impl std::ops::DerefMut for ClusterCellCache {
 
 // ---
 
+/// Incremented on each edit
+#[derive(Debug, Default, Clone, PartialEq, Eq, PartialOrd, Ord)]
+pub struct StoreGeneration(u64);
+
 /// A complete data store: covers all timelines, all entities, everything.
 ///
 /// ## Debugging
@@ -264,8 +268,10 @@ impl DataStore {
         "rerun.insert_id".into()
     }
 
-    pub fn last_insert_id(&self) -> u64 {
-        self.insert_id
+    /// Return the current `StoreGeneration`. This can be used to determine whether the
+    /// database has been modified since the last time it was queried.
+    pub fn generation(&self) -> StoreGeneration {
+        StoreGeneration(self.insert_id)
     }
 
     /// See [`Self::cluster_key`] for more information about the cluster key.

crates/re_data_store/src/entity_properties.rs

@@ -38,11 +38,12 @@ impl EntityPropertyMap {
     /// Determine whether this `EntityPropertyMap` has user-edits relative to another `EntityPropertyMap`
     pub fn has_edits(&self, other: &Self) -> bool {
         self.props.len() != other.props.len()
-            || self
-                .props
-                .iter()
-                .zip(other.props.iter())
-                .any(|(x, y)| x.0 != y.0 || x.1.has_edits(y.1))
+            || self.props.iter().any(|(key, val)| {
+                other
+                    .props
+                    .get(key)
+                    .map_or(true, |other_val| val.has_edits(other_val))
+            })
     }
 }
 

crates/re_data_store/src/store_db.rs

@@ -292,8 +292,10 @@ impl StoreDb {
             + self.entity_db.data_store.num_temporal_rows() as usize
     }
 
-    pub fn last_insert_id(&self) -> u64 {
-        self.entity_db.data_store.last_insert_id()
+    /// Return the current `StoreGeneration`. This can be used to determine whether the
+    /// database has been modified since the last time it was queried.
+    pub fn generation(&self) -> re_arrow_store::StoreGeneration {
+        self.entity_db.data_store.generation()
     }
 
     pub fn is_empty(&self) -> bool {

crates/re_space_view/src/data_blueprint.rs

@@ -115,12 +115,13 @@ impl DataBlueprintTree {
             data_blueprints,
         } = self;
 
-        // Note: this could fail unexpectedly if slotmap iteration order is unstable.
         groups.len() != other.groups.len()
-            || groups
-                .iter()
-                .zip(other.groups.iter())
-                .any(|(x, y)| x.0 != y.0 || x.1.has_edits(y.1))
+            || groups.iter().any(|(key, val)| {
+                other
+                    .groups
+                    .get(key)
+                    .map_or(true, |other_val| val.has_edits(other_val))
+            })
             || *path_to_group != other.path_to_group
             || *entity_paths != other.entity_paths
             || *root_group_handle != other.root_group_handle

crates/re_viewer/src/saving.rs

@@ -44,11 +44,9 @@ pub fn default_blueprint_path(app_id: &ApplicationId) -> anyhow::Result<std::pat
             // so insert the hash.
             if sanitized_app_id != app_id.0 {
                 // Hash the original app-id.
-                let salt: u64 = 0xc927_d8cd_910d_16a3;
 
                 let hash = {
                     let mut hasher = ahash::RandomState::with_seeds(1, 2, 3, 4).build_hasher();
-                    salt.hash(&mut hasher);
                     app_id.0.hash(&mut hasher);
                     hasher.finish()
                 };

crates/re_viewer/src/store_hub.rs

@@ -1,6 +1,6 @@
 use ahash::HashMap;
 use itertools::Itertools;
-use re_arrow_store::{DataStoreConfig, DataStoreStats};
+use re_arrow_store::{DataStoreConfig, DataStoreStats, StoreGeneration};
 use re_data_store::StoreDb;
 use re_log_types::{ApplicationId, StoreId, StoreKind};
 use re_viewer_context::StoreContext;
@@ -26,9 +26,9 @@ pub struct StoreHub {
     blueprint_by_app_id: HashMap<ApplicationId, StoreId>,
     store_dbs: StoreBundle,
 
-    // The number of rows in the blueprint the last time it was saved
+    // The [`StoreGeneration`] from when the [`StoreDb`] was last saved
     #[cfg(not(target_arch = "wasm32"))]
-    blueprint_last_save: HashMap<StoreId, u64>,
+    blueprint_last_save: HashMap<StoreId, StoreGeneration>,
 }
 
 /// Convenient information used for `MemoryPanel`
@@ -121,7 +121,7 @@ impl StoreHub {
     /// Change which blueprint is active for a given [`ApplicationId`]
     #[inline]
     pub fn set_blueprint_for_app_id(&mut self, blueprint_id: StoreId, app_id: ApplicationId) {
-        re_log::debug!("Switching blueprint for {:?} to {:?}", app_id, blueprint_id);
+        re_log::debug!("Switching blueprint for {app_id:?} to {blueprint_id:?}");
         self.blueprint_by_app_id.insert(app_id, blueprint_id);
     }
 
@@ -162,61 +162,56 @@ impl StoreHub {
     }
 
     /// Persist any in-use blueprints to durable storage.
-    // TODO(2579): implement persistence for web
+    // TODO(#2579): implement persistence for web
     #[cfg(not(target_arch = "wasm32"))]
     pub fn persist_app_blueprints(&mut self) -> anyhow::Result<()> {
         // Because we save blueprints based on their `ApplicationId`, we only
         // save the blueprints referenced by `blueprint_by_app_id`, even though
         // there may be other Blueprints in the Hub.
         for (app_id, blueprint_id) in &self.blueprint_by_app_id {
             let blueprint_path = default_blueprint_path(app_id)?;
-            re_log::debug!("Saving blueprint for {app_id} to {:?}", blueprint_path);
+            re_log::debug!("Saving blueprint for {app_id} to {blueprint_path:?}");
 
             if let Some(blueprint) = self.store_dbs.blueprint(blueprint_id) {
-                // TODO(jleibs): This check isn't perfect. If we GC the blueprint and then insert
-                // more rows we could end up with a collision.
-                if self.blueprint_last_save.get(blueprint_id) != Some(&blueprint.last_insert_id()) {
-                    // TODO(jleibs): We should "flatten" blueprints when we save them
+                if self.blueprint_last_save.get(blueprint_id) != Some(&blueprint.generation()) {
+                    // TODO(#1803): We should "flatten" blueprints when we save them
                     // TODO(jleibs): Should we push this into a background thread? Blueprints should generally
                     // be small & fast to save, but maybe not once we start adding big pieces of user data?
                     let file_saver = save_database_to_file(blueprint, blueprint_path, None)?;
                     file_saver()?;
                     self.blueprint_last_save
-                        .insert(blueprint_id.clone(), blueprint.last_insert_id());
+                        .insert(blueprint_id.clone(), blueprint.generation());
                 }
             }
         }
         Ok(())
     }
 
     /// Try to load the persisted blueprint for the given `ApplicationId`
-    // TODO(2579): implement persistence for web
+    // TODO(#2579): implement persistence for web
     #[cfg(not(target_arch = "wasm32"))]
     pub fn try_to_load_persisted_blueprint(
         &mut self,
         app_id: &ApplicationId,
     ) -> anyhow::Result<()> {
+        re_tracing::profile_function!();
         let blueprint_path = default_blueprint_path(app_id)?;
         if blueprint_path.exists() {
-            re_log::debug!(
-                "Trying to load blueprint for {app_id} from {:?}",
-                blueprint_path
-            );
+            re_log::debug!("Trying to load blueprint for {app_id} from {blueprint_path:?}",);
             if let Some(mut bundle) = load_file_path(&blueprint_path) {
                 for store in bundle.drain_store_dbs() {
                     if store.store_kind() == StoreKind::Blueprint && store.app_id() == Some(app_id)
                     {
                         // We found the blueprint we were looking for; make it active.
                         // borrow-checker won't let us just call `self.set_blueprint_for_app_id`
                         re_log::debug!(
-                            "Switching blueprint for {:?} to {:?}",
-                            app_id,
+                            "Switching blueprint for {app_id:?} to {:?}",
                             store.store_id(),
                         );
                         self.blueprint_by_app_id
                             .insert(app_id.clone(), store.store_id().clone());
                         self.blueprint_last_save
-                            .insert(store.store_id().clone(), store.last_insert_id());
+                            .insert(store.store_id().clone(), store.generation());
                         self.store_dbs.insert_blueprint(store);
                     } else {
                         re_log::warn!(