self-review

crates/re_query_cache/src/cache.rs

@@ -39,7 +39,7 @@ static CACHES: Lazy<Caches> = Lazy::new(Caches::default);
 /// Maintains the top-level cache mappings.
 //
 // TODO(cmc): Store subscriber and cache invalidation.
-// TODO(cmc): SizeBytes support + size stats + mem panel
+// TODO(#4730): SizeBytes support + size stats + mem panel
 // TODO(cmc): timeless caching support
 #[derive(Default)]
 pub struct Caches {
@@ -49,7 +49,7 @@ pub struct Caches {
 impl Caches {
     /// Clears all caches.
     //
-    // TODO(cmc): expose palette command.
+    // TODO(#4731): expose palette command.
     #[inline]
     pub fn clear() {
         let Caches { latest_at } = &*CACHES;
@@ -150,12 +150,12 @@ pub struct CacheBucket {
 
     /// The resulting component data, pre-deserialized, pre-joined.
     //
-    // TODO(cmc): Don't denormalize auto-generated instance keys.
-    // TODO(cmc): Don't denormalize splatted values.
+    // TODO(#4733): Don't denormalize auto-generated instance keys.
+    // TODO(#4734): Don't denormalize splatted values.
     pub(crate) components: BTreeMap<ComponentName, Box<dyn ErasedFlatVecDeque + Send + Sync>>,
     //
     // TODO(cmc): secondary cache
-    // TODO(cmc): size stats: this requires codegen'ing SizeBytes for all components!
+    // TODO(#4730): size stats: this requires codegen'ing SizeBytes for all components!
 }
 
 impl CacheBucket {
@@ -209,13 +209,13 @@ impl CacheBucket {
 
     /// How many timestamps' worth of data is stored in this bucket?
     #[inline]
-    pub fn len(&self) -> usize {
+    pub fn num_entries(&self) -> usize {
         self.pov_times.len()
     }
 
     #[inline]
     pub fn is_empty(&self) -> bool {
-        self.len() == 0
+        self.num_entries() == 0
     }
 }
 

crates/re_query_cache/src/lib.rs

@@ -10,6 +10,8 @@ pub use self::query::{
     query_archetype_pov1, query_archetype_with_history_pov1, MaybeCachedComponentData,
 };
 
+pub(crate) use self::cache::LatestAtCache;
+
 // TODO(cmc): Supporting N>1 generically is quite painful due to limitations in declarative macros,
 // not that we care at the moment.
 seq_macro::seq!(NUM_COMP in 0..10 { paste::paste! {

crates/re_query_cache/src/query.rs

@@ -18,17 +18,15 @@ pub enum MaybeCachedComponentData<'a, C> {
     Cached(&'a [C]),
     // TODO(cmc): Ideally, this would be a reference to a `dyn Iterator` that is the result of
     // calling `ArchetypeView::iter_{required|optional}_component`.
-    // In practice this enters lifetime invariance hell for, from what I can see, no particular gains.
+    // In practice this enters lifetime invariance hell for, from what I can see, no particular gains
+    // (rustc is pretty good at optimizing out collections into obvious temporary variables).
     Raw(Vec<C>),
 }
 
 impl<'a, C: Clone> MaybeCachedComponentData<'a, C> {
     #[inline]
     pub fn iter(&self) -> impl ExactSizeIterator<Item = &C> + '_ {
-        match self {
-            MaybeCachedComponentData::Cached(data) => itertools::Either::Left(data.iter()),
-            MaybeCachedComponentData::Raw(data) => itertools::Either::Right(data.iter()),
-        }
+        self.as_slice().iter()
     }
 
     #[inline]
@@ -97,54 +95,57 @@ macro_rules! impl_query_archetype {
                 format!("arch={} pov={} comp={}", A::name(), $N, $M)
             );
 
-            match &query {
-                AnyQuery::LatestAt(query) => {
-                    Caches::with_latest_at::<A, _, _>(
-                        store.id().clone(),
-                        entity_path.clone(),
-                        query,
-                        |cache| {
-                            re_tracing::profile_scope!("latest_at");
+            let mut latest_at_callback = |query: &LatestAtQuery, cache: &mut crate::LatestAtCache| {
+                re_tracing::profile_scope!("latest_at");
 
-                             let bucket = cache.entry(query.at).or_default();
-                            // NOTE: Implicitly dropping the write guard here: the LatestAtCache is
-                            // free once again!
+                let bucket = cache.entry(query.at).or_default();
+                // NOTE: Implicitly dropping the write guard here: the LatestAtCache is free once again!
 
-                            if bucket.is_empty() {
-                                let now = web_time::Instant::now();
-                                let arch_view = query_archetype::<A>(store, &query, entity_path)?;
+                if bucket.is_empty() {
+                    let now = web_time::Instant::now();
+                    let arch_view = query_archetype::<A>(store, &query, entity_path)?;
 
-                                bucket.[<insert_pov $N _comp$M>]::<A, $($pov,)+ $($comp,)*>(query.at, &arch_view)?;
+                    bucket.[<insert_pov $N _comp$M>]::<A, $($pov,)+ $($comp,)*>(query.at, &arch_view)?;
 
-                                // TODO(cmc): I'd love a way of putting this information into
-                                // the `puffin` span directly.
-                                let elapsed = now.elapsed();
-                                ::re_log::trace!(
-                                    "cached new entry in {elapsed:?} ({:0.3} entries/s)",
-                                    1f64 / elapsed.as_secs_f64()
-                                );
-                            }
+                    // TODO(cmc): I'd love a way of putting this information into the `puffin` span directly.
+                    let elapsed = now.elapsed();
+                    ::re_log::trace!(
+                        store_id=%store.id(),
+                        %entity_path,
+                        archetype=%A::name(),
+                        "cached new entry in {elapsed:?} ({:0.3} entries/s)",
+                        1f64 / elapsed.as_secs_f64()
+                    );
+                }
 
-                            let it = itertools::izip!(
-                                bucket.iter_pov_times(),
-                                bucket.iter_pov_instance_keys(),
-                                $(bucket.iter_component::<$pov>()?,)+
-                                $(bucket.iter_component_opt::<$comp>()?,)*
-                            ).map(|(time, instance_keys, $($pov,)+ $($comp,)*)| {
-                                (
-                                    *time,
-                                    MaybeCachedComponentData::Cached(instance_keys),
-                                    $(MaybeCachedComponentData::Cached($pov),)+
-                                    $(MaybeCachedComponentData::Cached($comp),)*
-                                )
-                            });
+                let it = itertools::izip!(
+                    bucket.iter_pov_times(),
+                    bucket.iter_pov_instance_keys(),
+                    $(bucket.iter_component::<$pov>()?,)+
+                    $(bucket.iter_component_opt::<$comp>()?,)*
+                ).map(|(time, instance_keys, $($pov,)+ $($comp,)*)| {
+                    (
+                        *time,
+                        MaybeCachedComponentData::Cached(instance_keys),
+                        $(MaybeCachedComponentData::Cached($pov),)+
+                        $(MaybeCachedComponentData::Cached($comp),)*
+                    )
+                });
 
-                            for data in it {
-                                f(data);
-                            }
+                for data in it {
+                    f(data);
+                }
 
-                            Ok(())
-                        }
+                Ok(())
+            };
+
+            match &query {
+                AnyQuery::LatestAt(query) => {
+                    Caches::with_latest_at::<A, _, _>(
+                        store.id().clone(),
+                        entity_path.clone(),
+                        query,
+                        |cache| latest_at_callback(query, cache),
                     )
                 },
             }