Skip to content

Commit

Permalink
Static data 0: revamped TimeInt (#5534)
Browse files Browse the repository at this point in the history
_Commits make no sense, review the final changelog directly._

_All the interesting bits happen in `re_log_types/time_point` & `re_sdk`
-- everything else is just change propagation._


- `TimeInt` now ranges from `i64:MIN + 1` to `i64::MAX`.
- `TimeInt::STATIC`, which takes the place of the now illegal
`TimeInt(i64::MIN)`, is now _the only way_ of identifying static data.
- It is impossible to create `TimeInt::STATIC` inadvertently -- users of
the SDK cannot set the clock to that value.
- Similarly, it is impossible to create a `TimeRange`, a `TimePoint`, a
`LatestAtQuery` or a `RangeQuery` that includes `TimeInt::STATIC`.
If static data exists, that's what will be returned, unconditionally --
there's no such thing as querying for it explicitely.
- `TimePoint::timeless` is gone -- we already have `TimePoint::default`
that we use all over the place, we don't need two ways of doing the same
thing.

There still exists a logical mapping between an empty `TimePoint` and
static data, as that is how one represents static data on the wire --
terminology wise: "a timeless timepoint results in static data".

Similar to the "ensure `RowId`s are unique" refactor from back when,
this seemingly tiny change on the surface will vastly simplify
downstream code that finally has some invariants to rely on.

- Fixes #4832
- Related to #5264


---

Part of a PR series that removes the concept of timeless data in favor
of the much simpler concept of static data:
- #5534
- #5535
- #5536
- #5537
- #5540
  • Loading branch information
teh-cmc committed Apr 5, 2024
1 parent f100637 commit eb6270d
Show file tree
Hide file tree
Showing 74 changed files with 1,101 additions and 743 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

19 changes: 11 additions & 8 deletions crates/re_data_source/src/data_loader/loader_archetype.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use re_log_types::{DataRow, EntityPath, RowId, TimePoint};
use re_log_types::{DataRow, EntityPath, RowId, TimeInt, TimePoint};

use crate::{DataLoader, DataLoaderError, LoadedData};

Expand Down Expand Up @@ -54,29 +54,32 @@ impl DataLoader for ArchetypeLoader {

let entity_path = EntityPath::from_file_path(&filepath);

let mut timepoint = TimePoint::timeless();
let mut timepoint = TimePoint::default();
// TODO(cmc): log these once heuristics (I think?) are fixed
if false {
if let Ok(metadata) = filepath.metadata() {
use re_log_types::{Time, Timeline};

if let Some(created) = metadata.created().ok().and_then(|t| Time::try_from(t).ok())
if let Some(created) = metadata
.created()
.ok()
.and_then(|t| TimeInt::try_from(Time::try_from(t).ok()?).ok())
{
timepoint.insert(Timeline::new_temporal("created_at"), created.into());
timepoint.insert(Timeline::new_temporal("created_at"), created);
}
if let Some(modified) = metadata
.modified()
.ok()
.and_then(|t| Time::try_from(t).ok())
.and_then(|t| TimeInt::try_from(Time::try_from(t).ok()?).ok())
{
timepoint.insert(Timeline::new_temporal("modified_at"), modified.into());
timepoint.insert(Timeline::new_temporal("modified_at"), modified);
}
if let Some(accessed) = metadata
.accessed()
.ok()
.and_then(|t| Time::try_from(t).ok())
.and_then(|t| TimeInt::try_from(Time::try_from(t).ok()?).ok())
{
timepoint.insert(Timeline::new_temporal("accessed_at"), accessed.into());
timepoint.insert(Timeline::new_temporal("accessed_at"), accessed);
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/re_data_source/src/data_loader/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ impl DataLoaderSettings {
}

if let Some(timepoint) = timepoint {
if timepoint.is_timeless() {
if timepoint.is_static() {
args.push("--timeless".to_owned());
}

Expand Down
4 changes: 3 additions & 1 deletion crates/re_data_store/benches/arrow2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,10 @@ fn erased_clone(c: &mut Criterion) {
.map(|array| array.total_size_bytes())
.sum::<u64>();
let expected_total_size_bytes = data.total_size_bytes();
// NOTE: `+ 1` because the computation is off by one bytes, which is irrelevant for the
// purposes of this benchmark.
assert!(
total_size_bytes >= expected_total_size_bytes,
total_size_bytes + 1 >= expected_total_size_bytes,
"Size for {} calculated to be {} bytes, but should be at least {} bytes",
T::name(),
total_size_bytes,
Expand Down
8 changes: 4 additions & 4 deletions crates/re_data_store/benches/data_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ fn insert_same_time_point(c: &mut Criterion) {
group.throughput(criterion::Throughput::Elements(num_rows * num_instances));

let rows = build_rows_ex(num_rows as _, num_instances as _, shuffled, packed, |_| {
TimePoint::from([build_frame_nr(TimeInt::from(0))])
TimePoint::from([build_frame_nr(TimeInt::ZERO)])
});

// Default config
Expand Down Expand Up @@ -385,7 +385,7 @@ fn build_rows_with_packed(packed: bool) -> Vec<DataRow> {
NUM_INSTANCES as _,
false,
packed,
|row_idx| TimePoint::from([build_frame_nr((row_idx as i64).into())]),
|row_idx| TimePoint::from([build_frame_nr(row_idx as i64)]),
)
}

Expand Down Expand Up @@ -453,7 +453,7 @@ fn latest_data_at<const N: usize>(
secondaries: &[ComponentName; N],
) -> [Option<DataCell>; N] {
let timeline_frame_nr = Timeline::new("frame_nr", TimeType::Sequence);
let timeline_query = LatestAtQuery::new(timeline_frame_nr, (NUM_ROWS / 2).into());
let timeline_query = LatestAtQuery::new(timeline_frame_nr, NUM_ROWS / 2);
let ent_path = EntityPath::from("large_structs");

store
Expand All @@ -466,7 +466,7 @@ fn range_data<const N: usize>(
components: [ComponentName; N],
) -> impl Iterator<Item = (Option<TimeInt>, [Option<DataCell>; N])> + '_ {
let timeline_frame_nr = Timeline::new("frame_nr", TimeType::Sequence);
let query = RangeQuery::new(timeline_frame_nr, TimeRange::new(0.into(), NUM_ROWS.into()));
let query = RangeQuery::new(timeline_frame_nr, TimeRange::new(TimeInt::ZERO, NUM_ROWS));
let ent_path = EntityPath::from("large_structs");

store
Expand Down
4 changes: 2 additions & 2 deletions crates/re_data_store/benches/gc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ fn plotting_dashboard(c: &mut Criterion) {
let mut timegen = |i| {
[
build_log_time(Time::from_seconds_since_epoch(i as _)),
build_frame_nr((i as i64).into()),
build_frame_nr(i as i64),
]
.into()
};
Expand Down Expand Up @@ -160,7 +160,7 @@ fn timeless_logs(c: &mut Criterion) {
time_budget: std::time::Duration::MAX,
};

let mut timegen = |_| TimePoint::timeless();
let mut timegen = |_| TimePoint::default();

let mut datagen = |i: usize| {
Box::new(re_types::archetypes::TextLog::new(i.to_string())) as Box<dyn AsComponents>
Expand Down
4 changes: 2 additions & 2 deletions crates/re_data_store/src/store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -317,8 +317,8 @@ impl DataStore {
let entry = oldest_time_per_timeline
.entry(bucket.timeline)
.or_insert(TimeInt::MAX);
if let Some(time) = bucket.inner.read().col_time.front() {
*entry = TimeInt::min(*entry, (*time).into());
if let Some(&time) = bucket.inner.read().col_time.front() {
*entry = TimeInt::min(*entry, TimeInt::new_temporal(time));
}
}
}
Expand Down
5 changes: 3 additions & 2 deletions crates/re_data_store/src/store_dump.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ use std::collections::BTreeMap;

use arrow2::Either;
use re_log_types::{
DataCellColumn, DataRow, DataTable, ErasedTimeVec, RowIdVec, TableId, TimeRange, Timeline,
DataCellColumn, DataRow, DataTable, ErasedTimeVec, RowIdVec, TableId, TimeInt, TimeRange,
Timeline,
};

use crate::{
Expand Down Expand Up @@ -248,6 +249,6 @@ fn filter_column<'a, T: 'a + Clone>(
col_time
.iter()
.zip(column)
.filter(move |(time, _)| time_filter.contains((**time).into()))
.filter(move |(&time, _)| time_filter.contains(TimeInt::new_temporal(time)))
.map(|(_, v)| v.clone())
}
40 changes: 20 additions & 20 deletions crates/re_data_store/src/store_event.rs
Original file line number Diff line number Diff line change
Expand Up @@ -297,9 +297,9 @@ mod tests {

let row_id1 = RowId::new();
let timepoint1 = TimePoint::from_iter([
(timeline_frame, 42.into()), //
(timeline_other, 666.into()), //
(timeline_yet_another, 1.into()), //
(timeline_frame, 42), //
(timeline_other, 666), //
(timeline_yet_another, 1), //
]);
let entity_path1: EntityPath = "entity_a".into();
let row1 = DataRow::from_component_batches(
Expand Down Expand Up @@ -328,9 +328,9 @@ mod tests {
(InstanceKey::name(), 1), //
],
[
(42.into(), 1), //
(666.into(), 1),
(1.into(), 1),
(42.try_into().unwrap(), 1), //
(666.try_into().unwrap(), 1),
(1.try_into().unwrap(), 1),
],
0,
),
Expand All @@ -339,8 +339,8 @@ mod tests {

let row_id2 = RowId::new();
let timepoint2 = TimePoint::from_iter([
(timeline_frame, 42.into()), //
(timeline_yet_another, 1.into()), //
(timeline_frame, 42), //
(timeline_yet_another, 1), //
]);
let entity_path2: EntityPath = "entity_b".into();
let row2 = {
Expand Down Expand Up @@ -380,17 +380,17 @@ mod tests {
(MyColor::name(), 1), //
],
[
(42.into(), 2), //
(666.into(), 1),
(1.into(), 2),
(42.try_into().unwrap(), 2), //
(666.try_into().unwrap(), 1),
(1.try_into().unwrap(), 2),
],
0,
),
view,
);

let row_id3 = RowId::new();
let timepoint3 = TimePoint::timeless();
let timepoint3 = TimePoint::default();
let row3 = {
let num_instances = 6;
let colors = vec![MyColor::from(0x00DD00FF); num_instances];
Expand Down Expand Up @@ -429,9 +429,9 @@ mod tests {
(MyColor::name(), 2), //
],
[
(42.into(), 2), //
(666.into(), 1),
(1.into(), 2),
(42.try_into().unwrap(), 2), //
(666.try_into().unwrap(), 1),
(1.try_into().unwrap(), 2),
],
1,
),
Expand Down Expand Up @@ -463,9 +463,9 @@ mod tests {
(MyColor::name(), 0), //
],
[
(42.into(), 0), //
(666.into(), 0),
(1.into(), 0),
(42.try_into().unwrap(), 0), //
(666.try_into().unwrap(), 0),
(1.try_into().unwrap(), 0),
],
0,
),
Expand All @@ -487,7 +487,7 @@ mod tests {

let row1 = DataRow::from_component_batches(
RowId::new(),
TimePoint::from_iter([(timeline_frame, 42.into())]),
TimePoint::from_iter([(timeline_frame, 42)]),
"entity_a".into(),
[&InstanceKey::from_iter(0..10) as _],
)?;
Expand All @@ -504,7 +504,7 @@ mod tests {

let row2 = DataRow::from_component_batches(
RowId::new(),
TimePoint::from_iter([(timeline_frame, 42.into())]),
TimePoint::from_iter([(timeline_frame, 42)]),
"entity_b".into(),
[&[MyColor::from(0xAABBCCDD)] as _],
)?;
Expand Down
2 changes: 1 addition & 1 deletion crates/re_data_store/src/store_format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ impl std::fmt::Display for IndexedBucket {

let time_range = {
let time_range = &self.inner.read().time_range;
if time_range.min != TimeInt::MAX && time_range.max != TimeInt::MIN {
if time_range.min() != TimeInt::MAX && time_range.max() != TimeInt::MIN {
format!(
" - {}: {}",
self.timeline.name(),
Expand Down
25 changes: 13 additions & 12 deletions crates/re_data_store/src/store_gc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use web_time::Instant;

use nohash_hasher::IntMap;
use re_log_types::{
DataCell, EntityPath, EntityPathHash, RowId, TimePoint, TimeRange, Timeline,
DataCell, EntityPath, EntityPathHash, RowId, TimeInt, TimePoint, TimeRange, Timeline,
VecDequeRemovalExt as _,
};
use re_types_core::{ComponentName, SizeBytes as _};
Expand Down Expand Up @@ -424,7 +424,7 @@ impl DataStore {
for (&timeline, &time) in timepoint {
if let Some(table) = tables.get_mut(&(*entity_path_hash, timeline)) {
let (removed, num_bytes_removed) =
table.try_drop_row(cluster_cell_cache, *row_id, time.as_i64());
table.try_drop_row(cluster_cell_cache, *row_id, time);
if let Some(inner) = diff.as_mut() {
if let Some(removed) = removed {
inner.times.extend(removed.times);
Expand All @@ -438,7 +438,7 @@ impl DataStore {

// TODO(jleibs): This is a worst-case removal-order. Would be nice to collect all the rows
// first and then remove them in one pass.
if timepoint.is_timeless() && gc_timeless {
if timepoint.is_static() && gc_timeless {
for table in timeless_tables.values_mut() {
// let deleted_comps = deleted.timeless.entry(ent_path.clone()_hash).or_default();
let (removed, num_bytes_removed) =
Expand Down Expand Up @@ -642,7 +642,8 @@ impl DataStore {
.entry(row_id)
.or_insert_with(|| StoreDiff::deletion(row_id, entity_path.clone()));

diff.times.push((bucket.timeline, time.into()));
diff.times
.push((bucket.timeline, TimeInt::new_temporal(time)));

for column in &mut inner.columns.values_mut() {
let cell = column[i].take();
Expand Down Expand Up @@ -702,7 +703,7 @@ impl IndexedTable {
let mut diff = StoreDiff::deletion(row_id, ent_path.clone());

if let Some(time) = col_time.pop_front() {
diff.times.push((timeline, time.into()));
diff.times.push((timeline, TimeInt::new_temporal(time)));
}

for (component_name, column) in &mut columns {
Expand Down Expand Up @@ -747,7 +748,7 @@ impl IndexedTable {
&mut self,
cluster_cache: &ClusterCellCache,
row_id: RowId,
time: i64,
time: TimeInt,
) -> (Option<StoreDiff>, u64) {
re_tracing::profile_function!();

Expand All @@ -757,7 +758,7 @@ impl IndexedTable {

let table_has_more_than_one_bucket = self.buckets.len() > 1;

let (bucket_key, bucket) = self.find_bucket_mut(time.into());
let (bucket_key, bucket) = self.find_bucket_mut(time);
let bucket_num_bytes = bucket.total_size_bytes();

let (diff, mut dropped_num_bytes) = {
Expand Down Expand Up @@ -806,7 +807,7 @@ impl IndexedBucketInner {
row_id: RowId,
timeline: Timeline,
ent_path: &EntityPath,
time: i64,
time: TimeInt,
) -> (Option<StoreDiff>, u64) {
self.sort();

Expand All @@ -825,8 +826,8 @@ impl IndexedBucketInner {
let mut diff: Option<StoreDiff> = None;
let mut dropped_num_bytes = 0u64;

let mut row_index = col_time.partition_point(|&time2| time2 < time);
while col_time.get(row_index) == Some(&time) {
let mut row_index = col_time.partition_point(|&time2| time2 < time.as_i64());
while col_time.get(row_index) == Some(&time.as_i64()) {
if col_row_id[row_index] != row_id {
row_index += 1;
continue;
Expand All @@ -842,11 +843,11 @@ impl IndexedBucketInner {
// We have at least two rows, so we can safely [index] here:
if row_index == 0 {
// We removed the first row, so the second row holds the new min
time_range.min = col_time[1].into();
time_range.set_min(col_time[1]);
}
if row_index + 1 == col_time.len() {
// We removed the last row, so the penultimate row holds the new max
time_range.max = col_time[row_index - 1].into();
time_range.set_max(col_time[row_index - 1]);
}
}

Expand Down
Loading

0 comments on commit eb6270d

Please sign in to comment.