From afa7490ef9c263aff2f2219b5314c99a673aaf2d Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Wed, 7 Aug 2024 09:23:27 +0200 Subject: [PATCH] Optimize gap detector on dense timelines, like `log_tick` (#7082) ### What * Closes https://github.com/rerun-io/rerun/issues/5162 We hit a specially bad case looking for small gaps on dense timeliens, like on `log_tick`. The new code does an early-out when the gap we're looking for becomes much smaller than the total time of the (non-gap) data. ### Checklist * [x] I have read and agree to [Contributor Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and the [Code of Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md) * [x] I've included a screenshot or gif (if applicable) * [x] I have tested the web demo (if applicable): * Using examples from latest `main` build: [rerun.io/viewer](https://rerun.io/viewer/pr/7082?manifest_url=https://app.rerun.io/version/main/examples_manifest.json) * Using full set of examples from `nightly` build: [rerun.io/viewer](https://rerun.io/viewer/pr/7082?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json) * [x] The PR title and labels are set such as to maximize their usefulness for the next release's CHANGELOG * [x] If applicable, add a new check to the [release checklist](https://github.com/rerun-io/rerun/blob/main/tests/python/release_checklist)! * [x] If have noted any breaking changes to the log API in `CHANGELOG.md` and the migration guide - [PR Build Summary](https://build.rerun.io/pr/7082) - [Recent benchmark results](https://build.rerun.io/graphs/crates.html) - [Wasm size tracking](https://build.rerun.io/graphs/sizes.html) To run all checks from `main`, comment on the PR with `@rerun-bot full-check`. --- Cargo.lock | 1 + crates/utils/re_int_histogram/src/lib.rs | 5 ++ crates/viewer/re_time_panel/Cargo.toml | 3 +- crates/viewer/re_time_panel/src/time_axis.rs | 48 ++++++++++++++------ 4 files changed, 42 insertions(+), 15 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5a2602acfd31..1d8e01757dfd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5092,6 +5092,7 @@ dependencies = [ "re_data_ui", "re_entity_db", "re_format", + "re_int_histogram", "re_log_types", "re_tracing", "re_types", diff --git a/crates/utils/re_int_histogram/src/lib.rs b/crates/utils/re_int_histogram/src/lib.rs index 8168ff9d5509..e65e13fc1a38 100644 --- a/crates/utils/re_int_histogram/src/lib.rs +++ b/crates/utils/re_int_histogram/src/lib.rs @@ -115,6 +115,11 @@ impl RangeI64 { pub fn contains(&self, value: i64) -> bool { self.min <= value && value <= self.max } + + #[inline] + pub fn length(&self) -> u64 { + (self.max - self.min + 1) as u64 + } } impl std::fmt::Debug for RangeI64 { diff --git a/crates/viewer/re_time_panel/Cargo.toml b/crates/viewer/re_time_panel/Cargo.toml index 25dbdc6b381f..ba1a4628d5e1 100644 --- a/crates/viewer/re_time_panel/Cargo.toml +++ b/crates/viewer/re_time_panel/Cargo.toml @@ -19,11 +19,12 @@ workspace = true all-features = true [dependencies] -re_context_menu.workspace = true re_chunk_store.workspace = true +re_context_menu.workspace = true re_data_ui.workspace = true re_entity_db.workspace = true re_format.workspace = true +re_int_histogram.workspace = true re_log_types.workspace = true re_tracing.workspace = true re_types.workspace = true diff --git a/crates/viewer/re_time_panel/src/time_axis.rs b/crates/viewer/re_time_panel/src/time_axis.rs index 4ca217262090..fe7b0252203a 100644 --- a/crates/viewer/re_time_panel/src/time_axis.rs +++ b/crates/viewer/re_time_panel/src/time_axis.rs @@ -1,5 +1,3 @@ -use itertools::Itertools as _; - use re_entity_db::TimeHistogram; use re_log_types::{ResolvedTimeRange, TimeInt, TimeType}; @@ -65,7 +63,8 @@ fn gap_size_heuristic(time_type: TimeType, times: &TimeHistogram) -> u64 { TimeType::Time => TimeInt::from_milliseconds(100.try_into().unwrap()).as_i64() as _, }; // Collect all gaps larger than our minimum gap size. - let mut gap_sizes = collect_candidate_gaps(times, min_gap_size, max_collapses); + let mut gap_sizes = + collect_candidate_gaps(times, min_gap_size, max_collapses).unwrap_or_default(); gap_sizes.sort_unstable(); // Only collapse gaps that take up a significant portion of the total time, @@ -91,11 +90,12 @@ fn gap_size_heuristic(time_type: TimeType, times: &TimeHistogram) -> u64 { gap_threshold } +/// Returns `None` to signal an abort. fn collect_candidate_gaps( times: &TimeHistogram, min_gap_size: u64, max_collapses: usize, -) -> Vec { +) -> Option> { re_tracing::profile_function!(); // We want this to be fast, even when we have _a lot_ of times. // `TimeHistogram::range` has a granularity argument: @@ -107,26 +107,46 @@ fn collect_candidate_gaps( let max_gap_size = times.max_key().unwrap() - times.min_key().unwrap(); let mut granularity = max_gap_size as u64; - let mut gaps = collect_gaps_with_granularity(times, granularity, min_gap_size); + let mut gaps = collect_gaps_with_granularity(times, granularity, min_gap_size)?; while gaps.len() < max_collapses && min_gap_size < granularity { granularity /= 2; - gaps = collect_gaps_with_granularity(times, granularity, min_gap_size); + gaps = collect_gaps_with_granularity(times, granularity, min_gap_size)?; } - gaps + Some(gaps) } +/// Returns `None` to signal an abort. fn collect_gaps_with_granularity( times: &TimeHistogram, granularity: u64, min_gap_size: u64, -) -> Vec { +) -> Option> { re_tracing::profile_function!(); - times - .range(.., granularity) - .tuple_windows() - .map(|((a, _), (b, _))| a.max.abs_diff(b.min)) - .filter(|&gap_size| min_gap_size < gap_size) - .collect_vec() + + let mut non_gap_time_span = 0; + + let mut gaps = vec![]; + let mut last_range: Option = None; + + for (range, _count) in times.range(.., granularity) { + non_gap_time_span += range.length(); + + if let Some(last_range) = last_range { + let gap_size = last_range.max.abs_diff(range.min); + if min_gap_size < gap_size { + gaps.push(gap_size); + } + } + last_range = Some(range); + } + + if min_gap_size * 100 < non_gap_time_span { + // If the gap is such a small fracion of the total time, we don't care about it, + // and we abort the gap-search, which is an important early-out. + return None; + } + + Some(gaps) } /// Collapse any gaps larger or equals to the given threshold.