more self review

crates/re_query_cache2/examples/range.rs

@@ -80,10 +80,6 @@ fn main() -> anyhow::Result<()> {
     //
     // Both the resolution and deserialization steps might fail, which is why this returns a `Result<Result<T>>`.
     // Use `PromiseResult::flatten` to simplify it down to a single result.
-    //
-    // A choice now has to be made regarding the nullability of the _component batch's instances_.
-    // Our IDL doesn't support nullable instances at the moment -- so for the foreseeable future you probably
-    // shouldn't be using anything but `iter_dense`.
     eprintln!("results:");
     for ((data_time, row_id), points, colors, labels) in all_frames {
         let colors = colors.unwrap_or(&[]);

crates/re_query_cache2/src/range/query.rs

@@ -61,7 +61,6 @@ pub struct RangeCache {
     /// All temporal data, organized by _data_ time.
     ///
     /// Query time is irrelevant for range queries.
-    // TODO(#4810): bucketize
     pub per_data_time: CachedRangeComponentResults,
 
     /// Everything greater than or equal to this timestamp has been asynchronously invalidated.
@@ -102,15 +101,11 @@ impl std::fmt::Debug for RangeCache {
             let per_data_time = per_data_time.read();
 
             let per_data_time_indices = &per_data_time.indices;
-            if !per_data_time_indices.is_empty() {
-                data_time_min = TimeInt::min(
-                    data_time_min,
-                    per_data_time_indices.front().map(|(t, _)| *t).unwrap(),
-                );
-                data_time_max = TimeInt::max(
-                    data_time_max,
-                    per_data_time_indices.back().map(|(t, _)| *t).unwrap(),
-                );
+            if let Some(time_front) = per_data_time_indices.front().map(|(t, _)| *t) {
+                data_time_min = TimeInt::min(data_time_min, time_front);
+            }
+            if let Some(time_back) = per_data_time_indices.back().map(|(t, _)| *t) {
+                data_time_max = TimeInt::max(data_time_max, time_back);
             }
         }
 
@@ -238,8 +233,15 @@ impl RangeCache {
 // ---
 
 impl CachedRangeComponentResultsInner {
+    /// How many _indices_ across this entire cache?
+    #[inline]
+    pub fn num_indices(&self) -> u64 {
+        self.indices.len() as _
+    }
+
+    /// How many _instances_ across this entire cache?
     #[inline]
-    pub fn num_values(&self) -> u64 {
+    pub fn num_instances(&self) -> u64 {
         self.cached_dense
             .as_ref()
             .map_or(0u64, |cached| cached.dyn_num_values() as _)
@@ -268,8 +270,8 @@ impl CachedRangeComponentResultsInner {
             return Some(reduced_query);
         }
 
-        // If this cache contains static data, then there's no point in querying anything since
-        // static data overrides everything else.
+        // If the cache contains static data, then there's no point in querying anything else since
+        // static data overrides everything anyway.
         if self
             .indices
             .front()
@@ -280,6 +282,10 @@ impl CachedRangeComponentResultsInner {
 
         // Otherwise, query for what's missing on the front-side of the cache, while making sure to
         // take pending promises into account!
+        //
+        // Keep in mind: it is not possible for the cache to contain only part of a given
+        // timestamp. All entries for a given timestamp are loaded and invalidated atomically,
+        // whether it's promises or already resolved entries.
 
         let pending_front_min = self
             .promises_front
@@ -329,8 +335,8 @@ impl CachedRangeComponentResultsInner {
             return None;
         }
 
-        // If this cache contains static data, then there's no point in querying anything since
-        // static data overrides everything else.
+        // If the cache contains static data, then there's no point in querying anything else since
+        // static data overrides everything anyway.
         if self
             .indices
             .front()
@@ -341,6 +347,10 @@ impl CachedRangeComponentResultsInner {
 
         // Otherwise, query for what's missing on the back-side of the cache, while making sure to
         // take pending promises into account!
+        //
+        // Keep in mind: it is not possible for the cache to contain only part of a given
+        // timestamp. All entries for a given timestamp are loaded and invalidated atomically,
+        // whether it's promises or already resolved entries.
 
         let pending_back_max = self
             .promises_back
@@ -365,6 +375,7 @@ impl CachedRangeComponentResultsInner {
         }
 
         // Back query should never overlap with the front query.
+        // Reminder: time ranges are all inclusive.
         if let Some(query_front) = query_front {
             let front_max_plus_one = query_front.range().max().as_i64().saturating_add(1);
             let back_min = reduced_query.range().min().as_i64();

crates/re_query_cache2/src/range/results.rs

@@ -273,6 +273,9 @@ impl CachedRangeComponentResults {
     /// deserializing the data into a single one, if you don't need the extra flexibility.
     #[inline]
     pub fn to_dense<C: Component>(&self, resolver: &PromiseResolver) -> CachedRangeData<'_, C> {
+        // It's tracing the deserialization of an entire range query at once -- it's fine.
+        re_tracing::profile_function!();
+
         // --- Step 1: try and upsert pending data (write lock) ---
 
         thread_local! {
@@ -325,12 +328,16 @@ impl CachedRangeComponentResults {
             }
 
             if !results.promises_front.is_empty() {
+                re_tracing::profile_scope!("front");
+
                 let mut resolved_indices = Vec::with_capacity(results.promises_front.len());
                 let mut resolved_data = Vec::with_capacity(results.promises_front.len());
 
                 // Pop the promises from the end so that if we encounter one that has yet to be
                 // resolved, we can stop right there and know we have a contiguous range of data
                 // available up to that point in time.
+                //
+                // Reminder: promises are sorted in ascending index order.
                 while let Some(((data_time, row_id), promise)) = results.promises_front.pop() {
                     let data = match resolver.resolve(&promise) {
                         PromiseResult::Pending => {
@@ -371,7 +378,7 @@ impl CachedRangeComponentResults {
                     .collect();
 
                 let resolved_data = FlatVecDeque::from_vecs(resolved_data);
-                // Unwraps: the data is created when entering this function -- we know it's there
+                // Unwraps: the deque is created when entering this function -- we know it's there
                 // and we know its type.
                 let cached_dense = results
                     .cached_dense
@@ -384,10 +391,16 @@ impl CachedRangeComponentResults {
             }
 
             if !results.promises_back.is_empty() {
+                re_tracing::profile_scope!("back");
+
                 let mut resolved_indices = Vec::with_capacity(results.promises_back.len());
                 let mut resolved_data = Vec::with_capacity(results.promises_back.len());
 
-                // Reverse the promises first so we can pop() from the back. See below why.
+                // Reverse the promises first so we can pop() from the back.
+                // It's fine, this is a one-time operation in the successful case, and it's extremely fast to do.
+                // See below why.
+                //
+                // Reminder: promises are sorted in ascending index order.
                 results.promises_back.reverse();
 
                 // Pop the promises from the end so that if we encounter one that has yet to be
@@ -422,13 +435,13 @@ impl CachedRangeComponentResults {
                     resolved_data.push(data);
                 }
 
-                // Reverse our reversal and give the promises back to their rightful owner.
+                // Reverse our reversal.
                 results.promises_back.reverse();
 
                 results.indices.extend(resolved_indices);
 
                 let resolved_data = FlatVecDeque::from_vecs(resolved_data);
-                // Unwraps: the data is created when entering this function -- we know it's there
+                // Unwraps: the deque is created when entering this function -- we know it's there
                 // and we know its type.
                 let cached_dense = results
                     .cached_dense
@@ -459,17 +472,25 @@ impl CachedRangeComponentResults {
         };
 
         let front_status = {
-            let (front_time, front_status) = &results.front_status;
-            if *front_time <= self.time_range.min() {
-                front_status.clone()
+            let (results_front_time, results_front_status) = &results.front_status;
+            let query_front_time = self.time_range.min();
+            if query_front_time < *results_front_time {
+                // If the query covers a larger time span on its front-side than the resulting data, then
+                // we should forward the status of the resulting data so the caller can know why it's
+                // been cropped off.
+                results_front_status.clone()
             } else {
                 PromiseResult::Ready(())
             }
         };
         let back_status = {
-            let (back_time, back_status) = &results.back_status;
-            if self.time_range.max() <= *back_time {
-                back_status.clone()
+            let (results_back_time, results_back_status) = &results.back_status;
+            let query_back_time = self.time_range.max();
+            if query_back_time > *results_back_time {
+                // If the query covers a larger time span on its back-side than the resulting data, then
+                // we should forward the status of the resulting data so the caller can know why it's
+                // been cropped off.
+                results_back_status.clone()
             } else {
                 PromiseResult::Ready(())
             }