From a826526b0cb8067daab0ce185f505a853fb02a85 Mon Sep 17 00:00:00 2001 From: Clement Rey Date: Mon, 8 Jan 2024 16:08:15 +0100 Subject: [PATCH] self-review --- crates/re_query_cache/src/cache.rs | 14 ++--- crates/re_query_cache/src/lib.rs | 2 + crates/re_query_cache/src/query.rs | 93 +++++++++++++++--------------- 3 files changed, 56 insertions(+), 53 deletions(-) diff --git a/crates/re_query_cache/src/cache.rs b/crates/re_query_cache/src/cache.rs index 56d0e2abfecd..1ad320ad0407 100644 --- a/crates/re_query_cache/src/cache.rs +++ b/crates/re_query_cache/src/cache.rs @@ -39,7 +39,7 @@ static CACHES: Lazy = 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>, // // 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 } } diff --git a/crates/re_query_cache/src/lib.rs b/crates/re_query_cache/src/lib.rs index a847d4d98878..4f57a78b83a6 100644 --- a/crates/re_query_cache/src/lib.rs +++ b/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! { diff --git a/crates/re_query_cache/src/query.rs b/crates/re_query_cache/src/query.rs index 9ee04d9894da..a0ce621aa705 100644 --- a/crates/re_query_cache/src/query.rs +++ b/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), } impl<'a, C: Clone> MaybeCachedComponentData<'a, C> { #[inline] pub fn iter(&self) -> impl ExactSizeIterator + '_ { - 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::( - 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::(store, &query, entity_path)?; + if bucket.is_empty() { + let now = web_time::Instant::now(); + let arch_view = query_archetype::(store, &query, entity_path)?; - bucket.[]::(query.at, &arch_view)?; + bucket.[]::(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::( + store.id().clone(), + entity_path.clone(), + query, + |cache| latest_at_callback(query, cache), ) }, }