From 5179c12d36ca1018692d1d4cb1c2274cfb26aef0 Mon Sep 17 00:00:00 2001
From: Clement Rey <cr.rey.clement@gmail.com>
Date: Sat, 8 Apr 2023 16:04:55 +0200
Subject: [PATCH] addressed PR comments

---
 crates/re_arrow_store/src/store_dump.rs  | 35 +++++++-----------------
 crates/re_arrow_store/src/store_write.rs |  3 +-
 crates/re_log_types/src/time_range.rs    |  6 ++++
 3 files changed, 18 insertions(+), 26 deletions(-)

diff --git a/crates/re_arrow_store/src/store_dump.rs b/crates/re_arrow_store/src/store_dump.rs
index bd43cdc18d7c..d24f3a317454 100644
--- a/crates/re_arrow_store/src/store_dump.rs
+++ b/crates/re_arrow_store/src/store_dump.rs
@@ -20,8 +20,6 @@ impl DataStore {
         &self,
         time_filter: Option<(Timeline, TimeRange)>,
     ) -> impl Iterator<Item = DataTable> + '_ {
-        crate::profile_function!();
-
         let timeless = self.dump_timeless_tables();
         let temporal = if let Some(time_filter) = time_filter {
             Either::Left(self.dump_temporal_tables_filtered(time_filter))
@@ -33,9 +31,9 @@ impl DataStore {
     }
 
     fn dump_timeless_tables(&self) -> impl Iterator<Item = DataTable> + '_ {
-        crate::profile_function!();
-
         self.timeless_tables.values().map(|table| {
+            crate::profile_scope!("timeless_table");
+
             let PersistentIndexedTable {
                 ent_path,
                 cluster_key: _,
@@ -46,7 +44,7 @@ impl DataStore {
             } = table;
 
             DataTable {
-                table_id: generate_table_id(),
+                table_id: TableId::random(),
                 col_row_id: col_row_id.clone(),
                 col_timelines: Default::default(),
                 col_entity_path: std::iter::repeat_with(|| ent_path.clone())
@@ -60,10 +58,10 @@ impl DataStore {
 
     fn dump_temporal_tables(&self) -> impl Iterator<Item = DataTable> + '_ {
         self.tables.values().flat_map(|table| {
-            crate::profile_function!();
+            crate::profile_scope!("temporal_table");
 
             table.buckets.values().map(move |bucket| {
-                crate::profile_function!();
+                crate::profile_scope!("temporal_bucket");
 
                 bucket.sort_indices_if_needed();
 
@@ -86,7 +84,7 @@ impl DataStore {
                 debug_assert!(is_sorted);
 
                 DataTable {
-                    table_id: generate_table_id(),
+                    table_id: TableId::random(),
                     col_row_id: col_row_id.clone(),
                     col_timelines: [(*timeline, col_time.iter().copied().map(Some).collect())]
                         .into(),
@@ -107,14 +105,14 @@ impl DataStore {
         self.tables
             .values()
             .filter_map(move |table| {
-                crate::profile_function!();
+                crate::profile_scope!("temporal_table_filtered");
 
                 if table.timeline != timeline_filter {
                     return None;
                 }
 
                 Some(table.buckets.values().filter_map(move |bucket| {
-                    crate::profile_function!();
+                    crate::profile_scope!("temporal_bucket_filtered");
 
                     bucket.sort_indices_if_needed();
 
@@ -136,7 +134,7 @@ impl DataStore {
                     } = &*inner.read();
                     debug_assert!(is_sorted);
 
-                    if time_range.min > time_filter.max || time_range.max < time_filter.min {
+                    if !time_range.intersects(time_filter) {
                         return None;
                     }
 
@@ -172,7 +170,7 @@ impl DataStore {
                     }
 
                     Some(DataTable {
-                        table_id: generate_table_id(),
+                        table_id: TableId::random(),
                         col_row_id,
                         col_timelines,
                         col_entity_path,
@@ -185,19 +183,6 @@ impl DataStore {
     }
 }
 
-fn generate_table_id() -> TableId {
-    // NOTE: The `TableId` is only used for debugging purposes so this doesn't matter
-    // much... also we don't support saving to file on the web anyhow.
-    #[cfg(not(target_arch = "wasm32"))]
-    {
-        TableId::random()
-    }
-    #[cfg(target_arch = "wasm32")]
-    {
-        TableId::ZERO
-    }
-}
-
 fn filter_column<'a, T: 'a + Clone>(
     col_time: &'a ErasedTimeVec,
     column: impl Iterator<Item = &'a T> + 'a,
diff --git a/crates/re_arrow_store/src/store_write.rs b/crates/re_arrow_store/src/store_write.rs
index 7994c83a992c..2f68bedf445c 100644
--- a/crates/re_arrow_store/src/store_write.rs
+++ b/crates/re_arrow_store/src/store_write.rs
@@ -214,7 +214,8 @@ impl DataStore {
             let values =
                 arrow2::array::UInt64Array::from_vec((0..num_instances as u64).collect_vec())
                     .boxed();
-            let cell = DataCell::from_arrow(InstanceKey::name(), values);
+            let mut cell = DataCell::from_arrow(InstanceKey::name(), values);
+            cell.compute_size_bytes();
 
             self.cluster_cell_cache
                 .insert(num_instances, cell.clone() /* shallow */);
diff --git a/crates/re_log_types/src/time_range.rs b/crates/re_log_types/src/time_range.rs
index 645e8aff889a..68b7e4a7f159 100644
--- a/crates/re_log_types/src/time_range.rs
+++ b/crates/re_log_types/src/time_range.rs
@@ -38,6 +38,7 @@ impl TimeRange {
         self.min.as_i64().abs_diff(self.max.as_i64())
     }
 
+    #[inline]
     pub fn center(&self) -> TimeInt {
         self.min + TimeInt::from((self.abs_length() / 2) as i64)
     }
@@ -47,6 +48,11 @@ impl TimeRange {
         self.min <= time && time <= self.max
     }
 
+    #[inline]
+    pub fn intersects(&self, other: Self) -> bool {
+        self.min <= other.max && self.max >= other.min
+    }
+
     #[inline]
     pub fn union(&self, other: Self) -> Self {
         Self {