Skip to content

Commit

Permalink
Optimize gap detector on dense timelines, like log_tick (#7082)
Browse files Browse the repository at this point in the history
### What
* Closes #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`.
  • Loading branch information
emilk authored Aug 7, 2024
1 parent cb89542 commit afa7490
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 15 deletions.
1 change: 1 addition & 0 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -5092,6 +5092,7 @@ dependencies = [
"re_data_ui",
"re_entity_db",
"re_format",
"re_int_histogram",
"re_log_types",
"re_tracing",
"re_types",
Expand Down
5 changes: 5 additions & 0 deletions crates/utils/re_int_histogram/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
3 changes: 2 additions & 1 deletion crates/viewer/re_time_panel/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
48 changes: 34 additions & 14 deletions crates/viewer/re_time_panel/src/time_axis.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
use itertools::Itertools as _;

use re_entity_db::TimeHistogram;
use re_log_types::{ResolvedTimeRange, TimeInt, TimeType};

Expand Down Expand Up @@ -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,
Expand All @@ -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<u64> {
) -> Option<Vec<u64>> {
re_tracing::profile_function!();
// We want this to be fast, even when we have _a lot_ of times.
// `TimeHistogram::range` has a granularity argument:
Expand All @@ -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<u64> {
) -> Option<Vec<u64>> {
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<re_int_histogram::RangeI64> = 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.
Expand Down

0 comments on commit afa7490

Please sign in to comment.